From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7DE28C433F5 for ; Thu, 21 Apr 2022 10:38:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1388462AbiDUKkx convert rfc822-to-8bit (ORCPT ); Thu, 21 Apr 2022 06:40:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59892 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1388461AbiDUKkw (ORCPT ); Thu, 21 Apr 2022 06:40:52 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 21B6225C7E for ; Thu, 21 Apr 2022 03:38:03 -0700 (PDT) Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nhUCH-0000Qa-Hh; Thu, 21 Apr 2022 12:38:01 +0200 Received: from [2a0a:edc0:0:900:1d::4e] (helo=lupine) by drehscheibe.grey.stw.pengutronix.de with esmtp (Exim 4.94.2) (envelope-from ) id 1nhUCI-004LGj-5H; Thu, 21 Apr 2022 12:38:00 +0200 Received: from pza by lupine with local (Exim 4.94.2) (envelope-from ) id 1nhUCG-0007Jo-6i; Thu, 21 Apr 2022 12:38:00 +0200 Message-ID: <4ddc131113b41bf8427d0b316b70335578971ff4.camel@pengutronix.de> Subject: Re: [PATCH 5/7] media: coda: fix default JPEG colorimetry From: Philipp Zabel To: Hans Verkuil , linux-media@vger.kernel.org Cc: Mauro Carvalho Chehab , kernel@pengutronix.de Date: Thu, 21 Apr 2022 12:38:00 +0200 In-Reply-To: <0f5d9c16-860b-015f-8028-234d2fb96959@xs4all.nl> References: <20220404163533.707508-1-p.zabel@pengutronix.de> <20220404163533.707508-5-p.zabel@pengutronix.de> <0f5d9c16-860b-015f-8028-234d2fb96959@xs4all.nl> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT User-Agent: Evolution 3.38.3-1 MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-media@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On Do, 2022-04-21 at 12:02 +0200, Hans Verkuil wrote: > Hi Philipp, > > On 04/04/2022 18:35, Philipp Zabel wrote: > > Set default colorspace to SRGB for JPEG encoder and decoder devices, > > to fix the following v4l2-compliance test failure: > > > > test VIDIOC_TRY_FMT: OK > > fail: v4l2-test-formats.cpp(818): fmt_raw.g_colorspace() != V4L2_COLORSPACE_SRGB > > > > Also explicitly set transfer function, YCbCr encoding and quantization > > range, as required by v4l2-compliance for the JPEG encoded side. > > I'm not quite sure if this patch addresses the correct issue. > > > > > Signed-off-by: Philipp Zabel > > --- > >  .../media/platform/chips-media/coda-common.c | 36 +++++++++++++------ > >  1 file changed, 25 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/media/platform/chips-media/coda-common.c b/drivers/media/platform/chips-media/coda-common.c > > index 4a7346ed771e..c068c16d1eb4 100644 > > --- a/drivers/media/platform/chips-media/coda-common.c > > +++ b/drivers/media/platform/chips-media/coda-common.c > > @@ -732,13 +732,22 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv, > >   return 0; > >  } > >   > > > > > > > > -static void coda_set_default_colorspace(struct v4l2_pix_format *fmt) > > +static void coda_set_default_colorspace(struct coda_ctx *ctx, > > + struct v4l2_pix_format *fmt) > >  { > >   enum v4l2_colorspace colorspace; > >   > > > > > > > > - if (fmt->pixelformat == V4L2_PIX_FMT_JPEG) > > - colorspace = V4L2_COLORSPACE_JPEG; > > It's perfectly fine to keep this, the problem occurs with the raw image side > (capture for the decoder, output for the encoder). > > There the colorspace must be SRGB, the xfer_func may be 0 or SRGB, and the > ycbcr_enc is 0 (if not a YUV pixelformat) or ENC_601 (if it is a YUV format). > Actually, if the hardware can convert from other YUV encodings such as 709, > then other YUV encodings are valid, but I assume that's not the case. So the driver has to support different colorspace on output and capture queues? > > - else if (fmt->width <= 720 && fmt->height <= 576) > > + if (ctx->cvd->src_formats[0] == V4L2_PIX_FMT_JPEG || > > + ctx->cvd->dst_formats[0] == V4L2_PIX_FMT_JPEG || > > + fmt->pixelformat == V4L2_PIX_FMT_JPEG) { > > + fmt->colorspace = V4L2_COLORSPACE_SRGB; > > + fmt->xfer_func = V4L2_XFER_FUNC_SRGB; > > + fmt->ycbcr_enc = V4L2_YCBCR_ENC_601; > > + fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE; > > + return; > > + } > > + > > + if (fmt->width <= 720 && fmt->height <= 576) > >   colorspace = V4L2_COLORSPACE_SMPTE170M; > >   else > >   colorspace = V4L2_COLORSPACE_REC709; > > @@ -763,7 +772,7 @@ static int coda_try_fmt_vid_out(struct file *file, void *priv, > >   return ret; > >   > > > > > > > >   if (f->fmt.pix.colorspace == V4L2_COLORSPACE_DEFAULT) > > - coda_set_default_colorspace(&f->fmt.pix); > > + coda_set_default_colorspace(ctx, &f->fmt.pix); > >   > > > > > > > >   q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE); > >   codec = coda_find_codec(dev, f->fmt.pix.pixelformat, q_data_dst->fourcc); > > @@ -1640,13 +1649,18 @@ static void set_default_params(struct coda_ctx *ctx) > >   csize = coda_estimate_sizeimage(ctx, usize, max_w, max_h); > >   > > > > > > > >   ctx->params.codec_mode = ctx->codec->mode; > > - if (ctx->cvd->src_formats[0] == V4L2_PIX_FMT_JPEG) > > - ctx->colorspace = V4L2_COLORSPACE_JPEG; > > - else > > + if (ctx->cvd->src_formats[0] == V4L2_PIX_FMT_JPEG || > > + ctx->cvd->dst_formats[0] == V4L2_PIX_FMT_JPEG) { > > + ctx->colorspace = V4L2_COLORSPACE_SRGB; > > + ctx->xfer_func = V4L2_XFER_FUNC_SRGB; > > + ctx->ycbcr_enc = V4L2_YCBCR_ENC_601; > > + ctx->quantization = V4L2_QUANTIZATION_FULL_RANGE; > > + } else { > >   ctx->colorspace = V4L2_COLORSPACE_REC709; > > My guess is that the raw format colorspace is set to REC709, which is definitely > wrong for a JPEG codec. For a JPEG codec that must be set to SRGB. > > I suspect that's the real bug here. > > I'm skipping this patch for now. Thank you, I think at least for the decoder the issue was that the driver defaulted to V4L2_COLORSPACE_JPEG, but since ctx->colorspace is used for both sides, that would also be used as colorspace for the raw image side. For the encoder it looks like you are right. I'll double check. regards Philipp