[FFmpeg-devel] cbs_mpeg2: Fix storage type for frame_centre_*_offset

Submitted by Andreas Rheinhardt on May 22, 2019, 9:53 a.m.

Details

Message ID 20190522095343.47094-1-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt May 22, 2019, 9:53 a.m.
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@gmail.com>
---
 libavcodec/cbs_mpeg2.c                 | 18 ++++++++++++++++++
 libavcodec/cbs_mpeg2.h                 |  4 ++--
 libavcodec/cbs_mpeg2_syntax_template.c |  4 ++--
 3 files changed, 22 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 437eac88a3..ba34ace9b4 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, MIN_INT_BITS(width), MAX_INT_BITS(width), subs, __VA_ARGS__)
 
 
 #define READ
@@ -62,6 +64,14 @@ 
         var = value; \
     } while (0)
 
+#define xsi(width, name, var, range_min, range_max, subs, ...) do { \
+        int32_t value; \
+        CHECK(ff_cbs_read_signed(ctx, rw, width, #name, \
+                                 SUBSCRIPTS(subs, __VA_ARGS__), &value, \
+                                 range_min, range_max)); \
+        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 +91,7 @@ 
 #undef READWRITE
 #undef RWContext
 #undef xui
+#undef xsi
 #undef marker_bit
 #undef nextbits
 #undef infer
@@ -96,6 +107,12 @@ 
                                     var, range_min, range_max)); \
     } while (0)
 
+#define xsi(width, name, var, range_min, range_max, subs, ...) do { \
+        CHECK(ff_cbs_write_signed(ctx, rw, width, #name, \
+                                  SUBSCRIPTS(subs, __VA_ARGS__), var, \
+                                  range_min, range_max)); \
+    } while (0)
+
 #define marker_bit() do { \
         CHECK(ff_cbs_write_unsigned(ctx, rw, 1, "marker_bit", NULL, 1, 1, 1)); \
     } while (0)
@@ -117,6 +134,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();
     }