diff mbox series

[08/10] media: coda: allow encoder to set colorimetry on the output queue

Message ID 20190408123256.22868-8-p.zabel@pengutronix.de (mailing list archive)
State New, archived
Headers show
Series [01/10] media: coda: set codec earlier | expand

Commit Message

Philipp Zabel April 8, 2019, 12:32 p.m. UTC
v4l2-compliance sets colorimetry on the output queue and then verifies
that querying colorimetry on the capture queue returns the same
configuration. For this to work, the encoder must allow setting context
colorimetry parameters on the output queue.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-common.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Hans Verkuil April 10, 2019, 1:48 p.m. UTC | #1
On 4/8/19 2:32 PM, Philipp Zabel wrote:
> v4l2-compliance sets colorimetry on the output queue and then verifies
> that querying colorimetry on the capture queue returns the same
> configuration. For this to work, the encoder must allow setting context
> colorimetry parameters on the output queue.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/coda/coda-common.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index 2966eb1c4d2d..7b89dbae938d 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -819,6 +819,11 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
>  	if (ret)
>  		return ret;
>  
> +	ctx->colorspace = f->fmt.pix.colorspace;
> +	ctx->xfer_func = f->fmt.pix.xfer_func;
> +	ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
> +	ctx->quantization = f->fmt.pix.quantization;
> +
>  	if (ctx->inst_type != CODA_INST_DECODER)
>  		return 0;
>  
> @@ -831,11 +836,6 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
>  	}
>  	ctx->codec = codec;
>  
> -	ctx->colorspace = f->fmt.pix.colorspace;
> -	ctx->xfer_func = f->fmt.pix.xfer_func;
> -	ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
> -	ctx->quantization = f->fmt.pix.quantization;
> -
>  	dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
>  	if (!dst_vq)
>  		return -EINVAL;
> 

Isn't the colorimetry information encoded in the stream's metadata? So should
it be set in an encoder register so it ends up in the right place in the bitstream?

And for decoders it should then be read back from a register.

Just curious how this would work for this codec and if it is even possible.

For now this patch is OK, but I think more work is needed.

Regards,

	Hans
Philipp Zabel April 10, 2019, 2:23 p.m. UTC | #2
On Wed, 2019-04-10 at 15:48 +0200, Hans Verkuil wrote:
[...]
> Isn't the colorimetry information encoded in the stream's metadata?

That depends on the codec. Colorimetry information can be stored in
optional headers (for example Sequence Display Extension for MPEG-2,
Video Usability Information for H.264) but I don't know that the CODA
firmware can parse or generate any of them.

> So should it be set i an encoder register so it ends up in the right
> place in the bitstream?

I could produce the necessary headers manually and inject them into the
bitstream in the driver.

But for example for JPEG it is not clear to me if there even is a
correct way to do that. Can V4L2 colorimetry settings be translated into
ICC profiles?

> And for decoders it should then be read back from a register.

Same as above, I'd have to parse the headers in the driver.

> Just curious how this would work for this codec and if it is even possible.
> 
> For now this patch is OK, but I think more work is needed.

Agreed.

regards
Philipp
diff mbox series

Patch

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 2966eb1c4d2d..7b89dbae938d 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -819,6 +819,11 @@  static int coda_s_fmt_vid_out(struct file *file, void *priv,
 	if (ret)
 		return ret;
 
+	ctx->colorspace = f->fmt.pix.colorspace;
+	ctx->xfer_func = f->fmt.pix.xfer_func;
+	ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
+	ctx->quantization = f->fmt.pix.quantization;
+
 	if (ctx->inst_type != CODA_INST_DECODER)
 		return 0;
 
@@ -831,11 +836,6 @@  static int coda_s_fmt_vid_out(struct file *file, void *priv,
 	}
 	ctx->codec = codec;
 
-	ctx->colorspace = f->fmt.pix.colorspace;
-	ctx->xfer_func = f->fmt.pix.xfer_func;
-	ctx->ycbcr_enc = f->fmt.pix.ycbcr_enc;
-	ctx->quantization = f->fmt.pix.quantization;
-
 	dst_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 	if (!dst_vq)
 		return -EINVAL;