On 23/04/2019 23:55, Andreas Rheinhardt wrote:
> horizontal/vertical_size_value (containing the twelve least significant
> bits of the frame size) mustn't be zero according to the specifications;
> and the value 0 is forbidden for the colour_description elements.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
> ---
> The actual function calls after macro expansion are the same as in the
> earlier versions with the exception of the extra_bit_slice calls.
> libavcodec/cbs_mpeg2.c | 14 ++++++++------
> libavcodec/cbs_mpeg2_syntax_template.c | 20 ++++++++++----------
> 2 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index cdde68ea38..fc21745a51 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -41,20 +41,22 @@
> #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs,
> __VA_ARGS__ }) : NULL)
>
> #define ui(width, name) \
> - xui(width, name, current->name, 0)
> + xui(width, name, current->name, 0, MAX_UINT_BITS(width), 0)
> +#define uir(width, name, range_min, range_max) \
> + xui(width, name, current->name, range_min, range_max, 0)
> #define uis(width, name, subs, ...) \
> - xui(width, name, current->name, subs, __VA_ARGS__)
> + xui(width, name, current->name, 0, MAX_UINT_BITS(width), subs,
> __VA_ARGS__)
>
>
> #define READ
> #define READWRITE read
> #define RWContext GetBitContext
>
> -#define xui(width, name, var, subs, ...) do { \
> +#define xui(width, name, var, range_min, range_max, subs, ...) do { \
> uint32_t value = 0; \
> CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \
> SUBSCRIPTS(subs, __VA_ARGS__), \
> - &value, 0, (1 << width) - 1)); \
> + &value, range_min, range_max)); \
> var = value; \
> } while (0)
>
> @@ -81,10 +83,10 @@
> #define READWRITE write
> #define RWContext PutBitContext
>
> -#define xui(width, name, var, subs, ...) do { \
> +#define xui(width, name, var, range_min, range_max, subs, ...) do { \
> CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \
> SUBSCRIPTS(subs, __VA_ARGS__), \
> - var, 0, (1 << width) - 1)); \
> + var, range_min, range_max)); \
> } while (0)
>
> #define marker_bit() do { \
> diff --git a/libavcodec/cbs_mpeg2_syntax_template.c
> b/libavcodec/cbs_mpeg2_syntax_template.c
> index 10aaea7734..27dcaad316 100644
> --- a/libavcodec/cbs_mpeg2_syntax_template.c
> +++ b/libavcodec/cbs_mpeg2_syntax_template.c
> @@ -26,8 +26,8 @@ static int FUNC(sequence_header)(CodedBitstreamContext
> *ctx, RWContext *rw,
>
> ui(8, sequence_header_code);
>
> - ui(12, horizontal_size_value);
> - ui(12, vertical_size_value);
> + uir(12, horizontal_size_value, 1, 4095);
> + uir(12, vertical_size_value, 1, 4095);
>
> mpeg2->horizontal_size = current->horizontal_size_value;
> mpeg2->vertical_size = current->vertical_size_value;
> @@ -79,7 +79,7 @@ static int FUNC(user_data)(CodedBitstreamContext *ctx,
> RWContext *rw,
> #endif
>
> for (k = 0; k < current->user_data_length; k++)
> - xui(8, user_data, current->user_data[k], 0);
> + xui(8, user_data, current->user_data[k], 0, 255, 0);
This could include the subscript while you're changing it.
uis(8, user_data[k], 1, k);
>
> return 0;
> }
> @@ -125,9 +125,9 @@ static int
> FUNC(sequence_display_extension)(CodedBitstreamContext *ctx, RWContex
>
> ui(1, colour_description);
> if (current->colour_description) {
> - ui(8, colour_primaries);
> - ui(8, transfer_characteristics);
> - ui(8, matrix_coefficients);
> + uir(8, colour_primaries, 1, 255);
> + uir(8, transfer_characteristics, 1, 255);
> + uir(8, matrix_coefficients, 1, 255);
I'm a bit unsure about this one. While the zero value is indeed banned by the
standard, it isn't uncommon to find (at least in H.264) streams with zeroes in
these fields. (The libavcodec encoder for MPEG-2 is happy to write such a
stream, too.) Start code aliasing is presumably the reason for the standard
disallowing zeroes, but they probably won't actually be a problem unless
display_horizontal_size happens to have a specific range of bad values (in
which case we already fail in a different way).
> }
>
> ui(14, display_horizontal_size);
> @@ -366,16 +366,16 @@ static int FUNC(slice_header)(CodedBitstreamContext
> *ctx, RWContext *rw,
> if (!current->extra_information)
> return AVERROR(ENOMEM);
> for (k = 0; k < current->extra_information_length; k++) {
> - xui(1, extra_bit_slice, bit, 0);
> + xui(1, extra_bit_slice, bit, 1, 1, 0);
> xui(8, extra_information_slice[k],
> - current->extra_information[k], 1, k);
> + current->extra_information[k], 0, 255, 1, k);
> }
> }
> #else
> for (k = 0; k < current->extra_information_length; k++) {
> - xui(1, extra_bit_slice, 1, 0);
> + xui(1, extra_bit_slice, 1, 1, 1, 0);
> xui(8, extra_information_slice[k],
> - current->extra_information[k], 1, k);
> + current->extra_information[k], 0, 255, 1, k);
> }
> #endif
> }
>
Everything else about this series LGTM.
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".
