[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