Created attachment 327806 [details] [review]
v4l2object: fill colorimetry in gst_v4l2_object_acquire_format

Review of attachment 327801 [details] [review]:

::: sys/v4l2/gstv4l2object.c
@@ +3714,3 @@
+      if (is_rgb)
+        info.colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;
+  }

That's a copy paste, don't.

Review of attachment 327806 [details] [review]:

::: sys/v4l2/gstv4l2object.c
@@ +3714,3 @@
+      if (is_rgb)
+        info->colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;
+  }

That's copy paste, don't.

As in "don't copy&paste, refactor and reuse instead", or as in "don't set the colorimetry matrix here"?

(In reply to Philipp Zabel from comment #4)
> As in "don't copy&paste, refactor and reuse instead", or as in "don't set
> the colorimetry matrix here"?

I mean, this patch should first refactor, and use the newly create helper function.

Created attachment 328121 [details] [review]
v4l2object: refactor gst_v4l2_object_get_colorspace to take a v4l2_format parameter

Move the extraction of colorimetry parameters from struct v4l2_format and the
setting of the identity matrix for RGB formats into the function to avoid code
duplication.

Created attachment 328122 [details] [review]
v4l2object: fill colorimetry in gst_v4l2_object_acquire_format

Instead of relying on the default colorimetry chosen by gst_video_info_set_format(), set info.colorimetry from the values returned by G_FMT. This allows decoders to propagate their input colorimetry downstream.

Thanks, I'll review soon.

All looks good now, thanks.

I'd like to proof test this one, but it seems a good candidate for 1.8 since
it likely fixex a regression caused by the introductions of colorimetry
(assuming) the driver is correct). Philipp, any opinion on this ?

Attachment 328121 [details] pushed as b5530a1 - v4l2object: refactor gst_v4l2_object_get_colorspace to take a v4l2_format parameter
Attachment 328122 [details] pushed as 616ccbb - v4l2object: fill colorimetry in gst_v4l2_object_acquire_format