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.
>
> > 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.
Indeed, the decoder does support ASO/FMO, assuming the extra buffer
space is allocated as per the documentation (says that slice_save_size
should be the max_width * max_height * 3 / 4). Did you have that
implemented ? If not, I'd keep that patch, but add a comment to explain
that it can be enabled later.
>
> regards
> Philipp
Attachment:
signature.asc
Description: This is a digitally signed message part
