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.
> + 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;
> +}
> +
> +int ff_cbs_write_signed(CodedBitstreamContext *ctx, PutBitContext *pbc,
> + int width, const char *name,
> + const int *subscripts, int32_t value,
> + int32_t range_min, int32_t range_max)
> +{
> + av_assert0(width > 0 && width <= 32);
> +
> + 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;
> + }
> +
> + if (put_bits_left(pbc) < width)
> + return AVERROR(ENOSPC);
> +
> + if (ctx->trace_enable) {
> + char bits[33];
> + int i;
> + for (i = 0; i < width; i++)
> + bits[i] = value & (1 << (width - i - 1)) ? '1' : '0';
As above.
> + bits[i] = 0;
> +
> + ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc),
> + name, subscripts, bits, value);
> + }
> +
> + if (width < 32)
> + put_sbits(pbc, width, value);
> + else
> + put_bits32(pbc, value);
> +
> + return 0;
> +}
> +
>
> int ff_cbs_alloc_unit_content(CodedBitstreamContext *ctx,
> CodedBitstreamUnit *unit,
> diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
> index 53f2e5d187..6ab85679dd 100644
> --- a/libavcodec/cbs_internal.h
> +++ b/libavcodec/cbs_internal.h
> @@ -81,10 +81,28 @@ int ff_cbs_write_unsigned(CodedBitstreamContext *ctx,
> PutBitContext *pbc,
> const int *subscripts, uint32_t value,
> uint32_t range_min, uint32_t range_max);
>
> -// The largest value representable in N bits, suitable for use as
> +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);
> +
> +int ff_cbs_write_signed(CodedBitstreamContext *ctx, PutBitContext *pbc,
> + int width, const char *name,
> + const int *subscripts, int32_t value,
> + int32_t range_min, int32_t range_max);
> +
> +// The largest unsigned value representable in N bits, suitable for use as
> // range_max in the above functions.
> #define MAX_UINT_BITS(length) ((UINT64_C(1) << (length)) - 1)
>
> +// The largest signed value representable in N bits, suitable for use as
> +// range_max in the above functions.
> +#define MAX_INT_BITS(length) ((INT64_C(1) << (length - 1)) - 1)
Not so good for, say, MAX_INT_BITS(a ? b : c).
> +
> +// The smallest signed value representable in N bits, suitable for use as
> +// range_min in the above functions.
> +#define MIN_INT_BITS(length) (-(INT64_C(1) << (length - 1)))
Same here.
> +
>
> extern const CodedBitstreamType ff_cbs_type_av1;
> extern const CodedBitstreamType ff_cbs_type_h264;
>
Thanks,
- 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".
