[FFmpeg-devel] [PATCH] avcodec/get_bits: consider bit_size of 0 an error

Ronald S. Bultje rsbultje at gmail.com
Mon Oct 28 19:13:50 CET 2013


Hi,

On Mon, Oct 28, 2013 at 1:35 PM, Paul B Mahol <onemda at gmail.com> wrote:

> On 10/28/13, Reimar Doeffinger <Reimar.Doeffinger at gmx.de> wrote:
> > On Mon, Oct 28, 2013 at 05:04:38PM +0000, Paul B Mahol wrote:
> >> On 10/28/13, Reimar Doeffinger <Reimar.Doeffinger at gmx.de> wrote:
> >> > On Mon, Oct 28, 2013 at 04:52:27PM +0000, Paul B Mahol wrote:
> >> >> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> >> >
> >> > Could you add the reason for this change to the commit message?
> >> > In theory being able to use 0 might work as an optimization in
> >> > some cases, so I don't think supporting it is as nonsense as it
> >> > might seem.
> >>
> >> What kind of optimization?
> >
> > Purely theoretical: You have a value A, depending on that following
> > value B is either not encoded, encoded with 2, 5 or 8 bits.
> > If get_bits supports 0 bits then you can just do something like
> > get_bits(... (int){0, 2, 5, 8}[A]);
> > if 0 is not allowed, you have to explicitly skip the get_bits for
> > A == 0.
> >
> >> If return value is never checked and bit_size is negative number next
> >> get_bits will happily crash.
> >
> > But negative number is already checked, your patch is about checking 0?!
>
> Yes, because here is what happens for 0 case:
>
> buffer is not set to NULL (for negative case this causes crash)
>
> get_bits may return 1 even if bit count is 0, and it will happily
> return it forever (until  whatever buffer points to is changed to 0)
>
> so something like
>
> while(get_bits1(gb)) {
> }
>
> loops forever.
>

That is a bug in the caller.

Ronald


More information about the ffmpeg-devel mailing list