[FFmpeg-devel] [PATCH 1/5] avcodec/cbs: add helper functions and macros to read and write signed values
Mark Thompson
sw at jkqxz.net
Wed Apr 17 02:27:39 EEST 2019
On 16/04/2019 23:54, James Almer wrote:
> On 4/16/2019 7:45 PM, Mark Thompson wrote:
>> On 15/04/2019 22:17, James Almer wrote:
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>> libavcodec/cbs.c | 79 +++++++++++++++++++++++++++++++++++++++
>>> libavcodec/cbs_internal.h | 20 +++++++++-
>>> 2 files changed, 98 insertions(+), 1 deletion(-)
>>
>> Looks like a sensible addition, some comments below.
>>
>>> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
>>> index c388be896b..726bd582f5 100644
>>> --- a/libavcodec/cbs.c
>>> +++ b/libavcodec/cbs.c
>>> @@ -504,6 +504,85 @@ int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc,
>>> return 0;
>>> }
>>>
>>> +int ff_cbs_read_signed(CodedBitstreamContext *ctx, GetBitContext *gbc,
>>> + int width, const char *name,
>>> + const int *subscripts, int32_t *write_to,
>>> + int32_t range_min, int32_t range_max)
>>> +{
>>> + int32_t value;
>>> + int position;
>>> +
>>> + av_assert0(width > 0 && width <= 32);
>>> +
>>> + if (get_bits_left(gbc) < width) {
>>> + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid value at "
>>> + "%s: bitstream ended.\n", name);
>>> + return AVERROR_INVALIDDATA;
>>> + }
>>> +
>>> + if (ctx->trace_enable)
>>> + position = get_bits_count(gbc);
>>> +
>>> + value = get_sbits_long(gbc, width);
>>> +
>>> + if (ctx->trace_enable) {
>>> + char bits[33];
>>> + int i;
>>> + for (i = 0; i < width; i++)
>>> + bits[i] = value & (1 << (width - i - 1)) ? '1' : '0';
>>
>> 1 << 31 is undefined behaviour for 32-bit int.
>>
>> The unsigned versions are careful to right-shift unsigned values to avoid overflow problems; a possible fix might be to strip the sign bit and then do the same thing.
>
> Would "1U << (width - i - 1)" be enough?
Probably? A negative value would be promoted to unsigned as positive 1111xxxx, which I think does the right thing.
>>> + bits[i] = 0;
>>> +
>>> + ff_cbs_trace_syntax_element(ctx, position, name, subscripts,
>>> + bits, value);
>>> + }
>>> +
>>> + if (value < range_min || value > range_max) {
>>> + av_log(ctx->log_ctx, AV_LOG_ERROR, "%s out of range: "
>>> + "%"PRId32", but must be in [%"PRId32",%"PRId32"].\n",
>>> + name, value, range_min, range_max);
>>> + return AVERROR_INVALIDDATA;
>>> + }
>>> +
>>> + *write_to = value;
>>> + return 0;
>>> +}
- Mark
More information about the ffmpeg-devel
mailing list