Coda: imx53 plays video with incorrect colors

[Philipp Zabel]

Hi Fabio,

On Mon, 2021-01-18 at 10:28 -0300, Fabio Estevam wrote:
> Hi Philipp,
> 
> Thanks for your reply.
> 
> On Mon, Jan 18, 2021 at 9:40 AM Philipp Zabel <p.zabel at pengutronix.de> wrote:
> 
> > The driver could be modified to switch the DP->DI0/DC->DI1 mapping
> > around to DP->DI1/DC->DI0 when required. As a simple test, you can
> > switch statically with:
> 
> It does change the colors but still does not play the video with the
> correct colors. Looks like it plays in black-and-white now.

Please try forcing decoder output to NV12 instead of I420.

> > Or, as a workaround, add a v4l2convert element and use the IC to convert
> > to BGRx between decoder and kmssink.
> 
> Yes, I have tried to do this, but it says that v4l2convert does not
> support bt601 colorimetry, and then a segfault occurs:
> 
> # gst-launch-1.0 filesrc location=/media/clip.mp4 ! qtdemux !
> h264parse ! v4l2h264dec ! v4l2convert ! video/x-raw,format=BGRx !
> kmssink
> Setting pipeline to PAUSED ...
> Pipeline is PREROLLING ...
> ERROR: from element /GstPipeline:pipeline0/v4l2convert:v4l2convert0:
> Device '/dev/video4' does not support bt601 colorimetry
> Additional debug info:
> ../sys/v4l2/gstv4l2object.c(4032): gst_v4l2_object_set_format_full ():
> /GstPipeline:pipeline0/v4l2convert:v4l2convert0:
> Device wants 2:4:5:4 colorimetry
> ERROR: pipeline doesn't want to preroll.
> Setting pipeline to NULL ...
> Caught SIGSEGV
> exec gdb failed: No such file or directory
> Spinning.  Please run 'gdb gst-launch-1.0 217' to continue debugging,
> Ctrl-C to quit, or Ctrl-\ to dump core.
> 
> Is the Gstreamer pipeline above correct?

Yes. Please try if the following patch makes it work:

----------8<----------
>From c45afcaf6fbef56a86dce19200c06df78718db60 Mon Sep 17 00:00:00 2001
From: Philipp Zabel <p.zabel at pengutronix.de>
Date: Mon, 18 Jan 2021 15:54:43 +0100
Subject: [PATCH] v4l2object: handle GST_VIDEO_TRANSFER_BT601

V4L2 makes no difference between the BT.601 and BT.709 transfer
functions [1], but GStreamer does since 1.18 [2].

Adapt gst_v4l2_object_get_colorspace() and
gst_v4l2_object_set_format_full().

[1] https://linuxtv.org/downloads/v4l-dvb-apis-new/userspace-api/v4l/colorspaces-details.html#colorspace-smpte-170m-v4l2-colorspace-smpte170m
[2] https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/724
---
 sys/v4l2/gstv4l2object.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sys/v4l2/gstv4l2object.c b/sys/v4l2/gstv4l2object.c
index ea4363e17303..b13216d75836 100644
--- a/sys/v4l2/gstv4l2object.c
+++ b/sys/v4l2/gstv4l2object.c
@@ -2334,7 +2334,7 @@ gst_v4l2_object_get_colorspace (struct v4l2_format *fmt,
     case V4L2_COLORSPACE_SMPTE170M:
       cinfo->range = GST_VIDEO_COLOR_RANGE_16_235;
       cinfo->matrix = GST_VIDEO_COLOR_MATRIX_BT601;
-      cinfo->transfer = GST_VIDEO_TRANSFER_BT709;
+      cinfo->transfer = GST_VIDEO_TRANSFER_BT601;
       cinfo->primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;
       break;
     case V4L2_COLORSPACE_REC709:
@@ -2463,6 +2463,8 @@ gst_v4l2_object_get_colorspace (struct v4l2_format *fmt,
     case V4L2_XFER_FUNC_709:
       if (colorspace == V4L2_COLORSPACE_BT2020 && fmt->fmt.pix.height >= 2160)
         cinfo->transfer = GST_VIDEO_TRANSFER_BT2020_12;
+      else if (colorspace == V4L2_COLORSPACE_SMPTE170M)
+        cinfo->transfer = GST_VIDEO_TRANSFER_BT601;
       else
         cinfo->transfer = GST_VIDEO_TRANSFER_BT709;
       break;
@@ -3855,6 +3857,7 @@ gst_v4l2_object_set_format_full (GstV4l2Object * v4l2object, GstCaps * caps,
     case GST_VIDEO_TRANSFER_GAMMA10:
       transfer = V4L2_XFER_FUNC_NONE;
       break;
+    case GST_VIDEO_TRANSFER_BT601:
     case GST_VIDEO_TRANSFER_BT2020_12:
     case GST_VIDEO_TRANSFER_BT709:
       transfer = V4L2_XFER_FUNC_709;
-- 
2.20.1
---------->8----------

This may not be the correct solution. GStreamer could keep choosing
BT709 as told by V4L2, and use the new
gst_video_color_transfer_is_equivalent() function to test for
equivalence instead.

regards
Philipp