[FFmpeg-devel] [PATCH 1/2] avcodec/rangecoder: Do not increase the pointer beyond the buffer

James Almer jamrial at gmail.com
Tue Aug 15 05:30:06 EEST 2017


On 8/14/2017 8:07 PM, Michael Niedermayer wrote:
> On Sun, Aug 13, 2017 at 07:18:11PM -0300, James Almer wrote:
>> On 8/13/2017 7:15 PM, Michael Niedermayer wrote:
>>> Fixes: undefined behavior
>>>
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavcodec/rangecoder.c | 1 +
>>>  libavcodec/rangecoder.h | 8 ++++++--
>>>  2 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/rangecoder.c b/libavcodec/rangecoder.c
>>> index 0bb79c880e..0d53bef076 100644
>>> --- a/libavcodec/rangecoder.c
>>> +++ b/libavcodec/rangecoder.c
>>> @@ -58,6 +58,7 @@ av_cold void ff_init_range_decoder(RangeCoder *c, const uint8_t *buf,
>>>  
>>>      c->low         = AV_RB16(c->bytestream);
>>>      c->bytestream += 2;
>>> +    c->overread    = 0;
>>>      if (c->low >= 0xFF00) {
>>>          c->low = 0xFF00;
>>>          c->bytestream_end = c->bytestream;
>>> diff --git a/libavcodec/rangecoder.h b/libavcodec/rangecoder.h
>>> index c3e81d0dcb..44af88b8f5 100644
>>> --- a/libavcodec/rangecoder.h
>>> +++ b/libavcodec/rangecoder.h
>>> @@ -42,6 +42,8 @@ typedef struct RangeCoder {
>>>      uint8_t *bytestream_start;
>>>      uint8_t *bytestream;
>>>      uint8_t *bytestream_end;
>>> +    int overread;
>>> +#define MAX_OVERREAD 2
>>>  } RangeCoder;
>>>  
>>>  void ff_init_range_encoder(RangeCoder *c, uint8_t *buf, int buf_size);
>>> @@ -106,9 +108,11 @@ static inline void refill(RangeCoder *c)
>>>      if (c->range < 0x100) {
>>>          c->range <<= 8;
>>>          c->low   <<= 8;
>>> -        if (c->bytestream < c->bytestream_end)
>>> +        if (c->bytestream < c->bytestream_end) {
>>>              c->low += c->bytestream[0];
>>> -        c->bytestream++;
>>> +            c->bytestream++;
>>> +        } else
>>> +            c->overread ++;
>>>      }
>>>  }
>>
>> Wouldn't it be better to port this to the bytestream2 reading api?
> 
> this is speed relevant code, i am not sure bytestream2 wouldnt add
> overhead. In fact i wasnt entirely sure keeping track of the overread
> bytes is a great idea. But i didnt see an easy way to avoid that.

Probably no overhead, since renorm_encoder() can use the unchecked
reader much like it's doing it now.

In any case, the buffer may be both read and written to, which means
you'd have to split things in two to use both the put and get
bytestream2 APIs, so it may not be worth doing (and probably the reason
why it's not already been done in the first place).


More information about the ffmpeg-devel mailing list