[Bug 743186] v4l2object: set colorspace in caps for capture devices

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=743186
  GStreamer | gst-plugins-good | git master

Nicolas Dufresne (stormer) <nicolas.dufresne> changed:

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

--- Comment #2 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> 2015-01-19 16:17:50 UTC ---
Review of attachment 294894:
 --> (https://bugzilla.gnome.org/review?bug=743186&attachment=294894)

Few comments, the last one is mostly a reflection, but I'm not 100% sure it
matter.

::: sys/v4l2/gstv4l2object.c
@@ +1675,3 @@
+      break;
+    default:
+      GST_ERROR ("Unknown enum v4l2_colorspace %d", colorspace);

I think some driver don't support it, hence will set 0. I think we should
handle that case without spamming the logs. I already have two spammer to fix 
now ;-P Also, see the guessing code we do in the opposite direction.

@@ +1791,3 @@
+    fmt.fmt.pix.height = height;
+    fmt.fmt.pix.pixelformat = pixelformat;
+  }

The if case is not needed, this part of the structure is that same for mplane.

@@ +2308,3 @@
         pixelformat);
     gst_v4l2_object_add_aspect_ratio (v4l2object, tmp);
+    gst_v4l2_object_add_colorspace (v4l2object, tmp, max_w, max_h,
pixelformat);

I think this is a case where just checking the max may really lead to wrong
result. We probably want to check min and max, if if different, put both in the
caps.

-- 
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.