On 11/14/2018 7:24 PM, Mark Thompson wrote:
> On 11/11/18 02:24, James Almer wrote:
>> Signed-off-by: James Almer <jamr...@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()?
>
>> + bits[i] = 0;
>>
>> ff_cbs_trace_syntax_element(ctx, position,
>> name, subscripts, bits, value);
>> @@ -226,29 +222,21 @@ static int cbs_av1_write_su(CodedBitstreamContext
>> *ctx, PutBitContext *pbc,
>> int width, const char *name,
>> const int *subscripts, int32_t value)
>> {
>> - uint32_t magnitude;
>> - int sign;
>> -
>> - if (put_bits_left(pbc) < width + 1)
>> + if (put_bits_left(pbc) < width)
>> return AVERROR(ENOSPC);
>>
>> - sign = value < 0;
>> - magnitude = sign ? -value : value;
>> -
>> 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';
>
> And here.
>
>> + bits[i] = 0;
>>
>> ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
>> name, subscripts, bits, value);
>> }
>>
>> - put_bits(pbc, width, magnitude);
>> - put_bits(pbc, 1, sign);
>> + put_sbits(pbc, width, value);
>>
>> return 0;
>> }
>>
>
> Otherwise fine. No idea where the version with the sign bit at the other end
> came from; oh well.
From a lack of samples to test and confirm that the code works :p
>
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
