[Bug 742878] decklink: Add initial support for 10bit YUV modes

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=742878

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

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

--- Comment #13 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 326256:
 --> (https://bugzilla.gnome.org/review?bug=742878&attachment=326256)

::: sys/decklink/gstdecklink.h
@@ +121,3 @@
+  GST_DECKLINK_VIDEO_10BIT_RGBXLE, /* bmdFormat10BitRGBXLE */
+  GST_DECKLINK_VIDEO_10BIT_RGBX, /* bmdFormat10BitRGBX */
+} GstDecklinkVideoFormatType;

GstDecklinkVideoFormat and GST_DECKLINK_VIDEO_FORMAT_AUTO, etc? Seems more
consistent

::: sys/decklink/gstdecklinkvideosink.cpp
@@ +208,3 @@

+  g_object_class_install_property (gobject_class, PROP_VIDEO_FORMAT,
+      g_param_spec_enum ("video-format-type", "Video format type",

And then here also video-format only

@@ +481,3 @@

+  switch (self->video_format) {
+    case GST_DECKLINK_VIDEO_AUTO:

For AUTO in the sink it might make sense to allow all formats in get_caps() and
then select the appropriate format based on the caps in set_caps(). As an
additional patch I guess, the same applies for the existing mode property.

@@ +488,3 @@
+      bpp = 4;
+      break;
+    default:

It would probably be good if a GST_ELEMENT_ERROR() would happen here... but
also in set_property() already a GST_WARNING or so about selecting an
unsupported format. Handling the RGB formats that are handled by the source
already should be easy though

::: sys/decklink/gstdecklinkvideosrc.cpp
@@ +387,3 @@
+  mode_caps = gst_decklink_mode_get_caps (mode, format);
+
+  if (gst_caps_is_empty (mode_caps)) {

Wouldn't this always return non-empty caps?

@@ +569,2 @@
     caps = gst_decklink_mode_get_caps (f->mode, f->format);
+    if (gst_caps_is_empty (caps)) {

And here. Isn't the actual problem you want to check here the case when
mode!=AUTO && f->mode!=mode (and the same with the format)? That is, we
captured a frame different to what is configured on the element

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.