Re: [FFmpeg-devel] [PATCH v2] libavcodec/cbs_av1: Add size check before parse obu

["Chen, Wenbin" <wenbin.chen-at-intel.com@ffmpeg.org>]

> On 29/03/2022 09:29, Wenbin Chen wrote:
> > cbs_av1_write_unit() check pbc size after parsing obu frame, and return
> > AVERROR(ENOSPC) if pbc is small. pbc will be reallocated and this obu
> > frame will be parsed again, but this may cause error because
> > CodedBitstreamAV1Context has already been updated, for example
> > ref_order_hint is updated and will not match the same obu frame. Now
> size
> > check is added before parsing obu frame to avoid this error.
> >
> > Signed-off-by: Wenbin Chen <wenbin.chen@intel.com>
> > ---
> >   libavcodec/cbs_av1.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> > index 1229480567..29e7bc16df 100644
> > --- a/libavcodec/cbs_av1.c
> > +++ b/libavcodec/cbs_av1.c
> > @@ -1075,6 +1075,9 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> >           put_bits32(pbc, 0);
> >       }
> >
> > +    if (8 * (unit->data_size + obu->obu_size) > put_bits_left(pbc))
> > +        return AVERROR(ENOSPC);
> 
> unit->data_size is not usefully set when we are writing here (it might be the
> size of the old bitstream in editing cases, or it might just be zero).

Thank you for pointing this out. If data_size is unset this check wouldn't work and
the problem still occurs. I will try to find a better way to fix this.

> 
> > +
> >       td = NULL;
> >       start_pos = put_bits_count(pbc);
> >
> > @@ -1196,9 +1199,6 @@ static int
> cbs_av1_write_obu(CodedBitstreamContext *ctx,
> >       flush_put_bits(pbc);
> >       av_assert0(data_pos <= start_pos);
> >
> > -    if (8 * obu->obu_size > put_bits_left(pbc))
> > -        return AVERROR(ENOSPC);
> > -
> >       if (obu->obu_size > 0) {
> >           memmove(pbc->buf + data_pos,
> >                   pbc->buf + start_pos, header_size);
> 
> So, this doesn't work?  The header hasn't been written that point, so you
> don't know if there is enough space for both the OBU header and the OBU
> data.
> 
> Having the check in both places would be fine (the newly-added one being a
> way to bail early when there definitely isn't enough space), but that wouldn't
> do what you want.

Ok, I will keep the both places in my next patch if I still fix issue in this way. 

> 
> I'm not sure what the right answer is here.  Do we need some way to unwind
> the written header?  The initial buffer size is 1MB and gets doubled each time,
> so this is not going to be hit very often.

Unwinding header is an alternative way. I will check If it is possible.

This problem is rare. The problem occurs when I frame below buffer size but one P/B
frame in the gop is greater than buffer size.

Thanks
Wenbin

> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".