[FFmpeg-devel] [PATCH 04/11] cbs_mpeg2: Fix storage type for frame_centre_*_offset

[James Almer]

On 5/21/2019 10:04 PM, Andreas Rheinhardt wrote:
> The frame_centre_horizontal/vertical_offset values contained in picture
> display extensions are actually signed values (i.e. it is possible to
> indicate that the display device should add black bars/pillars).
> 
> The files sony-ct3.bs and tcela-6.bits (which are both used in fate
> tests for mpeg2_metadata) contain picture display extensions; the former
> even contains a negative frame_centre_vertical_offset. Fortunately, the
> old code did not damage the picture display extensions when one did a
> cycle of reading and writing. For the same reason the fate tests needn't
> be updated either.
> 
> Furthermore these fields now use the trace output for matrices.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavcodec/cbs_mpeg2.c                 | 20 ++++++++++++++++++++
>  libavcodec/cbs_mpeg2.h                 |  4 ++--
>  libavcodec/cbs_mpeg2_syntax_template.c |  4 ++--
>  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 437eac88a3..de0ad8a043 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -48,6 +48,8 @@
>          xui(width, name, current->name, 0, MAX_UINT_BITS(width), subs, __VA_ARGS__)
>  #define uirs(width, name, subs, ...) \
>          xui(width, name, current->name, 1, MAX_UINT_BITS(width), subs, __VA_ARGS__)
> +#define sis(width, name, subs, ...) \
> +        xsi(width, name, current->name, subs, __VA_ARGS__)

For consistency's sake, add the two range parameters to xsi much like
you added them to xui in patch 2, instead of hardcoding a range inside
the macro.

>  
>  
>  #define READ
> @@ -62,6 +64,15 @@
>          var = value; \
>      } while (0)
>  
> +#define xsi(width, name, var, subs, ...) do { \
> +        int32_t value; \
> +        CHECK(ff_cbs_read_signed(ctx, rw, width, #name, \
> +                                 SUBSCRIPTS(subs, __VA_ARGS__), &value, \
> +                                 MIN_INT_BITS(width), \
> +                                 MAX_INT_BITS(width))); \
> +        var = value; \
> +    } while (0)
> +
>  #define marker_bit() do { \
>          av_unused uint32_t one; \
>          CHECK(ff_cbs_read_unsigned(ctx, rw, 1, "marker_bit", NULL, &one, 1, 1)); \
> @@ -81,6 +92,7 @@
>  #undef READWRITE
>  #undef RWContext
>  #undef xui
> +#undef xsi
>  #undef marker_bit
>  #undef nextbits
>  #undef infer
> @@ -96,6 +108,13 @@
>                                      var, range_min, range_max)); \
>      } while (0)
>  
> +#define xsi(width, name, var, subs, ...) do { \
> +        CHECK(ff_cbs_write_signed(ctx, rw, width, #name, \
> +                                  SUBSCRIPTS(subs, __VA_ARGS__), var, \
> +                                  MIN_INT_BITS(width), \
> +                                  MAX_INT_BITS(width))); \
> +    } while (0)
> +
>  #define marker_bit() do { \
>          CHECK(ff_cbs_write_unsigned(ctx, rw, 1, "marker_bit", NULL, 1, 1, 1)); \
>      } while (0)
> @@ -117,6 +136,7 @@
>  #undef READWRITE
>  #undef RWContext
>  #undef xui
> +#undef xsi
>  #undef marker_bit
>  #undef nextbits
>  #undef infer
> diff --git a/libavcodec/cbs_mpeg2.h b/libavcodec/cbs_mpeg2.h
> index 7565695acb..11f93b9df8 100644
> --- a/libavcodec/cbs_mpeg2.h
> +++ b/libavcodec/cbs_mpeg2.h
> @@ -164,8 +164,8 @@ typedef struct MPEG2RawQuantMatrixExtension {
>  } MPEG2RawQuantMatrixExtension;
>  
>  typedef struct MPEG2RawPictureDisplayExtension {
> -    uint16_t frame_centre_horizontal_offset[3];
> -    uint16_t frame_centre_vertical_offset[3];
> +    int16_t frame_centre_horizontal_offset[3];
> +    int16_t frame_centre_vertical_offset[3];
>  } MPEG2RawPictureDisplayExtension;
>  
>  typedef struct MPEG2RawExtensionData {
> diff --git a/libavcodec/cbs_mpeg2_syntax_template.c b/libavcodec/cbs_mpeg2_syntax_template.c
> index 87db0ad039..e34fa9495f 100644
> --- a/libavcodec/cbs_mpeg2_syntax_template.c
> +++ b/libavcodec/cbs_mpeg2_syntax_template.c
> @@ -303,9 +303,9 @@ static int FUNC(picture_display_extension)(CodedBitstreamContext *ctx, RWContext
>      HEADER("Picture Display Extension");
>  
>      for (i = 0; i < mpeg2->number_of_frame_centre_offsets; i++) {
> -        ui(16, frame_centre_horizontal_offset[i]);
> +        sis(16, frame_centre_horizontal_offset[i], 1, i);
>          marker_bit();
> -        ui(16, frame_centre_vertical_offset[i]);
> +        sis(16, frame_centre_vertical_offset[i],   1, i);
>          marker_bit();
>      }
>  
>