[FFmpeg-devel] [PATCH] avcodec/put_bits: Always check buffer end before writing

[Michael Niedermayer]

On Fri, Jan 01, 2016 at 08:59:23AM +0000, Paul B Mahol wrote:
> On 1/1/16, Michael Niedermayer <michaelni at gmx.at> wrote:
> > From: Michael Niedermayer <michael at niedermayer.cc>
> >
> > This causes a overall slowdown of 0.1 % (tested with mpeg4 single thread
> > encoding of matrixbench at QP=3)
> >
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/put_bits.h |   16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/put_bits.h b/libavcodec/put_bits.h
> > index 5b1bc8b..69a3049 100644
> > --- a/libavcodec/put_bits.h
> > +++ b/libavcodec/put_bits.h
> > @@ -163,9 +163,11 @@ static inline void put_bits(PutBitContext *s, int n,
> > unsigned int value)
> >  #ifdef BITSTREAM_WRITER_LE
> >      bit_buf |= value << (32 - bit_left);
> >      if (n >= bit_left) {
> > -        av_assert2(s->buf_ptr+3<s->buf_end);
> > -        AV_WL32(s->buf_ptr, bit_buf);
> > -        s->buf_ptr += 4;
> > +        if (3 < s->buf_end - s->buf_ptr) {
> > +            AV_WL32(s->buf_ptr, bit_buf);
> > +            s->buf_ptr += 4;
> > +        } else
> > +            av_assert0(0);
> >          bit_buf     = value >> bit_left;
> >          bit_left   += 32;
> >      }
> > @@ -177,9 +179,11 @@ static inline void put_bits(PutBitContext *s, int n,
> > unsigned int value)
> >      } else {
> >          bit_buf   <<= bit_left;
> >          bit_buf    |= value >> (n - bit_left);
> > -        av_assert2(s->buf_ptr+3<s->buf_end);
> > -        AV_WB32(s->buf_ptr, bit_buf);
> > -        s->buf_ptr += 4;
> > +        if (3 < s->buf_end - s->buf_ptr) {
> > +            AV_WB32(s->buf_ptr, bit_buf);
> > +            s->buf_ptr += 4;
> > +        } else
> > +            av_assert0(0);
> >          bit_left   += 32 - n;
> >          bit_buf     = value;
> >      }
> 
> If this can happen, using assert0 is bad idea.

Its a internal bug if it happens, it should not happen ...


> If this should not happen add if under assert2.

I tried doing the assert0 without the if() yestarday before sending
the patch but it doubbled the speedloss

Using assert2 instead of assert0 could be done but then there is
no indication of this fault with default configuration.

so i ended up with this a bit funny looking variant as it was the
fastest in my testing

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160101/a557e080/attachment.sig>