[Bug 739284] decklinksrc: add automatic mode detection and timeout property

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=739284
  GStreamer | gst-plugins-bad | git

Sebastian Dröge (slomo) <slomo> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #293953|none                        |needs-work
             status|                            |

--- Comment #8 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2015-01-08 12:03:47 UTC ---
Review of attachment 293953:
 --> (https://bugzilla.gnome.org/review?bug=739284&attachment=293953)

::: sys/decklink/gstdecklink.cpp
@@ +193,3 @@
+  switch(mode) {
+    case bmdModeNTSC:
+      displayMode = GST_DECKLINK_MODE_NTSC;

This could also become a table like for the connections:

{ bmdModeNTSC, GST_DECKLINK_MODE_NTSC },
{ bmdModePAL,  GST_DECKLINK_MODE_PAL  },
etc

@@ +283,3 @@
+      break;
+    default:
+      break;

default case should do a g_assert_not_reached()

@@ +452,3 @@
+    g_mutex_lock (&m_input->lock);
+
+    src = GST_DECKLINK_VIDEO_SRC_CAST(m_input->videosrc);

This should be done via a callback that is defined in gstdecklinkvideosrc.cpp

@@ +457,3 @@
+    m_input->input->FlushStreams();
+
+    m_input->input->EnableVideoInput (mode->GetDisplayMode(),

Isn't it necessary to first DisableVideoInput()?

@@ +463,3 @@
+    gst_video_info_from_caps (&src->info, caps);
+    gst_base_src_set_caps(GST_BASE_SRC_CAST(m_input->videosrc), caps);
+    gst_caps_unref (caps);

To make this work in a threadsafe way you would need to store the caps together
with the buffers and their timestamps inside the queue, and then inside
create() set new caps if they have changed

Otherwise the queue might still contain buffers with old caps after you set the
new caps. And you must only set the caps from the streaming thread (i.e. from
create()).

::: sys/decklink/gstdecklink.h
@@ +84,3 @@
+  GST_DECKLINK_MODE_3184p60,
+
+  GST_DECKLINK_MODE_UNKNOWN

Why is UNKNOWN needed? Ideally you would give it the value -1 or something like
that so we can easily extend this enum here later

::: sys/decklink/gstdecklinkvideosrc.cpp
@@ +448,3 @@

+  flags = bmdVideoInputFlagDefault;
+  if (self->mode == GST_DECKLINK_MODE_AUTO || self->mode ==
GST_DECKLINK_MODE_UNKNOWN) {

When would it ever be UNKNOWN?

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.