[FFmpeg-devel] [PATCH 09/11] cbs_mpeg2: Fix parsing of slice headers

[Andreas Rheinhardt]

1. The extra information in slice headers was parsed incorrectly:
In the first reading pass to derive the length of the extra information,
one should look at bits n, n + 9, n + 18, ... and check whether they
equal one (further extra information) or zero (end of extra information),
but instead bits n, n + 8, n + 16, ... were inspected. The second pass
of reading (where the length is already known and the bytes between the
length-determining bits are copied into a buffer) did not record what
was in bits n, n + 9, n + 18, ..., presuming they equal one. And during
writing, the bytes in the buffer are interleaved with set bits and
written. This means that if the detected length of the extra information
was greater than the real length, the output was corrupted. Fortunately
no sample is known that made use of this mechanism: The extra information
in slices is still marked as reserved in the specifications. cbs_mpeg2
is now ready in case this changes.

2. Furthermore, the buffer is now padded and slightly different, but
very similar code for reading resp. writing has been replaced by code
used for both. This was made possible by a new macro, the equivalent
to cbs_h2645's fixed().

3. These changes also made it possible to remove the extra_bit_slice
element from the MPEG2RawSliceHeader structure. Said element was always
zero except when the detected length of the extra information was less
than the real length.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
 libavcodec/cbs_mpeg2.c                 | 18 +++++++-----------
 libavcodec/cbs_mpeg2.h                 |  2 --
 libavcodec/cbs_mpeg2_syntax_template.c | 16 +++++-----------
 3 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 0110dc0e03..97425aa706 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -51,6 +51,13 @@
 #define sis(width, name, subs, ...) \
         xsi(width, name, current->name, subs, __VA_ARGS__)
 
+#define marker_bit() \
+        bit(marker_bit, 1)
+#define bit(name, value) do { \
+        av_unused uint32_t bit = value; \
+        xui(1, name, bit, value, value, 0); \
+    } while (0)
+
 
 #define READ
 #define READWRITE read
@@ -73,11 +80,6 @@
         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)); \
-    } while (0)
-
 #define nextbits(width, compare, var) \
     (get_bits_left(rw) >= width && \
      (var = show_bits(rw, width)) == (compare))
@@ -93,7 +95,6 @@
 #undef RWContext
 #undef xui
 #undef xsi
-#undef marker_bit
 #undef nextbits
 #undef infer
 
@@ -115,10 +116,6 @@
                                   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)
-
 #define nextbits(width, compare, var) (var)
 
 #define infer(name, value) do { \
@@ -137,7 +134,6 @@
 #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 11f93b9df8..db5ebc56ea 100644
--- a/libavcodec/cbs_mpeg2.h
+++ b/libavcodec/cbs_mpeg2.h
@@ -194,8 +194,6 @@ typedef struct MPEG2RawSliceHeader {
     uint8_t slice_picture_id_enable;
     uint8_t slice_picture_id;
 
-    uint8_t extra_bit_slice;
-
     size_t extra_information_length;
     uint8_t *extra_information;
     AVBufferRef *extra_information_ref;
diff --git a/libavcodec/cbs_mpeg2_syntax_template.c b/libavcodec/cbs_mpeg2_syntax_template.c
index d9ef480f39..1f2d2497c3 100644
--- a/libavcodec/cbs_mpeg2_syntax_template.c
+++ b/libavcodec/cbs_mpeg2_syntax_template.c
@@ -377,31 +377,25 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw,
             uint8_t bit;
             start = *rw;
             for (k = 0; nextbits(1, 1, bit); k++)
-                skip_bits(rw, 8);
+                skip_bits(rw, 1 + 8);
             current->extra_information_length = k;
             if (k > 0) {
                 *rw = start;
                 current->extra_information_ref =
-                    av_buffer_alloc(current->extra_information_length);
+                    av_buffer_allocz(k + AV_INPUT_BUFFER_PADDING_SIZE);
                 if (!current->extra_information_ref)
                     return AVERROR(ENOMEM);
                 current->extra_information = current->extra_information_ref->data;
-                for (k = 0; k < current->extra_information_length; k++) {
-                    xui(1, extra_bit_slice, bit, 1, 1, 0);
-                    xui(8, extra_information_slice[k],
-                        current->extra_information[k], 0, 255, 1, k);
-                }
             }
-#else
+#endif
             for (k = 0; k < current->extra_information_length; k++) {
-                xui(1, extra_bit_slice, 1, 1, 1, 0);
+                bit(extra_bit_slice, 1);
                 xui(8, extra_information_slice[k],
                     current->extra_information[k], 0, 255, 1, k);
             }
-#endif
         }
     }
-    ui(1, extra_bit_slice);
+    bit(extra_bit_slice, 0);
 
     return 0;
 }
-- 
2.21.0