On Mon, 2018-07-02 at 10:53 +0200, Hans Verkuil wrote:
> On 28/06/18 18:47, Philipp Zabel wrote:
> > The CODA firmware does not set the VUI frame cropping fields to properly
> > describe coded h.264 streams with frame sizes that are not a multiple of
> > the macroblock size.
> > This adds RBSP parsing code and a SPS fixup routine to manually replace
> > the cropping information in the headers produced by the firmware with
> > the correct values.
> >
> > Signed-off-by: Philipp Zabel <p.za...@pengutronix.de>
> > ---
> > drivers/media/platform/coda/coda-bit.c | 11 +
> > drivers/media/platform/coda/coda-h264.c | 302 ++++++++++++++++++++++++
> > drivers/media/platform/coda/coda.h | 2 +
> > 3 files changed, 315 insertions(+)
> >
> > diff --git a/drivers/media/platform/coda/coda-bit.c
> > b/drivers/media/platform/coda/coda-bit.c
> > index 94ba3cc3bf14..d6380a10fa2c 100644
> > --- a/drivers/media/platform/coda/coda-bit.c
> > +++ b/drivers/media/platform/coda/coda-bit.c
> > @@ -1197,6 +1197,17 @@ static int coda_start_encoding(struct coda_ctx *ctx)
> > if (ret < 0)
> > goto out;
> >
> > + if ((q_data_src->rect.width % 16) ||
> > + (q_data_src->rect.height % 16)) {
> > + ret = coda_sps_fixup(ctx, q_data_src->rect.width,
> > + q_data_src->rect.height,
> > + &ctx->vpu_header[0][0],
> > + &ctx->vpu_header_size[0],
> > + sizeof(ctx->vpu_header[0]));
>
> You need to add a comment here why this is needed.
[...]
> > +int coda_sps_fixup(struct coda_ctx *ctx, int width, int height, char *buf,
> > + int *size, int max_size)
>
> Same here: why it is needed and what does it do.
Thank you, I'll send a v2 with this fixed.
This function rewrites the h.264 SPS header to set the crop_right and
crop_bottom fields correctly, as these are always set to 0 by the
CODA7541 firmware.
Setting the crop_* fields is important for frame sizes that are not
multiples of the macroblock size: those fields are used together with
the pic_width_in_mbs_minus1 and pic_height_in_map_units_minus1 fields
to determine the visible frame width and height.
regards
Philipp
