[PATCH v5 2/3] [media] s5p-mfc: Set colorspace in VIDIO_{G,TRY}_FMT if DEFAULT provided
From: Andrzej Hajda <hidden>
Date: 2017-02-23 13:53:40
Also in:
linux-media, linux-samsung-soc, lkml
On 21.02.2017 20:20, Thibault Saunier wrote:
quoted hunk ↗ jump to hunk
The media documentation says that the V4L2_COLORSPACE_SMPTE170M colorspace should be used for SDTV and V4L2_COLORSPACE_REC709 for HDTV but the driver didn't set the colorimetry when userspace provided V4L2_COLORSPACE_DEFAULT. Use 576p display resolution as a threshold to set this as suggested by EIA CEA 861B. Signed-off-by: Thibault Saunier <redacted> --- Changes in v5: None Changes in v4: - Set the colorspace only if the user passed V4L2_COLORSPACE_DEFAULT, in all other cases just use what userspace provided. Changes in v3: - Do not check values in the g_fmt functions as Andrzej explained in previous review - Set colorspace if user passed V4L2_COLORSPACE_DEFAULT in Changes in v2: None drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c index 367ef8e8dbf0..0976c3e0a5ce 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c@@ -354,6 +354,11 @@ static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f) pix_mp->plane_fmt[0].sizeimage = ctx->luma_size; pix_mp->plane_fmt[1].bytesperline = ctx->buf_width; pix_mp->plane_fmt[1].sizeimage = ctx->chroma_size; + + if (pix_mp->width > 720 && pix_mp->height > 576) /* HD */ + pix_mp->colorspace = V4L2_COLORSPACE_REC709; + else /* SD */ + pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M;
Again, it seems much more complicated for decoder, I suppose colorspace field should be filled according to decoded information from the header of byte stream. It looks like MFC does not parse stream header for such info. So it should be done either by the driver, either by userspace, if userspace is able to get such info. For example in H.264 this info is encoded in VUI header [1], I do not know about other codecs. I guess the best solution for now is to not touch this field unless you can retrieve this info from MFC. [1]: https://github.com/copiousfreetime/mp4parser/blob/master/isoparser/src/main/java/com/googlecode/mp4parser/h264/model/SeqParameterSet.java#L227 Regards Andrzej
quoted hunk ↗ jump to hunk
} else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { /* This is run on OUTPUT The buffer contains compressed image@@ -378,6 +383,7 @@ static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f) static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f) { struct s5p_mfc_dev *dev = video_drvdata(file); + struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; struct s5p_mfc_fmt *fmt; mfc_debug(2, "Type is %d\n", f->type);@@ -405,6 +411,14 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f) mfc_err("Unsupported format by this MFC version.\n"); return -EINVAL; } + + if (pix_mp->colorspace == V4L2_COLORSPACE_DEFAULT) { + if (pix_mp->width > 720 && + pix_mp->height > 576) /* HD */ + pix_mp->colorspace = V4L2_COLORSPACE_REC709; + else /* SD */ + pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M; + } } return 0;