diff mbox series

[FFmpeg-devel,v3,8/8] avpriv_find_start_code(): make start_code output only

Message ID 20220209032854.565698-9-scott.the.elm@gmail.com
State New
Headers show
Series rewrite avpriv_find_start_code() for clarity | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Scott Theisen Feb. 9, 2022, 3:28 a.m. UTC
The input/output functionality was used by only
(ff_)mpeg1_find_frame_end().

If the state/start_code input is a local variable and only one buffer is used,
then no history is needed.

In loops and inline functions: if ignoring history, don't initialize start_code,
so it isn't reset twice each time.

There is a slight functional change:
00 00 01 00 01 XX no longer incorrectly returns a start code at
offset 7 that overlaps the start code at offset 4 if the start_code
input is not modified between the two calls.
---
 libavcodec/cbs_mpeg2.c                |  5 ----
 libavcodec/h264_parser.c              |  2 +-
 libavcodec/mpeg12.c                   | 41 +++++++++++++++++++++++++-
 libavcodec/mpeg4_unpack_bframes_bsf.c |  1 -
 libavcodec/mpegvideo_parser.c         | 42 +++++++++++++++++++++++++--
 libavcodec/startcode.h                | 14 +++------
 libavcodec/utils.c                    | 16 ++++------
 libavcodec/vc1_common.h               |  2 +-
 libavformat/rtpenc_mpv.c              |  2 +-
 9 files changed, 92 insertions(+), 33 deletions(-)
diff mbox series

Patch

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index f0a2265938..870119bab0 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -160,11 +160,6 @@  static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         const uint8_t *end;
         size_t unit_size;
 
-        // Reset start_code to ensure that avpriv_find_start_code()
-        // really reads a new start code and does not reuse the old
-        // start code in any way (as e.g. happens when there is a
-        // Sequence End unit at the very end of a packet).
-        start_code = UINT32_MAX;
         end = avpriv_find_start_code(start--, frag->data + frag->data_size,
                                      &start_code);
 
diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 50810f1789..b67830d40e 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -69,7 +69,7 @@  typedef struct H264ParseContext {
 static int find_start_code(const uint8_t *buf, int buf_size,
                                   int buf_index, int next_avc)
 {
-    uint32_t state = -1;
+    uint32_t state;
 
     buf_index = avpriv_find_start_code(buf + buf_index, buf + next_avc + 1, &state) - buf - 1;
 
diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index 85f4f432bd..9639423152 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -165,6 +165,45 @@  av_cold void ff_mpeg12_init_vlcs(void)
 }
 
 #if FF_API_FLAG_TRUNCATED
+/**
+ * By preserving the <b>@p start_code</b> value between subsequent calls, the caller can
+ * detect start codes across buffer boundaries.
+ *
+ * @param[in,out] start_code A pointer to a mutable @c uint32_t.<br>
+ *          As input: For no history preset to <b>@c ~0 </b>, otherwise preset to the last
+ *                    returned start code to enable detecting start codes across
+ *                    buffer boundaries.<br>
+ *          On output: Set to the found start code if it exists or an invalid
+ *                     start code (the 4 bytes prior to the returned value,
+ *                     using the input history if @f$ end - p < 4 @f$).
+ *
+ * @sa avpriv_find_start_code()
+ */
+static const uint8_t *find_start_code_truncated(const uint8_t *av_restrict p,
+                                      const uint8_t * const end,
+                                      uint32_t * const av_restrict start_code)
+{
+    av_assert0(p <= end);
+    if (p >= end)
+        return end;
+
+    if (*start_code == 0x100)
+        *start_code = ~0;
+    // invalidate byte 0 so overlapping start codes are not erroneously detected
+
+    // read up to the first three bytes in p to enable reading a start code across
+    // two (to four) buffers
+    for (int i = 0; i < 3; i++) {
+        *start_code <<= 8;
+        *start_code += *p;
+        p++;
+        if (start_code_is_valid(*start_code) || p == end)
+            return p;
+    }
+    // buffer length is at least 4
+    return avpriv_find_start_code(p - 3, end, start_code);
+}
+
 /**
  * Find the end of the current frame in the bitstream.
  * @return the position of the first byte of the next frame, or -1
@@ -199,7 +238,7 @@  int ff_mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size,
             }
             state++;
         } else {
-            i = avpriv_find_start_code(buf + i, buf + buf_size, &state) - buf - 1;
+            i = find_start_code_truncated(buf + i, buf + buf_size, &state) - buf - 1;
             if (pc->frame_start_found == 0 && state >= SLICE_MIN_START_CODE && state <= SLICE_MAX_START_CODE) {
                 i++;
                 pc->frame_start_found = 4;
diff --git a/libavcodec/mpeg4_unpack_bframes_bsf.c b/libavcodec/mpeg4_unpack_bframes_bsf.c
index ae2c129d88..1030dd60aa 100644
--- a/libavcodec/mpeg4_unpack_bframes_bsf.c
+++ b/libavcodec/mpeg4_unpack_bframes_bsf.c
@@ -36,7 +36,6 @@  static void scan_buffer(const uint8_t *buf, int buf_size,
     const uint8_t *end = buf + buf_size, *pos = buf;
 
     while (pos < end) {
-        startcode = -1;
         pos = avpriv_find_start_code(pos, end, &startcode);
 
         if (startcode == USER_DATA_STARTCODE && pos_p) {
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index 82fdd1a1bb..0c42c6397a 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -33,6 +33,45 @@  struct MpvParseContext {
 };
 
 #if !FF_API_FLAG_TRUNCATED
+/**
+ * By preserving the <b>@p start_code</b> value between subsequent calls, the caller can
+ * detect start codes across buffer boundaries.
+ *
+ * @param[in,out] start_code A pointer to a mutable @c uint32_t.<br>
+ *          As input: For no history preset to <b>@c ~0 </b>, otherwise preset to the last
+ *                    returned start code to enable detecting start codes across
+ *                    buffer boundaries.<br>
+ *          On output: Set to the found start code if it exists or an invalid
+ *                     start code (the 4 bytes prior to the returned value,
+ *                     using the input history if @f$ end - p < 4 @f$).
+ *
+ * @sa avpriv_find_start_code()
+ */
+static const uint8_t *find_start_code_truncated(const uint8_t *av_restrict p,
+                                      const uint8_t * const end,
+                                      uint32_t * const av_restrict start_code)
+{
+    av_assert0(p <= end);
+    if (p >= end)
+        return end;
+
+    if (*start_code == 0x100)
+        *start_code = ~0;
+    // invalidate byte 0 so overlapping start codes are not erroneously detected
+
+    // read up to the first three bytes in p to enable reading a start code across
+    // two (to four) buffers
+    for (int i = 0; i < 3; i++) {
+        *start_code <<= 8;
+        *start_code += *p;
+        p++;
+        if (start_code_is_valid(*start_code) || p == end)
+            return p;
+    }
+    // buffer length is at least 4
+    return avpriv_find_start_code(p - 3, end, start_code);
+}
+
 /**
  * Find the end of the current frame in the bitstream.
  * @return the position of the first byte of the next frame, or -1
@@ -68,7 +107,7 @@  static int mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf,
             }
             state++;
         } else {
-            i = avpriv_find_start_code(buf + i, buf + buf_size, &state) - buf - 1;
+            i = find_start_code_truncated(buf + i, buf + buf_size, &state) - buf - 1;
             if (pc->frame_start_found == 0 && state >= SLICE_MIN_START_CODE && state <= SLICE_MAX_START_CODE) {
                 i++;
                 pc->frame_start_found = 4;
@@ -120,7 +159,6 @@  static void mpegvideo_extract_headers(AVCodecParserContext *s,
     s->repeat_pict = 0;
 
     while (buf < buf_end) {
-        start_code= -1;
         buf= avpriv_find_start_code(buf, buf_end, &start_code);
         bytes_left = buf_end - buf;
         switch(start_code) {
diff --git a/libavcodec/startcode.h b/libavcodec/startcode.h
index 69389c729c..7e1df68a3b 100644
--- a/libavcodec/startcode.h
+++ b/libavcodec/startcode.h
@@ -50,20 +50,14 @@  static av_always_inline int start_code_is_valid(uint32_t start_code) {
  * A start code is a sequence of 4 bytes with the hexadecimal value <b><tt> 00 00 01 XX </tt></b>,
  * where <b><tt> XX </tt></b> represents any value and memory address increases left to right.
  *
- * By preserving the <b>@p start_code</b> value between subsequent calls, the caller can
- * detect start codes across buffer boundaries.
- *
  * @param[in] p     A pointer to the start of the memory buffer to scan.
  * @param[in] end   A pointer to the past-the-end memory address for the buffer
  *                  given by @p p.  <b>@p p</b> must be ≤ <b>@p end</b>.
  *
- * @param[in,out] start_code A pointer to a mutable @c uint32_t.<br>
- *          As input: For no history preset to <b>@c ~0 </b>, otherwise preset to the last
- *                    returned start code to enable detecting start codes across
- *                    buffer boundaries.<br>
- *          On output: Set to the found start code if it exists or an invalid
- *                     start code (the 4 bytes prior to the returned value,
- *                     using the input history if @f$ end - p < 4 @f$).
+ * @param[out] start_code A pointer to a mutable @c uint32_t.<br>
+ *             Set to the found start code if it exists or an invalid start code
+ *             (the 4 bytes prior to the returned value or <b>@c ~0 </b> if
+ *             @f$ end - p < 4 @f$).
  *
  * @return A pointer to the memory address following the found start code, or <b>@p end</b>
  *         if no start code was found.
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index d11d92d21e..b9a396b181 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -941,19 +941,13 @@  const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       uint32_t * const av_restrict start_code)
 {
     av_assert0(p <= end);
-    if (p >= end)
+    // minimum length for a start code
+    if (p + 4 > end) {
+        *start_code = ~0; // set to an invalid start code
         return end;
-
-    // read up to the first three bytes in p to enable reading a start code across
-    // two (to four) buffers
-    for (int i = 0; i < 3; i++) {
-        *start_code <<= 8;
-        *start_code += *p;
-        p++;
-        if (start_code_is_valid(*start_code) || p == end)
-            return p;
     }
-    // p is now properly incremented for the negative indices in the while loop
+
+    p += 3; // offset for negative indices in while loop
 
     /* with memory address increasing left to right, we are looking for (in hexadecimal):
      * 00 00 01 XX
diff --git a/libavcodec/vc1_common.h b/libavcodec/vc1_common.h
index 8ff9802a51..ac8dbe3fb6 100644
--- a/libavcodec/vc1_common.h
+++ b/libavcodec/vc1_common.h
@@ -57,7 +57,7 @@  enum Profile {
 static av_always_inline const uint8_t* find_next_marker(const uint8_t *src, const uint8_t *end)
 {
     if (end - src >= 4) {
-        uint32_t mrk = 0xFFFFFFFF;
+        uint32_t mrk;
         src = avpriv_find_start_code(src, end, &mrk);
         if (start_code_is_valid(mrk))
             return src - 4;
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index 9c0816ef95..dbd4acd474 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -54,7 +54,7 @@  void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
 
             r1 = buf1;
             while (1) {
-                uint32_t start_code = ~0;
+                uint32_t start_code;
                 r = avpriv_find_start_code(r1, end, &start_code);
                 if (start_code_is_valid(start_code)) {
                     /* New start code found */