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 <jamr...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".