Le vendredi 17 juillet 2020 à 10:14 +0200, Philipp Zabel a écrit :
> On Fri, 2020-07-17 at 00:49 -0300, Ezequiel Garcia wrote:
> > From: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> >
> > The CODA960 manual states that ASO/FMO features of baseline are not
> > supported, so for this reason this driver should only report
> > constrained baseline support.
>
> I know the encoder doesn't support this, but is this also true of the
> decoder? The i.MX6DQ Reference Manual explicitly lists H.264/AVC decoder
> support for both baseline profile and constrained base line profile.
Hmm, double checking, you are right this is documented in the encoding tools
sections, not the decoding. But there is extra buffers that need to be passed
for ASO/FMO to work, I greatly doubt you have ever tested it. This is not
supported by GStreamer parser, or FFMPEG parsers either. Again, we need to make
sure in V2 that encoding and decoding capabilities are well seperated.
As for advertising ASO/FMO, I can leave it there, but be aware I won't be
testing it. I can provide you links to streams if you care (they are publicly
accessible throught the ITU conformance streams published by the ITU). But as
for GStreamer and FFMPEG, this is not supported anyway.
>
> > This fixes negotiation issue with constrained baseline content
> > on GStreamer 1.17.1.
> >
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: 42a68012e67c2 ("media: coda: add read-only h.264 decoder
> > profile/level controls")
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> > Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> > ---
> > drivers/media/platform/coda/coda-common.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/coda/coda-common.c
> > b/drivers/media/platform/coda/coda-common.c
> > index 3ab3d976d8ca..c641d1608825 100644
> > --- a/drivers/media/platform/coda/coda-common.c
> > +++ b/drivers/media/platform/coda/coda-common.c
> > @@ -2335,8 +2335,8 @@ static void coda_encode_ctrls(struct coda_ctx *ctx)
> > V4L2_CID_MPEG_VIDEO_H264_CHROMA_QP_INDEX_OFFSET, -12, 12, 1, 0);
> > v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
> > V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> > - V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE, 0x0,
> > - V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE);
> > + V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE, 0x0,
> > + V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE);
>
> Encoder support is listed as baseline, not constrained baseline, in the
> manual, but the SPS NALs produced by the encoder start with:
> 00 00 00 01 67 42 40
> ^
> so that is profile_idc=66, constraint_set1_flag==1, constrained baseline
> indeed. I think this change is correct.
>
> > if (ctx->dev->devtype->product == CODA_HX4 ||
> > ctx->dev->devtype->product == CODA_7541) {
> > v4l2_ctrl_new_std_menu(&ctx->ctrls, &coda_ctrl_ops,
> > @@ -2417,7 +2417,7 @@ static void coda_decode_ctrls(struct coda_ctx *ctx)
> > ctx->h264_profile_ctrl = v4l2_ctrl_new_std_menu(&ctx->ctrls,
> > &coda_ctrl_ops, V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> > V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> > - ~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
> > + ~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) |
> > (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
> > (1 << V4L2_MPEG_VIDEO_H264_PROFILE_HIGH)),
> > V4L2_MPEG_VIDEO_H264_PROFILE_HIGH);
>
> I'm not sure about this one.
>
> regards
> Philipp
