[FFmpeg-devel] [PATCH 2/2] avcodec/cbs_av1: fix parsing signed integer values

Mark Thompson sw at jkqxz.net
Thu Nov 15 01:04:10 EET 2018


On 14/11/18 22:39, James Almer wrote:
> On 11/14/2018 7:24 PM, Mark Thompson wrote:
>> On 11/11/18 02:24, James Almer wrote:
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>> See https://0x0.st/sljR.webm
>>>
>>>  libavcodec/cbs_av1.c | 30 +++++++++---------------------
>>>  1 file changed, 9 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
>>> index ff32a6fca5..215f9384e8 100644
>>> --- a/libavcodec/cbs_av1.c
>>> +++ b/libavcodec/cbs_av1.c
>>> @@ -189,30 +189,26 @@ static int cbs_av1_read_su(CodedBitstreamContext *ctx, GetBitContext *gbc,
>>>                             int width, const char *name,
>>>                             const int *subscripts, int32_t *write_to)
>>>  {
>>> -    uint32_t magnitude;
>>> -    int position, sign;
>>> +    int position;
>>>      int32_t value;
>>>  
>>>      if (ctx->trace_enable)
>>>          position = get_bits_count(gbc);
>>>  
>>> -    if (get_bits_left(gbc) < width + 1) {
>>> +    if (get_bits_left(gbc) < width) {
>>>          av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid signed value at "
>>>                 "%s: bitstream ended.\n", name);
>>>          return AVERROR_INVALIDDATA;
>>>      }
>>>  
>>> -    magnitude = get_bits(gbc, width);
>>> -    sign      = get_bits1(gbc);
>>> -    value     = sign ? -(int32_t)magnitude : magnitude;
>>> +    value = get_sbits(gbc, width);
>>>  
>>>      if (ctx->trace_enable) {
>>>          char bits[33];
>>>          int i;
>>>          for (i = 0; i < width; i++)
>>> -            bits[i] = magnitude >> (width - i - 1) & 1 ? '1' : '0';
>>> -        bits[i] = sign ? '1' : '0';
>>> -        bits[i + 1] = 0;
>>> +            bits[i] = value >> (width - i - 1) & 1 ? '1' : '0';
>>
>> Not sure if we care, but right-shifting a negative value is implementation-defined.
> 
> Is casting value to unsigned enough, or should i use something like
> zero_extend()?

I think using left-shift instead is safe:

value & 1 << (...) ? '1' : '0'


More information about the ffmpeg-devel mailing list