[FFmpeg-devel] [PATCH 0/8] rewrite avpriv_find_start_code() for clarity

[FFmpeg-devel] [PATCH 0/8] rewrite avpriv_find_start_code() for clarity

[Scott Theisen]

I am endeavoring to reduce MythTV’s downstream changes to FFmpeg.
avpriv_find_start_code() is one of the internal FFmpeg functions that
MythTV uses.

I was planning on copying it into MythTV to eventually eliminate all
uses of internal FFmpeg headers; however, what avpriv_find_start_code()
actually does was not very clear.  Therefore, I rewrote it and added
comments to make its purpose and effects clearer.


_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/8] avpriv_find_start_code(): readability enhancement part 1

[Scott Theisen]

No functional change.
---
 libavcodec/utils.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index b19befef21..cb4437edc2 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -967,10 +967,14 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
         }
     }
 
-    p = FFMIN(p, end) - 4;
-    *state = AV_RB32(p);
+    if (p > end)
+        p = end;
+    // this will cause the last 4 bytes before end to be read,
+    // i.e. no out of bounds memory access occurs
 
-    return p + 4;
+    *state = AV_RB32(p - 4); // read the previous 4 bytes
+
+    return p;
 }
 
 AVCPBProperties *av_cpb_properties_alloc(size_t *size)
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/8] avpriv_find_start_code(): rewrite while loop and add comments for clarity

[Scott Theisen]

The expected number of iterations may increase by one for an input of alternating
0 and 1 bytes.  Instead of incrementing by 2 everytime, it now alternates between
incrementing by 1 and by 3.

No functional change, but now much clearer.
---
 libavcodec/utils.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index cb4437edc2..882f90be79 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -957,12 +957,26 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
             return p;
     }
 
+    /* with memory address increasing left to right, we are looking for (in hexadecimal):
+     * 00 00 01 XX
+     * p points at the address which should have the value of XX
+     */
     while (p < end) {
-        if      (p[-1] > 1      ) p += 3;
-        else if (p[-2]          ) p += 2;
-        else if (p[-3]|(p[-1]-1)) p++;
+        // UU UU UU
+        if      (p[-1]  > 1) p += 3;    // start check over with 3 new bytes
+        else if (p[-1] == 0) p++;       // could be in a start code, so check next byte
+        // this should be one comparison against 1 since p is unsigned,
+        // i.e. p[-1] == 0 is equivalent to p[-1] < 1
+
+        // UU UU 01
+        else if (p[-2] != 0) p += 2;    // we have UU YY 01, so increment by 2
+                                        // to start check over with 3 new bytes
+        // UU 00 01
+        else if (p[-3] != 0) p += 3;    // we have YY 00 01, so increment by 3
+                                        // to start check over with 3 new bytes
+        // 00 00 01
         else {
-            p++;
+            p++; // p now points at the address following the start code value XX
             break;
         }
     }
@@ -972,7 +986,8 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     // this will cause the last 4 bytes before end to be read,
     // i.e. no out of bounds memory access occurs
 
-    *state = AV_RB32(p - 4); // read the previous 4 bytes
+    *state = AV_RB32(p - 4);
+    // read the previous 4 bytes, i.e. bytes {p - 4, p - 3, p - 2, p - 1}
 
     return p;
 }
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/8] avpriv_find_start_code(): make the state parameter output only

[Scott Theisen]

---
 libavcodec/cbs_mpeg2.c                | 8 --------
 libavcodec/mpeg12dec.c                | 5 ++---
 libavcodec/mpeg4_unpack_bframes_bsf.c | 1 -
 libavcodec/mpegvideo_parser.c         | 3 +--
 libavcodec/utils.c                    | 1 +
 libavformat/rtpenc_mpv.c              | 3 +--
 6 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 26400f279f..03ea49cbca 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -160,14 +160,6 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
     for (i = 0;; i++) {
         unit_type = start_code & 0xff;
 
-        if (start == frag->data + frag->data_size) {
-            // The last four bytes form a start code which constitutes
-            // a unit of its own.  In this situation avpriv_find_start_code
-            // won't modify start_code at all so modify start_code so that
-            // the next unit will be treated as the last unit.
-            start_code = 0;
-        }
-
         end = avpriv_find_start_code(start--, frag->data + frag->data_size,
                                      &start_code);
 
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 4a7bd6d466..1110fcb319 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1770,7 +1770,7 @@ static int mpeg_decode_slice(MpegEncContext *s, int mb_y,
 
     if (avctx->hwaccel && avctx->hwaccel->decode_slice) {
         const uint8_t *buf_end, *buf_start = *buf - 4; /* include start_code */
-        int start_code = -1;
+        uint32_t start_code;
         buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code);
         if (buf_end < *buf + buf_size)
             buf_end -= 4;
@@ -2020,7 +2020,6 @@ static int slice_decode_thread(AVCodecContext *c, void *arg)
         if (s->mb_y == s->end_mb_y)
             return 0;
 
-        start_code = -1;
         buf        = avpriv_find_start_code(buf, s->gb.buffer_end, &start_code);
         if (start_code < SLICE_MIN_START_CODE || start_code > SLICE_MAX_START_CODE)
             return AVERROR_INVALIDDATA;
@@ -2475,7 +2474,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
 
     for (;;) {
         /* find next start code */
-        uint32_t start_code = -1;
+        uint32_t start_code;
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
         if (start_code > 0x1ff) {
             if (!skip_frame) {
diff --git a/libavcodec/mpeg4_unpack_bframes_bsf.c b/libavcodec/mpeg4_unpack_bframes_bsf.c
index 6f8595713d..8b3fda0b03 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 c5dc867d24..c991a82405 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -105,7 +105,6 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
 {
     struct MpvParseContext *pc = s->priv_data;
     const uint8_t *buf_end = buf + buf_size;
-    uint32_t start_code;
     int frame_rate_index, ext_type, bytes_left;
     int frame_rate_ext_n, frame_rate_ext_d;
     int top_field_first, repeat_first_field, progressive_frame;
@@ -120,7 +119,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
     s->repeat_pict = 0;
 
     while (buf < buf_end) {
-        start_code= -1;
+        uint32_t start_code;
         buf= avpriv_find_start_code(buf, buf_end, &start_code);
         bytes_left = buf_end - buf;
         switch(start_code) {
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 882f90be79..cf88e0a759 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -950,6 +950,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     if (p >= end)
         return end;
 
+    *state = ~0;
     for (i = 0; i < 3; i++) {
         uint32_t tmp = *state << 8;
         *state = tmp + *(p++);
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index 4b45f51772..bb63c9bdc6 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -51,11 +51,10 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
             end_of_slice = 1;
         } else {
             const uint8_t *r, *r1;
-            int start_code;
 
             r1 = buf1;
             while (1) {
-                start_code = -1;
+                uint32_t start_code;
                 r = avpriv_find_start_code(r1, end, &start_code);
                 if((start_code & 0xFFFFFF00) == 0x100) {
                     /* New start code found */
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/8] avpriv_find_start_code(): make the state parameter output only

[Andreas Rheinhardt]

Scott Theisen:
> ---
>  libavcodec/cbs_mpeg2.c                | 8 --------
>  libavcodec/mpeg12dec.c                | 5 ++---
>  libavcodec/mpeg4_unpack_bframes_bsf.c | 1 -
>  libavcodec/mpegvideo_parser.c         | 3 +--
>  libavcodec/utils.c                    | 1 +
>  libavformat/rtpenc_mpv.c              | 3 +--
>  6 files changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 26400f279f..03ea49cbca 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -160,14 +160,6 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>      for (i = 0;; i++) {
>          unit_type = start_code & 0xff;
>  
> -        if (start == frag->data + frag->data_size) {
> -            // The last four bytes form a start code which constitutes
> -            // a unit of its own.  In this situation avpriv_find_start_code
> -            // won't modify start_code at all so modify start_code so that
> -            // the next unit will be treated as the last unit.
> -            start_code = 0;
> -        }
> -
>          end = avpriv_find_start_code(start--, frag->data + frag->data_size,
>                                       &start_code);
>  
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 4a7bd6d466..1110fcb319 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -1770,7 +1770,7 @@ static int mpeg_decode_slice(MpegEncContext *s, int mb_y,
>  
>      if (avctx->hwaccel && avctx->hwaccel->decode_slice) {
>          const uint8_t *buf_end, *buf_start = *buf - 4; /* include start_code */
> -        int start_code = -1;
> +        uint32_t start_code;
>          buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code);
>          if (buf_end < *buf + buf_size)
>              buf_end -= 4;
> @@ -2020,7 +2020,6 @@ static int slice_decode_thread(AVCodecContext *c, void *arg)
>          if (s->mb_y == s->end_mb_y)
>              return 0;
>  
> -        start_code = -1;
>          buf        = avpriv_find_start_code(buf, s->gb.buffer_end, &start_code);
>          if (start_code < SLICE_MIN_START_CODE || start_code > SLICE_MAX_START_CODE)
>              return AVERROR_INVALIDDATA;
> @@ -2475,7 +2474,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
>  
>      for (;;) {
>          /* find next start code */
> -        uint32_t start_code = -1;
> +        uint32_t start_code;
>          buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
>          if (start_code > 0x1ff) {
>              if (!skip_frame) {
> diff --git a/libavcodec/mpeg4_unpack_bframes_bsf.c b/libavcodec/mpeg4_unpack_bframes_bsf.c
> index 6f8595713d..8b3fda0b03 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 c5dc867d24..c991a82405 100644
> --- a/libavcodec/mpegvideo_parser.c
> +++ b/libavcodec/mpegvideo_parser.c
> @@ -105,7 +105,6 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
>  {
>      struct MpvParseContext *pc = s->priv_data;
>      const uint8_t *buf_end = buf + buf_size;
> -    uint32_t start_code;
>      int frame_rate_index, ext_type, bytes_left;
>      int frame_rate_ext_n, frame_rate_ext_d;
>      int top_field_first, repeat_first_field, progressive_frame;
> @@ -120,7 +119,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
>      s->repeat_pict = 0;
>  
>      while (buf < buf_end) {
> -        start_code= -1;
> +        uint32_t start_code;
>          buf= avpriv_find_start_code(buf, buf_end, &start_code);
>          bytes_left = buf_end - buf;
>          switch(start_code) {
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 882f90be79..cf88e0a759 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -950,6 +950,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
>      if (p >= end)
>          return end;
>  
> +    *state = ~0;
>      for (i = 0; i < 3; i++) {
>          uint32_t tmp = *state << 8;
>          *state = tmp + *(p++);
> diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
> index 4b45f51772..bb63c9bdc6 100644
> --- a/libavformat/rtpenc_mpv.c
> +++ b/libavformat/rtpenc_mpv.c
> @@ -51,11 +51,10 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
>              end_of_slice = 1;
>          } else {
>              const uint8_t *r, *r1;
> -            int start_code;
>  
>              r1 = buf1;
>              while (1) {
> -                start_code = -1;
> +                uint32_t start_code;
>                  r = avpriv_find_start_code(r1, end, &start_code);
>                  if((start_code & 0xFFFFFF00) == 0x100) {
>                      /* New start code found */

It seems you didn't get why the code currently is as it is: It allows to
use this function with non-contiguous input (as happens in our parsers
which operate on data that is not cleanly split into packets yet (it is
their job to create proper packets out of the input)). As an example,
say you have a buffer that ends with 0x00 00 and the next buffer
starting with 0x01 01. Then avpriv_find_start_code() will detect a start
code at the beginning of the second buffer if the user passes in the
same state again (and does not modify it in the meantime).
You would have noticed this if you had run FATE on your patches (see
https://ffmpeg.org/fate.html#Using-FATE-from-your-FFmpeg-source-directory).

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/8] avpriv_find_start_code(): make the state parameter output only

[Scott Theisen]

On 2/1/22 19:15, Andreas Rheinhardt wrote:
> It seems you didn't get why the code currently is as it is: It allows to
> use this function with non-contiguous input (as happens in our parsers
> which operate on data that is not cleanly split into packets yet (it is
> their job to create proper packets out of the input)). As an example,
> say you have a buffer that ends with 0x00 00 and the next buffer
> starting with 0x01 01. Then avpriv_find_start_code() will detect a start
> code at the beginning of the second buffer if the user passes in the
> same state again (and does not modify it in the meantime).

Without documentation or comments, in/out parameters are hard to infer 
and most uses were only out.

I might make a context free version for when the buffer is already 
packetized properly.

> You would have noticed this if you had run FATE on your patches (see
> https://ffmpeg.org/fate.html#Using-FATE-from-your-FFmpeg-source-directory).

Thanks for the link, I didn't see how to run FATE or get the samples for it.

Unfortunately, FATE fails before that:
```
CC    tests/api/api-threadmessage-test.o
LD    tests/api/api-threadmessage-test
TEST    api-threadmessage
Test api-threadmessage failed. Look at 
tests/data/fate/api-threadmessage.err for details.
make: *** [tests/Makefile:257: fate-api-threadmessage] Error 127
```

```
cat tests/data/fate/api-threadmessage.err
/home/htpc/build/mythtv-ffmpeg/FFmpeg/tests/api/api-threadmessage-test: 
error while loading shared libraries: libavutil.so.57: cannot open 
shared object file: No such file or directory
```

Which is strange because it does exist.
```
ls -l libavutil/libavutil*
-rw-rw-r-- 1 htpc htpc 4426450 Feb  1 22:06 libavutil/libavutil.a
-rw-rw-r-- 1 htpc htpc     353 Feb  1 21:06 libavutil/libavutil.pc
lrwxrwxrwx 1 htpc htpc      15 Feb  1 22:04 libavutil/libavutil.so -> 
libavutil.so.57
-rwxrwxr-x 1 htpc htpc 2602848 Feb  1 22:04 libavutil/libavutil.so.57
-rw-rw-r-- 1 htpc htpc      68 Nov  1 22:34 libavutil/libavutil.v
-rw-rw-r-- 1 htpc htpc      65 Feb  1 22:04 libavutil/libavutil.ver
-rw-rw-r-- 1 htpc htpc      82 Feb  1 21:05 libavutil/libavutil.version
```

I run configure with:
./configure --sysinclude=/usr/include --cc='ccache gcc' --cxx='ccache 
g++' --prefix=/usr/local --libdir=/usr/local/lib --enable-vdpau 
--enable-libxml2 --enable-libass --enable-libbluray --enable-vaapi 
--enable-libdrm --enable-gnutls --disable-stripping --disable-manpages 
--disable-podpages --disable-doc --disable-nvenc --enable-shared 
--disable-static --enable-gpl --enable-pic

Any idea why it's failing?

Thanks,
Scott
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/8] avpriv_find_start_code(): make the state parameter output only

[Andreas Rheinhardt]

Scott Theisen:
> On 2/1/22 19:15, Andreas Rheinhardt wrote:
>> It seems you didn't get why the code currently is as it is: It allows to
>> use this function with non-contiguous input (as happens in our parsers
>> which operate on data that is not cleanly split into packets yet (it is
>> their job to create proper packets out of the input)). As an example,
>> say you have a buffer that ends with 0x00 00 and the next buffer
>> starting with 0x01 01. Then avpriv_find_start_code() will detect a start
>> code at the beginning of the second buffer if the user passes in the
>> same state again (and does not modify it in the meantime).
> 
> Without documentation or comments, in/out parameters are hard to infer
> and most uses were only out.
> 
> I might make a context free version for when the buffer is already
> packetized properly.
> 
>> You would have noticed this if you had run FATE on your patches (see
>> https://ffmpeg.org/fate.html#Using-FATE-from-your-FFmpeg-source-directory).
>>
> 
> Thanks for the link, I didn't see how to run FATE or get the samples for
> it.
> 
> Unfortunately, FATE fails before that:
> ```
> CC    tests/api/api-threadmessage-test.o
> LD    tests/api/api-threadmessage-test
> TEST    api-threadmessage
> Test api-threadmessage failed. Look at
> tests/data/fate/api-threadmessage.err for details.
> make: *** [tests/Makefile:257: fate-api-threadmessage] Error 127
> ```
> 
> ```
> cat tests/data/fate/api-threadmessage.err
> /home/htpc/build/mythtv-ffmpeg/FFmpeg/tests/api/api-threadmessage-test:
> error while loading shared libraries: libavutil.so.57: cannot open
> shared object file: No such file or directory
> ```
> 
> Which is strange because it does exist.
> ```
> ls -l libavutil/libavutil*
> -rw-rw-r-- 1 htpc htpc 4426450 Feb  1 22:06 libavutil/libavutil.a
> -rw-rw-r-- 1 htpc htpc     353 Feb  1 21:06 libavutil/libavutil.pc
> lrwxrwxrwx 1 htpc htpc      15 Feb  1 22:04 libavutil/libavutil.so ->
> libavutil.so.57
> -rwxrwxr-x 1 htpc htpc 2602848 Feb  1 22:04 libavutil/libavutil.so.57
> -rw-rw-r-- 1 htpc htpc      68 Nov  1 22:34 libavutil/libavutil.v
> -rw-rw-r-- 1 htpc htpc      65 Feb  1 22:04 libavutil/libavutil.ver
> -rw-rw-r-- 1 htpc htpc      82 Feb  1 21:05 libavutil/libavutil.version
> ```
> 
> I run configure with:
> ./configure --sysinclude=/usr/include --cc='ccache gcc' --cxx='ccache
> g++' --prefix=/usr/local --libdir=/usr/local/lib --enable-vdpau
> --enable-libxml2 --enable-libass --enable-libbluray --enable-vaapi
> --enable-libdrm --enable-gnutls --disable-stripping --disable-manpages
> --disable-podpages --disable-doc --disable-nvenc --enable-shared
> --disable-static --enable-gpl --enable-pic
> 
> Any idea why it's failing?
> 

libavutil.so.57 (and the other libraries) are probably not in your
library search path. You might e.g. set an rpath.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/8] avpriv_find_start_code(): add doxygen comment and rename a parameter

[Scott Theisen]

---
 libavcodec/internal.h | 15 ++++++++++++++-
 libavcodec/utils.c    | 10 +++++-----
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 72ca1553f6..07098e1522 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -285,9 +285,22 @@ int ff_thread_can_start_frame(AVCodecContext *avctx);
 
 int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
 
+/**
+ * Find the first start code in the buffer p.  A start code is a sequence of 4
+ * bytes, with memory address increasing left to right and in hexadecimal, with
+ * the value 00 00 01 XX, where XX represents any value.
+ *
+ * @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 must be <= end.
+ *
+ * @param[out] start_code The found start code if it exists, otherwise an invalid start code.
+ * @return A pointer to the memory address following the found start code, or end
+ *         if no start code was found.
+ */
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t *end,
-                                      uint32_t *state);
+                                      uint32_t *start_code);
 
 int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec);
 
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index cf88e0a759..54c9dd056d 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -942,7 +942,7 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
 
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
-                                      uint32_t *av_restrict state)
+                                      uint32_t *av_restrict start_code)
 {
     int i;
 
@@ -950,10 +950,10 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     if (p >= end)
         return end;
 
-    *state = ~0;
+    *start_code = ~0;
     for (i = 0; i < 3; i++) {
-        uint32_t tmp = *state << 8;
-        *state = tmp + *(p++);
+        uint32_t tmp = *start_code << 8;
+        *start_code = tmp + *(p++);
         if (tmp == 0x100 || p == end)
             return p;
     }
@@ -987,7 +987,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     // this will cause the last 4 bytes before end to be read,
     // i.e. no out of bounds memory access occurs
 
-    *state = AV_RB32(p - 4);
+    *start_code = AV_RB32(p - 4);
     // read the previous 4 bytes, i.e. bytes {p - 4, p - 3, p - 2, p - 1}
 
     return p;
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/8] avpriv_find_start_code(): replace unnecessary for loop

[Scott Theisen]

start_code will still be invalid, i.e. all ones, but will no longer have
up to the first three bytes in p shifted in.
---
 libavcodec/utils.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 54c9dd056d..b4c5fa5009 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -944,19 +944,14 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
                                       uint32_t *av_restrict start_code)
 {
-    int i;
+    *start_code = ~0;
 
     av_assert0(p <= end);
-    if (p >= end)
+    // minimum length for a start code
+    if (p + 4 > end)
         return end;
 
-    *start_code = ~0;
-    for (i = 0; i < 3; i++) {
-        uint32_t tmp = *start_code << 8;
-        *start_code = tmp + *(p++);
-        if (tmp == 0x100 || p == end)
-            return p;
-    }
+    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
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 6/8] avcodec/internal.h: create avpriv_start_code_is_valid()

[Scott Theisen]

---
 libavcodec/cavsdec.c               |  2 +-
 libavcodec/cbs_mpeg2.c             |  4 ++--
 libavcodec/extract_extradata_bsf.c |  2 +-
 libavcodec/internal.h              | 14 ++++++++++++++
 libavcodec/mpeg12.c                |  2 +-
 libavcodec/mpeg12dec.c             |  2 +-
 libavcodec/mpegvideo_parser.c      |  2 +-
 libavcodec/remove_extradata_bsf.c  |  8 +++-----
 libavcodec/vaapi_vc1.c             |  2 +-
 libavcodec/vc1_common.h            |  4 +---
 libavcodec/vc1dec.c                |  2 +-
 libavformat/avs2dec.c              |  4 ++--
 libavformat/avs3dec.c              |  4 ++--
 libavformat/cavsvideodec.c         |  2 +-
 libavformat/mpegvideodec.c         |  2 +-
 libavformat/rtpenc_mpv.c           |  2 +-
 16 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 692c77eb39..a62177d520 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -1249,7 +1249,7 @@ static int cavs_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
     buf_end = buf + buf_size;
     for(;;) {
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
-        if ((stc & 0xFFFFFE00) || buf_ptr == buf_end) {
+        if (!avpriv_start_code_is_valid(stc) || buf_ptr == buf_end) {
             if (!h->stc)
                 av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
             return FFMAX(0, buf_ptr - buf);
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 03ea49cbca..648b270f44 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -152,7 +152,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
 
     start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
                                    &start_code);
-    if (start_code >> 8 != 0x000001) {
+    if (!avpriv_start_code_is_valid(start_code)) {
         // No start code found.
         return AVERROR_INVALIDDATA;
     }
@@ -167,7 +167,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         // (may be the last byte of fragment->data); end points to the byte
         // following the byte containing the start code identifier (or to
         // the end of fragment->data).
-        if (start_code >> 8 == 0x000001) {
+        if (avpriv_start_code_is_valid(start_code)) {
             // Unit runs from start to the beginning of the start code
             // pointed to by end (including any padding zeroes).
             unit_size = (end - 4) - start;
diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c
index dbcb8508b0..4df1c97139 100644
--- a/libavcodec/extract_extradata_bsf.c
+++ b/libavcodec/extract_extradata_bsf.c
@@ -240,7 +240,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt,
         ptr = avpriv_find_start_code(ptr, end, &state);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             has_extradata = 1;
-        } else if (has_extradata && IS_MARKER(state)) {
+        } else if (has_extradata && avpriv_start_code_is_valid(state)) {
             extradata_size = ptr - 4 - pkt->data;
             break;
         }
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 07098e1522..2f9f99482c 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -285,6 +285,20 @@ int ff_thread_can_start_frame(AVCodecContext *avctx);
 
 int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
 
+/**
+ * @brief Test whether a start code found by avpriv_find_start_code() is valid.
+ *
+ * Use this to test the validity of a start code or if a start code can be at the
+ * end of the buffer, where testing the return value of avpriv_find_start_code()
+ * would incorrectly imply that the start code is invalid.
+ *
+ * @param[in] start_code The start code to test.
+ * @return A boolean that is true if and only if start_code is valid
+ */
+static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
+    return (start_code & 0xFFFFFF00) == 0x100;
+}
+
 /**
  * Find the first start code in the buffer p.  A start code is a sequence of 4
  * bytes, with memory address increasing left to right and in hexadecimal, with
diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index 58e03c05d4..e45bc74479 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -217,7 +217,7 @@ int ff_mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size,
                 pc->frame_start_found = 0;
             if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
                 pc->frame_start_found++;
-            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
+            if (pc->frame_start_found == 4 && avpriv_start_code_is_valid(state)) {
                 if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
                     pc->frame_start_found = 0;
                     pc->state             = -1;
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 1110fcb319..691b535bfc 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2476,7 +2476,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
         /* find next start code */
         uint32_t start_code;
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
-        if (start_code > 0x1ff) {
+        if (!avpriv_start_code_is_valid(start_code)) {
             if (!skip_frame) {
                 if (HAVE_THREADS &&
                     (avctx->active_thread_type & FF_THREAD_SLICE) &&
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index c991a82405..9fd145586f 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -82,7 +82,7 @@ static int mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf,
                 pc->frame_start_found = 0;
             if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
                 pc->frame_start_found++;
-            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
+            if (pc->frame_start_found == 4 && avpriv_start_code_is_valid(state)) {
                 if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
                     pc->frame_start_found = 0;
                     pc->state             = -1;
diff --git a/libavcodec/remove_extradata_bsf.c b/libavcodec/remove_extradata_bsf.c
index 1d5f193f89..0e42174912 100644
--- a/libavcodec/remove_extradata_bsf.c
+++ b/libavcodec/remove_extradata_bsf.c
@@ -35,8 +35,6 @@ enum RemoveFreq {
     REMOVE_FREQ_NONKEYFRAME,
 };
 
-#define START_CODE 0x000001
-
 typedef struct RemoveExtradataContext {
     const AVClass *class;
     int freq;
@@ -73,7 +71,7 @@ static int h264_split(const uint8_t *buf, int buf_size)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &state);
-        if ((state & 0xFFFFFF00) != 0x100)
+        if (!avpriv_start_code_is_valid(state))
             break;
         nalu_type = state & 0x1F;
         if (nalu_type == H264_NAL_SPS) {
@@ -111,7 +109,7 @@ static int hevc_split(const uint8_t *buf, int buf_size)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &state);
-        if ((state >> 8) != START_CODE)
+        if (!avpriv_start_code_is_valid(state))
             break;
         nut = (state >> 1) & 0x3F;
         if (nut == HEVC_NAL_VPS)
@@ -171,7 +169,7 @@ static int vc1_split(const uint8_t *buf, int buf_size)
         ptr = avpriv_find_start_code(ptr, end, &state);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             charged = 1;
-        } else if (charged && IS_MARKER(state))
+        } else if (charged && avpriv_start_code_is_valid(state))
             return ptr - 4 - buf;
     }
 
diff --git a/libavcodec/vaapi_vc1.c b/libavcodec/vaapi_vc1.c
index 4e9607d9be..379104f688 100644
--- a/libavcodec/vaapi_vc1.c
+++ b/libavcodec/vaapi_vc1.c
@@ -471,7 +471,7 @@ static int vaapi_vc1_decode_slice(AVCodecContext *avctx, const uint8_t *buffer,
     int err;
 
     /* Current bit buffer is beyond any marker for VC-1, so skip it */
-    if (avctx->codec_id == AV_CODEC_ID_VC1 && IS_MARKER(AV_RB32(buffer))) {
+    if (avctx->codec_id == AV_CODEC_ID_VC1 && avpriv_start_code_is_valid(AV_RB32(buffer))) {
         buffer += 4;
         size -= 4;
     }
diff --git a/libavcodec/vc1_common.h b/libavcodec/vc1_common.h
index b46c33f7e2..483f86a4ee 100644
--- a/libavcodec/vc1_common.h
+++ b/libavcodec/vc1_common.h
@@ -41,8 +41,6 @@ enum VC1Code {
 };
 //@}
 
-#define IS_MARKER(x) (((x) & ~0xFF) == VC1_CODE_RES0)
-
 /** Available Profiles */
 //@{
 enum Profile {
@@ -61,7 +59,7 @@ static av_always_inline const uint8_t* find_next_marker(const uint8_t *src, cons
     if (end - src >= 4) {
         uint32_t mrk = 0xFFFFFFFF;
         src = avpriv_find_start_code(src, end, &mrk);
-        if (IS_MARKER(mrk))
+        if (avpriv_start_code_is_valid(mrk))
             return src - 4;
     }
     return end;
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index 7ed5133cfa..86749e5973 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -664,7 +664,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
         if (!buf2)
             return AVERROR(ENOMEM);
 
-        if (IS_MARKER(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
+        if (avpriv_start_code_is_valid(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
             const uint8_t *start, *end, *next;
             int size;
 
diff --git a/libavformat/avs2dec.c b/libavformat/avs2dec.c
index 51908d2b63..bdeb746fc7 100644
--- a/libavformat/avs2dec.c
+++ b/libavformat/avs2dec.c
@@ -42,8 +42,8 @@ static int avs2_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        state = code & 0xFF;
-        if ((code & 0xffffff00) == 0x100) {
+        if (avpriv_start_code_is_valid(code)) {
+            state = code & 0xFF;
             if (AVS2_ISUNIT(state)) {
                 if (sqb && !hds) {
                     hds = ptr - sqb;
diff --git a/libavformat/avs3dec.c b/libavformat/avs3dec.c
index 253caa7c1d..2daccd3d15 100644
--- a/libavformat/avs3dec.c
+++ b/libavformat/avs3dec.c
@@ -36,8 +36,8 @@ static int avs3video_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        state = code & 0xFF;
-        if ((code & 0xFFFFFF00) == 0x100) {
+        if (avpriv_start_code_is_valid(code)) {
+            state = code & 0xFF;
             if (state < AVS3_SEQ_START_CODE) {
                 if (code < slice_pos)
                     return 0;
diff --git a/libavformat/cavsvideodec.c b/libavformat/cavsvideodec.c
index 8900b97597..1d007708cc 100644
--- a/libavformat/cavsvideodec.c
+++ b/libavformat/cavsvideodec.c
@@ -38,7 +38,7 @@ static int cavsvideo_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        if ((code & 0xffffff00) == 0x100) {
+        if (avpriv_start_code_is_valid(code)) {
             if(code < CAVS_SEQ_START_CODE) {
                 /* slices have to be consecutive */
                 if(code < slice_pos)
diff --git a/libavformat/mpegvideodec.c b/libavformat/mpegvideodec.c
index 2d6f81aaa1..a9829dc1df 100644
--- a/libavformat/mpegvideodec.c
+++ b/libavformat/mpegvideodec.c
@@ -44,7 +44,7 @@ static int mpegvideo_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        if ((code & 0xffffff00) == 0x100) {
+        if (avpriv_start_code_is_valid(code)) {
             switch(code){
             case     SEQ_START_CODE:
                 if (!(ptr[3 + 1 + 2] & 0x20))
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index bb63c9bdc6..abfe3b32eb 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -56,7 +56,7 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
             while (1) {
                 uint32_t start_code;
                 r = avpriv_find_start_code(r1, end, &start_code);
-                if((start_code & 0xFFFFFF00) == 0x100) {
+                if (avpriv_start_code_is_valid(start_code)) {
                     /* New start code found */
                     if (start_code == 0x100) {
                         frame_type = (r[1] & 0x38) >> 3;
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 7/8] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop

[Scott Theisen]

This enhances the clarity of the code.
---
 libavcodec/cbs_mpeg2.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 648b270f44..2b80266910 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -148,7 +148,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
     CodedBitstreamUnitType unit_type;
     uint32_t start_code = -1;
     size_t unit_size;
-    int err, i, final = 0;
+    int err, i = 0, final = 0;
 
     start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
                                    &start_code);
@@ -157,7 +157,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         return AVERROR_INVALIDDATA;
     }
 
-    for (i = 0;; i++) {
+    while (!final) {
         unit_type = start_code & 0xff;
 
         end = avpriv_find_start_code(start--, frag->data + frag->data_size,
@@ -182,10 +182,8 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         if (err < 0)
             return err;
 
-        if (final)
-            break;
-
         start = end;
+        i++;
     }
 
     return 0;
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 8/8] avpriv_find_start_code(): reduce the number of iterations

[Scott Theisen]

by correctly starting with three new bytes on the next iteration,
instead of keeping byte p[-3] which is invalid, i.e. known to be 01
when it must be 00.
---
 libavcodec/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index b4c5fa5009..d485d0c96b 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -965,7 +965,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
         // i.e. p[-1] == 0 is equivalent to p[-1] < 1
 
         // UU UU 01
-        else if (p[-2] != 0) p += 2;    // we have UU YY 01, so increment by 2
+        else if (p[-2] != 0) p += 3;    // we have UU YY 01, so increment by 3
                                         // to start check over with 3 new bytes
         // UU 00 01
         else if (p[-3] != 0) p += 3;    // we have YY 00 01, so increment by 3
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 0/13] rewrite avpriv_find_start_code() for clarity

[Scott Theisen]

start_code is used as an [in,out] parameter only twice, once each in mpeg12.c
and mpegvideo_parser.c.  The input part of avpriv_find_start_code() has now
been rewritten to still allow this.

I did manage to run FATE this time and it completes without errors.

Regards,
Scott



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 01/13] avcodec/internal.h: create avpriv_start_code_is_valid()

[Scott Theisen]

---
 libavcodec/cavsdec.c               |  2 +-
 libavcodec/cbs_mpeg2.c             |  4 ++--
 libavcodec/extract_extradata_bsf.c |  2 +-
 libavcodec/internal.h              | 15 +++++++++++++++
 libavcodec/mpeg12.c                |  2 +-
 libavcodec/mpeg12dec.c             |  2 +-
 libavcodec/mpegvideo_parser.c      |  2 +-
 libavcodec/remove_extradata_bsf.c  |  8 +++-----
 libavcodec/vaapi_vc1.c             |  2 +-
 libavcodec/vc1_common.h            |  4 +---
 libavcodec/vc1dec.c                |  2 +-
 libavformat/avs2dec.c              |  4 ++--
 libavformat/avs3dec.c              |  4 ++--
 libavformat/cavsvideodec.c         |  2 +-
 libavformat/mpegvideodec.c         |  2 +-
 libavformat/rtpenc_mpv.c           |  2 +-
 16 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 692c77eb39..a62177d520 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -1249,7 +1249,7 @@ static int cavs_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
     buf_end = buf + buf_size;
     for(;;) {
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
-        if ((stc & 0xFFFFFE00) || buf_ptr == buf_end) {
+        if (!avpriv_start_code_is_valid(stc) || buf_ptr == buf_end) {
             if (!h->stc)
                 av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
             return FFMAX(0, buf_ptr - buf);
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 26400f279f..eb45929132 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -152,7 +152,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
 
     start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
                                    &start_code);
-    if (start_code >> 8 != 0x000001) {
+    if (!avpriv_start_code_is_valid(start_code)) {
         // No start code found.
         return AVERROR_INVALIDDATA;
     }
@@ -175,7 +175,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         // (may be the last byte of fragment->data); end points to the byte
         // following the byte containing the start code identifier (or to
         // the end of fragment->data).
-        if (start_code >> 8 == 0x000001) {
+        if (avpriv_start_code_is_valid(start_code)) {
             // Unit runs from start to the beginning of the start code
             // pointed to by end (including any padding zeroes).
             unit_size = (end - 4) - start;
diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c
index dbcb8508b0..4df1c97139 100644
--- a/libavcodec/extract_extradata_bsf.c
+++ b/libavcodec/extract_extradata_bsf.c
@@ -240,7 +240,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt,
         ptr = avpriv_find_start_code(ptr, end, &state);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             has_extradata = 1;
-        } else if (has_extradata && IS_MARKER(state)) {
+        } else if (has_extradata && avpriv_start_code_is_valid(state)) {
             extradata_size = ptr - 4 - pkt->data;
             break;
         }
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 72ca1553f6..005f308b70 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -285,6 +285,21 @@ int ff_thread_can_start_frame(AVCodecContext *avctx);
 
 int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
 
+/**
+ * @brief Test whether a start code found by avpriv_find_start_code() is valid.
+ *
+ * Use this to test the validity of a start code especially if a start code can
+ * be at the end of the buffer, where testing the return value of avpriv_find_start_code()
+ * would incorrectly imply that the start code is invalid (since the returned value
+ * equals <b>@c end </b>).
+ *
+ * @param[in] start_code The start code to test.
+ * @return A boolean that is true if and only if <b>@p start_code</b> is valid
+ */
+static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
+    return (start_code & 0xFFFFFF00) == 0x100;
+}
+
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t *end,
                                       uint32_t *state);
diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index 58e03c05d4..e45bc74479 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -217,7 +217,7 @@ int ff_mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size,
                 pc->frame_start_found = 0;
             if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
                 pc->frame_start_found++;
-            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
+            if (pc->frame_start_found == 4 && avpriv_start_code_is_valid(state)) {
                 if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
                     pc->frame_start_found = 0;
                     pc->state             = -1;
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 4a7bd6d466..65b66d11f8 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2477,7 +2477,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
         /* find next start code */
         uint32_t start_code = -1;
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
-        if (start_code > 0x1ff) {
+        if (!avpriv_start_code_is_valid(start_code)) {
             if (!skip_frame) {
                 if (HAVE_THREADS &&
                     (avctx->active_thread_type & FF_THREAD_SLICE) &&
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index c5dc867d24..f0897e7e2c 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -82,7 +82,7 @@ static int mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf,
                 pc->frame_start_found = 0;
             if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
                 pc->frame_start_found++;
-            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
+            if (pc->frame_start_found == 4 && avpriv_start_code_is_valid(state)) {
                 if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
                     pc->frame_start_found = 0;
                     pc->state             = -1;
diff --git a/libavcodec/remove_extradata_bsf.c b/libavcodec/remove_extradata_bsf.c
index 1d5f193f89..0e42174912 100644
--- a/libavcodec/remove_extradata_bsf.c
+++ b/libavcodec/remove_extradata_bsf.c
@@ -35,8 +35,6 @@ enum RemoveFreq {
     REMOVE_FREQ_NONKEYFRAME,
 };
 
-#define START_CODE 0x000001
-
 typedef struct RemoveExtradataContext {
     const AVClass *class;
     int freq;
@@ -73,7 +71,7 @@ static int h264_split(const uint8_t *buf, int buf_size)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &state);
-        if ((state & 0xFFFFFF00) != 0x100)
+        if (!avpriv_start_code_is_valid(state))
             break;
         nalu_type = state & 0x1F;
         if (nalu_type == H264_NAL_SPS) {
@@ -111,7 +109,7 @@ static int hevc_split(const uint8_t *buf, int buf_size)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &state);
-        if ((state >> 8) != START_CODE)
+        if (!avpriv_start_code_is_valid(state))
             break;
         nut = (state >> 1) & 0x3F;
         if (nut == HEVC_NAL_VPS)
@@ -171,7 +169,7 @@ static int vc1_split(const uint8_t *buf, int buf_size)
         ptr = avpriv_find_start_code(ptr, end, &state);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             charged = 1;
-        } else if (charged && IS_MARKER(state))
+        } else if (charged && avpriv_start_code_is_valid(state))
             return ptr - 4 - buf;
     }
 
diff --git a/libavcodec/vaapi_vc1.c b/libavcodec/vaapi_vc1.c
index 4e9607d9be..379104f688 100644
--- a/libavcodec/vaapi_vc1.c
+++ b/libavcodec/vaapi_vc1.c
@@ -471,7 +471,7 @@ static int vaapi_vc1_decode_slice(AVCodecContext *avctx, const uint8_t *buffer,
     int err;
 
     /* Current bit buffer is beyond any marker for VC-1, so skip it */
-    if (avctx->codec_id == AV_CODEC_ID_VC1 && IS_MARKER(AV_RB32(buffer))) {
+    if (avctx->codec_id == AV_CODEC_ID_VC1 && avpriv_start_code_is_valid(AV_RB32(buffer))) {
         buffer += 4;
         size -= 4;
     }
diff --git a/libavcodec/vc1_common.h b/libavcodec/vc1_common.h
index b46c33f7e2..483f86a4ee 100644
--- a/libavcodec/vc1_common.h
+++ b/libavcodec/vc1_common.h
@@ -41,8 +41,6 @@ enum VC1Code {
 };
 //@}
 
-#define IS_MARKER(x) (((x) & ~0xFF) == VC1_CODE_RES0)
-
 /** Available Profiles */
 //@{
 enum Profile {
@@ -61,7 +59,7 @@ static av_always_inline const uint8_t* find_next_marker(const uint8_t *src, cons
     if (end - src >= 4) {
         uint32_t mrk = 0xFFFFFFFF;
         src = avpriv_find_start_code(src, end, &mrk);
-        if (IS_MARKER(mrk))
+        if (avpriv_start_code_is_valid(mrk))
             return src - 4;
     }
     return end;
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index 7ed5133cfa..86749e5973 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -664,7 +664,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
         if (!buf2)
             return AVERROR(ENOMEM);
 
-        if (IS_MARKER(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
+        if (avpriv_start_code_is_valid(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
             const uint8_t *start, *end, *next;
             int size;
 
diff --git a/libavformat/avs2dec.c b/libavformat/avs2dec.c
index 51908d2b63..bdeb746fc7 100644
--- a/libavformat/avs2dec.c
+++ b/libavformat/avs2dec.c
@@ -42,8 +42,8 @@ static int avs2_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        state = code & 0xFF;
-        if ((code & 0xffffff00) == 0x100) {
+        if (avpriv_start_code_is_valid(code)) {
+            state = code & 0xFF;
             if (AVS2_ISUNIT(state)) {
                 if (sqb && !hds) {
                     hds = ptr - sqb;
diff --git a/libavformat/avs3dec.c b/libavformat/avs3dec.c
index 253caa7c1d..2daccd3d15 100644
--- a/libavformat/avs3dec.c
+++ b/libavformat/avs3dec.c
@@ -36,8 +36,8 @@ static int avs3video_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        state = code & 0xFF;
-        if ((code & 0xFFFFFF00) == 0x100) {
+        if (avpriv_start_code_is_valid(code)) {
+            state = code & 0xFF;
             if (state < AVS3_SEQ_START_CODE) {
                 if (code < slice_pos)
                     return 0;
diff --git a/libavformat/cavsvideodec.c b/libavformat/cavsvideodec.c
index 8900b97597..1d007708cc 100644
--- a/libavformat/cavsvideodec.c
+++ b/libavformat/cavsvideodec.c
@@ -38,7 +38,7 @@ static int cavsvideo_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        if ((code & 0xffffff00) == 0x100) {
+        if (avpriv_start_code_is_valid(code)) {
             if(code < CAVS_SEQ_START_CODE) {
                 /* slices have to be consecutive */
                 if(code < slice_pos)
diff --git a/libavformat/mpegvideodec.c b/libavformat/mpegvideodec.c
index 2d6f81aaa1..a9829dc1df 100644
--- a/libavformat/mpegvideodec.c
+++ b/libavformat/mpegvideodec.c
@@ -44,7 +44,7 @@ static int mpegvideo_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        if ((code & 0xffffff00) == 0x100) {
+        if (avpriv_start_code_is_valid(code)) {
             switch(code){
             case     SEQ_START_CODE:
                 if (!(ptr[3 + 1 + 2] & 0x20))
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index 4b45f51772..05a77fa11c 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -57,7 +57,7 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
             while (1) {
                 start_code = -1;
                 r = avpriv_find_start_code(r1, end, &start_code);
-                if((start_code & 0xFFFFFF00) == 0x100) {
+                if (avpriv_start_code_is_valid(start_code)) {
                     /* New start code found */
                     if (start_code == 0x100) {
                         frame_type = (r[1] & 0x38) >> 3;
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 01/13] avcodec/internal.h: create avpriv_start_code_is_valid()

[Andreas Rheinhardt]

Scott Theisen:
> ---
>  libavcodec/cavsdec.c               |  2 +-
>  libavcodec/cbs_mpeg2.c             |  4 ++--
>  libavcodec/extract_extradata_bsf.c |  2 +-
>  libavcodec/internal.h              | 15 +++++++++++++++
>  libavcodec/mpeg12.c                |  2 +-
>  libavcodec/mpeg12dec.c             |  2 +-
>  libavcodec/mpegvideo_parser.c      |  2 +-
>  libavcodec/remove_extradata_bsf.c  |  8 +++-----
>  libavcodec/vaapi_vc1.c             |  2 +-
>  libavcodec/vc1_common.h            |  4 +---
>  libavcodec/vc1dec.c                |  2 +-
>  libavformat/avs2dec.c              |  4 ++--
>  libavformat/avs3dec.c              |  4 ++--
>  libavformat/cavsvideodec.c         |  2 +-
>  libavformat/mpegvideodec.c         |  2 +-
>  libavformat/rtpenc_mpv.c           |  2 +-
>  16 files changed, 35 insertions(+), 24 deletions(-)
> 
> diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
> index 692c77eb39..a62177d520 100644
> --- a/libavcodec/cavsdec.c
> +++ b/libavcodec/cavsdec.c
> @@ -1249,7 +1249,7 @@ static int cavs_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>      buf_end = buf + buf_size;
>      for(;;) {
>          buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
> -        if ((stc & 0xFFFFFE00) || buf_ptr == buf_end) {
> +        if (!avpriv_start_code_is_valid(stc) || buf_ptr == buf_end) {
>              if (!h->stc)
>                  av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
>              return FFMAX(0, buf_ptr - buf);
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 26400f279f..eb45929132 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -152,7 +152,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>  
>      start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
>                                     &start_code);
> -    if (start_code >> 8 != 0x000001) {
> +    if (!avpriv_start_code_is_valid(start_code)) {
>          // No start code found.
>          return AVERROR_INVALIDDATA;
>      }
> @@ -175,7 +175,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>          // (may be the last byte of fragment->data); end points to the byte
>          // following the byte containing the start code identifier (or to
>          // the end of fragment->data).
> -        if (start_code >> 8 == 0x000001) {
> +        if (avpriv_start_code_is_valid(start_code)) {
>              // Unit runs from start to the beginning of the start code
>              // pointed to by end (including any padding zeroes).
>              unit_size = (end - 4) - start;
> diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c
> index dbcb8508b0..4df1c97139 100644
> --- a/libavcodec/extract_extradata_bsf.c
> +++ b/libavcodec/extract_extradata_bsf.c
> @@ -240,7 +240,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt,
>          ptr = avpriv_find_start_code(ptr, end, &state);
>          if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
>              has_extradata = 1;
> -        } else if (has_extradata && IS_MARKER(state)) {
> +        } else if (has_extradata && avpriv_start_code_is_valid(state)) {
>              extradata_size = ptr - 4 - pkt->data;
>              break;
>          }
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 72ca1553f6..005f308b70 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -285,6 +285,21 @@ int ff_thread_can_start_frame(AVCodecContext *avctx);
>  
>  int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
>  
> +/**
> + * @brief Test whether a start code found by avpriv_find_start_code() is valid.
> + *
> + * Use this to test the validity of a start code especially if a start code can
> + * be at the end of the buffer, where testing the return value of avpriv_find_start_code()
> + * would incorrectly imply that the start code is invalid (since the returned value
> + * equals <b>@c end </b>).
> + *
> + * @param[in] start_code The start code to test.
> + * @return A boolean that is true if and only if <b>@p start_code</b> is valid
> + */
> +static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
> +    return (start_code & 0xFFFFFF00) == 0x100;
> +}
> +
>  const uint8_t *avpriv_find_start_code(const uint8_t *p,
>                                        const uint8_t *end,
>                                        uint32_t *state);
> diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
> index 58e03c05d4..e45bc74479 100644
> --- a/libavcodec/mpeg12.c
> +++ b/libavcodec/mpeg12.c
> @@ -217,7 +217,7 @@ int ff_mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size,
>                  pc->frame_start_found = 0;
>              if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
>                  pc->frame_start_found++;
> -            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
> +            if (pc->frame_start_found == 4 && avpriv_start_code_is_valid(state)) {
>                  if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
>                      pc->frame_start_found = 0;
>                      pc->state             = -1;
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 4a7bd6d466..65b66d11f8 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -2477,7 +2477,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
>          /* find next start code */
>          uint32_t start_code = -1;
>          buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
> -        if (start_code > 0x1ff) {
> +        if (!avpriv_start_code_is_valid(start_code)) {
>              if (!skip_frame) {
>                  if (HAVE_THREADS &&
>                      (avctx->active_thread_type & FF_THREAD_SLICE) &&
> diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
> index c5dc867d24..f0897e7e2c 100644
> --- a/libavcodec/mpegvideo_parser.c
> +++ b/libavcodec/mpegvideo_parser.c
> @@ -82,7 +82,7 @@ static int mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf,
>                  pc->frame_start_found = 0;
>              if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
>                  pc->frame_start_found++;
> -            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
> +            if (pc->frame_start_found == 4 && avpriv_start_code_is_valid(state)) {
>                  if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
>                      pc->frame_start_found = 0;
>                      pc->state             = -1;
> diff --git a/libavcodec/remove_extradata_bsf.c b/libavcodec/remove_extradata_bsf.c
> index 1d5f193f89..0e42174912 100644
> --- a/libavcodec/remove_extradata_bsf.c
> +++ b/libavcodec/remove_extradata_bsf.c
> @@ -35,8 +35,6 @@ enum RemoveFreq {
>      REMOVE_FREQ_NONKEYFRAME,
>  };
>  
> -#define START_CODE 0x000001
> -
>  typedef struct RemoveExtradataContext {
>      const AVClass *class;
>      int freq;
> @@ -73,7 +71,7 @@ static int h264_split(const uint8_t *buf, int buf_size)
>  
>      while (ptr < end) {
>          ptr = avpriv_find_start_code(ptr, end, &state);
> -        if ((state & 0xFFFFFF00) != 0x100)
> +        if (!avpriv_start_code_is_valid(state))
>              break;
>          nalu_type = state & 0x1F;
>          if (nalu_type == H264_NAL_SPS) {
> @@ -111,7 +109,7 @@ static int hevc_split(const uint8_t *buf, int buf_size)
>  
>      while (ptr < end) {
>          ptr = avpriv_find_start_code(ptr, end, &state);
> -        if ((state >> 8) != START_CODE)
> +        if (!avpriv_start_code_is_valid(state))
>              break;
>          nut = (state >> 1) & 0x3F;
>          if (nut == HEVC_NAL_VPS)
> @@ -171,7 +169,7 @@ static int vc1_split(const uint8_t *buf, int buf_size)
>          ptr = avpriv_find_start_code(ptr, end, &state);
>          if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
>              charged = 1;
> -        } else if (charged && IS_MARKER(state))
> +        } else if (charged && avpriv_start_code_is_valid(state))
>              return ptr - 4 - buf;
>      }
>  
> diff --git a/libavcodec/vaapi_vc1.c b/libavcodec/vaapi_vc1.c
> index 4e9607d9be..379104f688 100644
> --- a/libavcodec/vaapi_vc1.c
> +++ b/libavcodec/vaapi_vc1.c
> @@ -471,7 +471,7 @@ static int vaapi_vc1_decode_slice(AVCodecContext *avctx, const uint8_t *buffer,
>      int err;
>  
>      /* Current bit buffer is beyond any marker for VC-1, so skip it */
> -    if (avctx->codec_id == AV_CODEC_ID_VC1 && IS_MARKER(AV_RB32(buffer))) {
> +    if (avctx->codec_id == AV_CODEC_ID_VC1 && avpriv_start_code_is_valid(AV_RB32(buffer))) {
>          buffer += 4;
>          size -= 4;
>      }
> diff --git a/libavcodec/vc1_common.h b/libavcodec/vc1_common.h
> index b46c33f7e2..483f86a4ee 100644
> --- a/libavcodec/vc1_common.h
> +++ b/libavcodec/vc1_common.h
> @@ -41,8 +41,6 @@ enum VC1Code {
>  };
>  //@}
>  
> -#define IS_MARKER(x) (((x) & ~0xFF) == VC1_CODE_RES0)
> -
>  /** Available Profiles */
>  //@{
>  enum Profile {
> @@ -61,7 +59,7 @@ static av_always_inline const uint8_t* find_next_marker(const uint8_t *src, cons
>      if (end - src >= 4) {
>          uint32_t mrk = 0xFFFFFFFF;
>          src = avpriv_find_start_code(src, end, &mrk);
> -        if (IS_MARKER(mrk))
> +        if (avpriv_start_code_is_valid(mrk))
>              return src - 4;
>      }
>      return end;
> diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> index 7ed5133cfa..86749e5973 100644
> --- a/libavcodec/vc1dec.c
> +++ b/libavcodec/vc1dec.c
> @@ -664,7 +664,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
>          if (!buf2)
>              return AVERROR(ENOMEM);
>  
> -        if (IS_MARKER(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
> +        if (avpriv_start_code_is_valid(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
>              const uint8_t *start, *end, *next;
>              int size;
>  
> diff --git a/libavformat/avs2dec.c b/libavformat/avs2dec.c
> index 51908d2b63..bdeb746fc7 100644
> --- a/libavformat/avs2dec.c
> +++ b/libavformat/avs2dec.c
> @@ -42,8 +42,8 @@ static int avs2_probe(const AVProbeData *p)
>  
>      while (ptr < end) {
>          ptr = avpriv_find_start_code(ptr, end, &code);
> -        state = code & 0xFF;
> -        if ((code & 0xffffff00) == 0x100) {
> +        if (avpriv_start_code_is_valid(code)) {
> +            state = code & 0xFF;
>              if (AVS2_ISUNIT(state)) {
>                  if (sqb && !hds) {
>                      hds = ptr - sqb;
> diff --git a/libavformat/avs3dec.c b/libavformat/avs3dec.c
> index 253caa7c1d..2daccd3d15 100644
> --- a/libavformat/avs3dec.c
> +++ b/libavformat/avs3dec.c
> @@ -36,8 +36,8 @@ static int avs3video_probe(const AVProbeData *p)
>  
>      while (ptr < end) {
>          ptr = avpriv_find_start_code(ptr, end, &code);
> -        state = code & 0xFF;
> -        if ((code & 0xFFFFFF00) == 0x100) {
> +        if (avpriv_start_code_is_valid(code)) {
> +            state = code & 0xFF;
>              if (state < AVS3_SEQ_START_CODE) {
>                  if (code < slice_pos)
>                      return 0;
> diff --git a/libavformat/cavsvideodec.c b/libavformat/cavsvideodec.c
> index 8900b97597..1d007708cc 100644
> --- a/libavformat/cavsvideodec.c
> +++ b/libavformat/cavsvideodec.c
> @@ -38,7 +38,7 @@ static int cavsvideo_probe(const AVProbeData *p)
>  
>      while (ptr < end) {
>          ptr = avpriv_find_start_code(ptr, end, &code);
> -        if ((code & 0xffffff00) == 0x100) {
> +        if (avpriv_start_code_is_valid(code)) {
>              if(code < CAVS_SEQ_START_CODE) {
>                  /* slices have to be consecutive */
>                  if(code < slice_pos)
> diff --git a/libavformat/mpegvideodec.c b/libavformat/mpegvideodec.c
> index 2d6f81aaa1..a9829dc1df 100644
> --- a/libavformat/mpegvideodec.c
> +++ b/libavformat/mpegvideodec.c
> @@ -44,7 +44,7 @@ static int mpegvideo_probe(const AVProbeData *p)
>  
>      while (ptr < end) {
>          ptr = avpriv_find_start_code(ptr, end, &code);
> -        if ((code & 0xffffff00) == 0x100) {
> +        if (avpriv_start_code_is_valid(code)) {
>              switch(code){
>              case     SEQ_START_CODE:
>                  if (!(ptr[3 + 1 + 2] & 0x20))
> diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
> index 4b45f51772..05a77fa11c 100644
> --- a/libavformat/rtpenc_mpv.c
> +++ b/libavformat/rtpenc_mpv.c
> @@ -57,7 +57,7 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
>              while (1) {
>                  start_code = -1;
>                  r = avpriv_find_start_code(r1, end, &start_code);
> -                if((start_code & 0xFFFFFF00) == 0x100) {
> +                if (avpriv_start_code_is_valid(start_code)) {
>                      /* New start code found */
>                      if (start_code == 0x100) {
>                          frame_type = (r[1] & 0x38) >> 3;

a) We use the avpriv prefix for things that should be exported from one
library to be used in other libraries, but not for public use. Therefore
the avpriv prefix is inappropriate here as this function is static. And
it makes a very long name.
b) internal.h is the wrong header for this: There are more start codes
than 00 00 01. That's why I sent the patch to move
avpriv_find_start_code() to libavcodec/startcode.h.
c) I am not sure that the new code is equivalent to the old one in all
instances: mpeg12dec.c checks for "start_code > 0x1ff" to mean "no valid
start code", yet if the buffer ended with anything in the range
0x00-0xff it would be considered a start code before this patch and now
it would no longer be a start code. I don't think this is a bad change,
though, but it should be noted in the commit message.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 01/13] avcodec/internal.h: create avpriv_start_code_is_valid()

[Scott Theisen]

On 2/5/22 00:42, Andreas Rheinhardt wrote:
> a) We use the avpriv prefix for things that should be exported from one
> library to be used in other libraries, but not for public use. Therefore
> the avpriv prefix is inappropriate here as this function is static. And
> it makes a very long name.

So since this is static av_always_inline, it should just be 
start_code_is_valid()?

> b) internal.h is the wrong header for this: There are more start codes
> than 00 00 01. That's why I sent the patch to move
> avpriv_find_start_code() to libavcodec/startcode.h.

I only put it there because that is where avpriv_find_start_code() is 
declared, so I knew it was already included.

Should I move the definition and declaration of avpriv_find_start_code() 
to startcode.(c|h)?

> c) I am not sure that the new code is equivalent to the old one in all
> instances: mpeg12dec.c checks for "start_code > 0x1ff" to mean "no valid
> start code", yet if the buffer ended with anything in the range
> 0x00-0xff it would be considered a start code before this patch and now
> it would no longer be a start code. I don't think this is a bad change,
> though, but it should be noted in the commit message.

I'll have to look at them all again and I'll add that to the commit message.

-Scott
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 02/13] avpriv_find_start_code(): readability enhancement part 1

[Scott Theisen]

No functional change.
---
 libavcodec/utils.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index b19befef21..cb4437edc2 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -967,10 +967,14 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
         }
     }
 
-    p = FFMIN(p, end) - 4;
-    *state = AV_RB32(p);
+    if (p > end)
+        p = end;
+    // this will cause the last 4 bytes before end to be read,
+    // i.e. no out of bounds memory access occurs
 
-    return p + 4;
+    *state = AV_RB32(p - 4); // read the previous 4 bytes
+
+    return p;
 }
 
 AVCPBProperties *av_cpb_properties_alloc(size_t *size)
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 02/13] avpriv_find_start_code(): readability enhancement part 1

[Andreas Rheinhardt]

Scott Theisen:
> No functional change.
> ---
>  libavcodec/utils.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index b19befef21..cb4437edc2 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -967,10 +967,14 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
>          }
>      }
>  
> -    p = FFMIN(p, end) - 4;
> -    *state = AV_RB32(p);
> +    if (p > end)
> +        p = end;
> +    // this will cause the last 4 bytes before end to be read,
> +    // i.e. no out of bounds memory access occurs
>  
> -    return p + 4;
> +    *state = AV_RB32(p - 4); // read the previous 4 bytes
> +
> +    return p;
>  }
>  
>  AVCPBProperties *av_cpb_properties_alloc(size_t *size)

Where exactly is the readability enhancement supposed to be? I only see
the opposite: The earlier code spoke for itself; not this simplicity is
obscured by lots of comments. Having to parse lots of comments makes the
code harder to read.
This is also my impression with your other clarification patches.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 03/13] avpriv_find_start_code(): rewrite while loop and add comments for clarity

[Scott Theisen]

The expected number of iterations may increase by one for an input of alternating
0 and 1 bytes.  Instead of incrementing by 2 everytime, it now alternates between
incrementing by 1 and by 3.

No functional change, but now much clearer.

For the check p[-2] != 0:
Also reduce the number of iterations by correctly starting with three new bytes on the next iteration,
instead of keeping byte p[-3] which is invalid, i.e. known to be 01 when it must be 00.
---
 libavcodec/utils.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index cb4437edc2..8f8cc820bd 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -957,12 +957,26 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
             return p;
     }
 
+    /* with memory address increasing left to right, we are looking for (in hexadecimal):
+     * 00 00 01 XX
+     * p points at the address which should have the value of XX
+     */
     while (p < end) {
-        if      (p[-1] > 1      ) p += 3;
-        else if (p[-2]          ) p += 2;
-        else if (p[-3]|(p[-1]-1)) p++;
+        // UU UU UU
+        if      (p[-1]  > 1) p += 3;    // start check over with 3 new bytes
+        else if (p[-1] == 0) p++;       // could be in a start code, so check next byte
+        // this should be one comparison against 1 since p is unsigned,
+        // i.e. p[-1] == 0 is equivalent to p[-1] < 1
+
+        // UU UU 01
+        else if (p[-2] != 0) p += 3;    // we have UU YY 01, so increment by 3
+                                        // to start check over with 3 new bytes
+        // UU 00 01
+        else if (p[-3] != 0) p += 3;    // we have YY 00 01, so increment by 3
+                                        // to start check over with 3 new bytes
+        // 00 00 01
         else {
-            p++;
+            p++; // p now points at the address following the start code value XX
             break;
         }
     }
@@ -972,7 +986,8 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     // this will cause the last 4 bytes before end to be read,
     // i.e. no out of bounds memory access occurs
 
-    *state = AV_RB32(p - 4); // read the previous 4 bytes
+    *state = AV_RB32(p - 4);
+    // read the previous 4 bytes, i.e. bytes {p - 4, p - 3, p - 2, p - 1}
 
     return p;
 }
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 04/13] avpriv_find_start_code(): rewrite for loop for clarity

[Scott Theisen]

---
 libavcodec/utils.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 8f8cc820bd..1a806b9fa8 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -944,18 +944,20 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
                                       uint32_t *av_restrict state)
 {
-    int i;
-
     av_assert0(p <= end);
     if (p >= end)
         return end;
 
-    for (i = 0; i < 3; i++) {
-        uint32_t tmp = *state << 8;
-        *state = tmp + *(p++);
-        if (tmp == 0x100 || p == 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++) {
+        *state <<= 8;
+        *state += *p;
+        p++;
+        if (avpriv_start_code_is_valid(*state) || p == end)
             return p;
     }
+    // p is now properly incremented for the negative indices in the while loop
 
     /* with memory address increasing left to right, we are looking for (in hexadecimal):
      * 00 00 01 XX
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 05/13] avpriv_find_start_code(): add doxygen comment and rename a parameter

[Scott Theisen]

---
 libavcodec/internal.h | 26 +++++++++++++++++++++++++-
 libavcodec/utils.c    | 10 +++++-----
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 005f308b70..94c41aef0b 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -300,9 +300,33 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
     return (start_code & 0xFFFFFF00) == 0x100;
 }
 
+/**
+ * @brief Find the first start code in the buffer @p p.
+ *
+ * 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 reference 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$ p - end < 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.
+ */
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t *end,
-                                      uint32_t *state);
+                                      uint32_t *start_code);
 
 int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec);
 
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 1a806b9fa8..80ccde023f 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -942,7 +942,7 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
 
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
-                                      uint32_t *av_restrict state)
+                                      uint32_t *av_restrict start_code)
 {
     av_assert0(p <= end);
     if (p >= end)
@@ -951,10 +951,10 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     // 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++) {
-        *state <<= 8;
-        *state += *p;
+        *start_code <<= 8;
+        *start_code += *p;
         p++;
-        if (avpriv_start_code_is_valid(*state) || p == end)
+        if (avpriv_start_code_is_valid(*start_code) || p == end)
             return p;
     }
     // p is now properly incremented for the negative indices in the while loop
@@ -988,7 +988,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     // this will cause the last 4 bytes before end to be read,
     // i.e. no out of bounds memory access occurs
 
-    *state = AV_RB32(p - 4);
+    *start_code = AV_RB32(p - 4);
     // read the previous 4 bytes, i.e. bytes {p - 4, p - 3, p - 2, p - 1}
 
     return p;
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 06/13] avpriv_find_start_code(): correct type of start_code parameter

[Scott Theisen]

---
 libavcodec/mpeg12dec.c   | 2 +-
 libavformat/rtpenc_mpv.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 65b66d11f8..6b0b84ae64 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1770,7 +1770,7 @@ static int mpeg_decode_slice(MpegEncContext *s, int mb_y,
 
     if (avctx->hwaccel && avctx->hwaccel->decode_slice) {
         const uint8_t *buf_end, *buf_start = *buf - 4; /* include start_code */
-        int start_code = -1;
+        uint32_t start_code = ~0;
         buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code);
         if (buf_end < *buf + buf_size)
             buf_end -= 4;
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index 05a77fa11c..91c07574bb 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -51,11 +51,10 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
             end_of_slice = 1;
         } else {
             const uint8_t *r, *r1;
-            int start_code;
 
             r1 = buf1;
             while (1) {
-                start_code = -1;
+                uint32_t start_code = ~0;
                 r = avpriv_find_start_code(r1, end, &start_code);
                 if (avpriv_start_code_is_valid(start_code)) {
                     /* New start code found */
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 07/13] avpriv_find_start_code(): constify pointer parameters

[Scott Theisen]

Have the compiler enforce not changing the addresses these parameters point to.

No functional change.
---
 libavcodec/internal.h | 6 +++---
 libavcodec/utils.c    | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 94c41aef0b..dadd8d4a10 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -313,7 +313,7 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
  * @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 reference to a mutable @c uint32_t.<br>
+ * @param[in,out] start_code A constant pointer (reference) 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>
@@ -325,8 +325,8 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
  *         if no start code was found.
  */
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
-                                      const uint8_t *end,
-                                      uint32_t *start_code);
+                                      const uint8_t * const end,
+                                      uint32_t * const start_code);
 
 int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec);
 
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 80ccde023f..cf92d29f67 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -941,8 +941,8 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
 #endif
 
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
-                                      const uint8_t *end,
-                                      uint32_t *av_restrict start_code)
+                                      const uint8_t * const end,
+                                      uint32_t * const av_restrict start_code)
 {
     av_assert0(p <= end);
     if (p >= end)
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 07/13] avpriv_find_start_code(): constify pointer parameters

[Andreas Rheinhardt]

Scott Theisen:
> Have the compiler enforce not changing the addresses these parameters point to.
> 
> No functional change.
> ---
>  libavcodec/internal.h | 6 +++---
>  libavcodec/utils.c    | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 94c41aef0b..dadd8d4a10 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -313,7 +313,7 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
>   * @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 reference to a mutable @c uint32_t.<br>
> + * @param[in,out] start_code A constant pointer (reference) to a mutable @c uint32_t.<br>

There are no references in C.

>   *          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>
> @@ -325,8 +325,8 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
>   *         if no start code was found.
>   */
>  const uint8_t *avpriv_find_start_code(const uint8_t *p,
> -                                      const uint8_t *end,
> -                                      uint32_t *start_code);
> +                                      const uint8_t * const end,
> +                                      uint32_t * const start_code);
>  
>  int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec);
>  
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 80ccde023f..cf92d29f67 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -941,8 +941,8 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
>  #endif
>  
>  const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
> -                                      const uint8_t *end,
> -                                      uint32_t *av_restrict start_code)
> +                                      const uint8_t * const end,
> +                                      uint32_t * const av_restrict start_code)
>  {
>      av_assert0(p <= end);
>      if (p >= end)

Documenting restrictions on the callee that are irrelevant to the caller
in a public header seems weird.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 07/13] avpriv_find_start_code(): constify pointer parameters

[Scott Theisen]

On 2/5/22 00:49, Andreas Rheinhardt wrote:
>> - * @param[in,out] start_code A reference to a mutable @c uint32_t.<br>
>> + * @param[in,out] start_code A constant pointer (reference) to a mutable @c uint32_t.<br>
> There are no references in C.

A pointer is a type of reference.  However, I agree "A pointer to a 
mutable @c uint32_t.<br>" would be clearer.

> Documenting restrictions on the callee that are irrelevant to the caller
> in a public header seems weird.

Fair enough, the added consts shall be limited to libavcodec/utils.c.

-Scott
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 08/13] avpriv_find_start_code(): add parameter for ignoring history

[Scott Theisen]

If true, this skips the for loop at the beginning of avpriv_find_start_code().

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.

cbs_mpeg2.c needs further changes to use output_only = true.

Use output_only = 0 for no functional change.  For example, if the state/start_code
is passed into the function calling avpriv_find_start_code().
---
 libavcodec/cavsdec.c                  |  2 +-
 libavcodec/cbs_mpeg2.c                |  4 ++--
 libavcodec/extract_extradata_bsf.c    |  4 ++--
 libavcodec/h264_parser.c              |  2 +-
 libavcodec/internal.h                 |  9 ++++++++-
 libavcodec/mpeg12.c                   |  2 +-
 libavcodec/mpeg12dec.c                |  9 ++++-----
 libavcodec/mpeg4_unpack_bframes_bsf.c |  3 +--
 libavcodec/mpegvideo_parser.c         |  5 ++---
 libavcodec/remove_extradata_bsf.c     |  8 ++++----
 libavcodec/utils.c                    | 14 +++++++++++++-
 libavcodec/vc1_common.h               |  4 ++--
 libavformat/avidec.c                  |  6 +++---
 libavformat/avs2dec.c                 |  2 +-
 libavformat/avs3dec.c                 |  2 +-
 libavformat/cavsvideodec.c            |  2 +-
 libavformat/mpegtsenc.c               |  4 ++--
 libavformat/mpegvideodec.c            |  2 +-
 libavformat/mxfenc.c                  |  2 +-
 libavformat/rtpenc_mpv.c              |  4 ++--
 20 files changed, 53 insertions(+), 37 deletions(-)

diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index a62177d520..c4c78cd534 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -1248,7 +1248,7 @@ static int cavs_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
     buf_ptr = buf;
     buf_end = buf + buf_size;
     for(;;) {
-        buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
+        buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc, 1);
         if (!avpriv_start_code_is_valid(stc) || buf_ptr == buf_end) {
             if (!h->stc)
                 av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index eb45929132..d41477620e 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -151,7 +151,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
     int err, i, final = 0;
 
     start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
-                                   &start_code);
+                                   &start_code, 1);
     if (!avpriv_start_code_is_valid(start_code)) {
         // No start code found.
         return AVERROR_INVALIDDATA;
@@ -169,7 +169,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         }
 
         end = avpriv_find_start_code(start--, frag->data + frag->data_size,
-                                     &start_code);
+                                     &start_code, 0);
 
         // start points to the byte containing the start_code_identifier
         // (may be the last byte of fragment->data); end points to the byte
diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c
index 4df1c97139..1a3ddceff2 100644
--- a/libavcodec/extract_extradata_bsf.c
+++ b/libavcodec/extract_extradata_bsf.c
@@ -237,7 +237,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt,
     int has_extradata = 0, extradata_size = 0;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &state);
+        ptr = avpriv_find_start_code(ptr, end, &state, 1);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             has_extradata = 1;
         } else if (has_extradata && avpriv_start_code_is_valid(state)) {
@@ -300,7 +300,7 @@ static int extract_extradata_mpeg4(AVBSFContext *ctx, AVPacket *pkt,
     uint32_t state = UINT32_MAX;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &state);
+        ptr = avpriv_find_start_code(ptr, end, &state, 1);
         if (state == 0x1B3 || state == 0x1B6) {
             if (ptr - pkt->data > 4) {
                 *size = ptr - 4 - pkt->data;
diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 4002bcad77..0ca411c592 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -72,7 +72,7 @@ static int find_start_code(const uint8_t *buf, int buf_size,
 {
     uint32_t state = -1;
 
-    buf_index = avpriv_find_start_code(buf + buf_index, buf + next_avc + 1, &state) - buf - 1;
+    buf_index = avpriv_find_start_code(buf + buf_index, buf + next_avc + 1, &state, 1) - buf - 1;
 
     return FFMIN(buf_index, buf_size);
 }
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index dadd8d4a10..57e2816a95 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -309,6 +309,9 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
  * By preserving the <b>@p start_code</b> value between subsequent calls, the caller can
  * detect start codes across buffer boundaries.
  *
+ * If <b>@p output_only</b> is true, <b>@p start_code</b> is reset to <b>@c ~0 </b>
+ * if @f$ p - end < 4 @f$.
+ *
  * @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>.
@@ -321,12 +324,16 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
  *                     start code (the 4 bytes prior to the returned value,
  *                     using the input history if @f$ p - end < 4 @f$).
  *
+ * @param[in] output_only   Boolean that if true makes <b>@p start_code</b> an
+ *                          output only parameter.
+ *
  * @return A pointer to the memory address following the found start code, or <b>@p end</b>
  *         if no start code was found.
  */
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t * const end,
-                                      uint32_t * const start_code);
+                                      uint32_t * const start_code,
+                                      int output_only);
 
 int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec);
 
diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index e45bc74479..d788aeb9e2 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -203,7 +203,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 = avpriv_find_start_code(buf + i, buf + buf_size, &state, 0) - 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/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 6b0b84ae64..cad17d1b1a 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1771,7 +1771,7 @@ static int mpeg_decode_slice(MpegEncContext *s, int mb_y,
     if (avctx->hwaccel && avctx->hwaccel->decode_slice) {
         const uint8_t *buf_end, *buf_start = *buf - 4; /* include start_code */
         uint32_t start_code = ~0;
-        buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code);
+        buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code, 1);
         if (buf_end < *buf + buf_size)
             buf_end -= 4;
         s->mb_y = mb_y;
@@ -2020,8 +2020,7 @@ static int slice_decode_thread(AVCodecContext *c, void *arg)
         if (s->mb_y == s->end_mb_y)
             return 0;
 
-        start_code = -1;
-        buf        = avpriv_find_start_code(buf, s->gb.buffer_end, &start_code);
+        buf        = avpriv_find_start_code(buf, s->gb.buffer_end, &start_code, 1);
         if (start_code < SLICE_MIN_START_CODE || start_code > SLICE_MAX_START_CODE)
             return AVERROR_INVALIDDATA;
         mb_y       = start_code - SLICE_MIN_START_CODE;
@@ -2475,8 +2474,8 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
 
     for (;;) {
         /* find next start code */
-        uint32_t start_code = -1;
-        buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
+        uint32_t start_code;
+        buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code, 1);
         if (!avpriv_start_code_is_valid(start_code)) {
             if (!skip_frame) {
                 if (HAVE_THREADS &&
diff --git a/libavcodec/mpeg4_unpack_bframes_bsf.c b/libavcodec/mpeg4_unpack_bframes_bsf.c
index 6f8595713d..37fb0e4b66 100644
--- a/libavcodec/mpeg4_unpack_bframes_bsf.c
+++ b/libavcodec/mpeg4_unpack_bframes_bsf.c
@@ -36,8 +36,7 @@ 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);
+        pos = avpriv_find_start_code(pos, end, &startcode, 1);
 
         if (startcode == USER_DATA_STARTCODE && pos_p) {
             /* check if the (DivX) userdata string ends with 'p' (packed) */
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index f0897e7e2c..14e350eaea 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -68,7 +68,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 = avpriv_find_start_code(buf + i, buf + buf_size, &state, 0) - 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,8 +120,7 @@ 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);
+        buf = avpriv_find_start_code(buf, buf_end, &start_code, 1);
         bytes_left = buf_end - buf;
         switch(start_code) {
         case PICTURE_START_CODE:
diff --git a/libavcodec/remove_extradata_bsf.c b/libavcodec/remove_extradata_bsf.c
index 0e42174912..46e914053d 100644
--- a/libavcodec/remove_extradata_bsf.c
+++ b/libavcodec/remove_extradata_bsf.c
@@ -70,7 +70,7 @@ static int h264_split(const uint8_t *buf, int buf_size)
     int nalu_type;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &state);
+        ptr = avpriv_find_start_code(ptr, end, &state, 1);
         if (!avpriv_start_code_is_valid(state))
             break;
         nalu_type = state & 0x1F;
@@ -108,7 +108,7 @@ static int hevc_split(const uint8_t *buf, int buf_size)
     int nut;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &state);
+        ptr = avpriv_find_start_code(ptr, end, &state, 1);
         if (!avpriv_start_code_is_valid(state))
             break;
         nut = (state >> 1) & 0x3F;
@@ -151,7 +151,7 @@ static int mpeg4video_split(const uint8_t *buf, int buf_size)
     uint32_t state = -1;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &state);
+        ptr = avpriv_find_start_code(ptr, end, &state, 1);
         if (state == 0x1B3 || state == 0x1B6)
             return ptr - 4 - buf;
     }
@@ -166,7 +166,7 @@ static int vc1_split(const uint8_t *buf, int buf_size)
     int charged = 0;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &state);
+        ptr = avpriv_find_start_code(ptr, end, &state, 1);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             charged = 1;
         } else if (charged && avpriv_start_code_is_valid(state))
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index cf92d29f67..da057bad3e 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -942,9 +942,20 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
 
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t * const end,
-                                      uint32_t * const av_restrict start_code)
+                                      uint32_t * const av_restrict start_code,
+                                      const int output_only)
 {
     av_assert0(p <= end);
+    if (output_only) {
+        // minimum length for a start code
+        if (p + 4 > end) {
+            *start_code = ~0; // set to an invalid start code
+            return end;
+        }
+
+        p += 3; // offset for negative indices in while loop
+    }
+    else {
     if (p >= end)
         return end;
 
@@ -958,6 +969,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
             return p;
     }
     // p is now properly incremented for the negative indices in the 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 483f86a4ee..453dec34c6 100644
--- a/libavcodec/vc1_common.h
+++ b/libavcodec/vc1_common.h
@@ -57,8 +57,8 @@ 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;
-        src = avpriv_find_start_code(src, end, &mrk);
+        uint32_t mrk;
+        src = avpriv_find_start_code(src, end, &mrk, 1);
         if (avpriv_start_code_is_valid(mrk))
             return src - 4;
     }
diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index 86f857b1e3..674f313e84 100644
--- a/libavformat/avidec.c
+++ b/libavformat/avidec.c
@@ -1530,12 +1530,12 @@ resync:
                 if (index >= 0 && e->timestamp == ast->frame_offset) {
                     if (index == sti->nb_index_entries-1) {
                         int key=1;
-                        uint32_t state=-1;
                         if (st->codecpar->codec_id == AV_CODEC_ID_MPEG4) {
                             const uint8_t *ptr = pkt->data, *end = ptr + FFMIN(size, 256);
                             while (ptr < end) {
-                                ptr = avpriv_find_start_code(ptr, end, &state);
-                                if (state == 0x1B6 && ptr < end) {
+                                uint32_t start_code;
+                                ptr = avpriv_find_start_code(ptr, end, &start_code, 1);
+                                if (start_code == 0x1B6 && ptr < end) {
                                     key = !(*ptr & 0xC0);
                                     break;
                                 }
diff --git a/libavformat/avs2dec.c b/libavformat/avs2dec.c
index bdeb746fc7..f2349dde45 100644
--- a/libavformat/avs2dec.c
+++ b/libavformat/avs2dec.c
@@ -41,7 +41,7 @@ static int avs2_probe(const AVProbeData *p)
     }
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &code);
+        ptr = avpriv_find_start_code(ptr, end, &code, 1);
         if (avpriv_start_code_is_valid(code)) {
             state = code & 0xFF;
             if (AVS2_ISUNIT(state)) {
diff --git a/libavformat/avs3dec.c b/libavformat/avs3dec.c
index 2daccd3d15..2238193ff4 100644
--- a/libavformat/avs3dec.c
+++ b/libavformat/avs3dec.c
@@ -35,7 +35,7 @@ static int avs3video_probe(const AVProbeData *p)
     int ret = 0;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &code);
+        ptr = avpriv_find_start_code(ptr, end, &code, 1);
         if (avpriv_start_code_is_valid(code)) {
             state = code & 0xFF;
             if (state < AVS3_SEQ_START_CODE) {
diff --git a/libavformat/cavsvideodec.c b/libavformat/cavsvideodec.c
index 1d007708cc..8f4004acea 100644
--- a/libavformat/cavsvideodec.c
+++ b/libavformat/cavsvideodec.c
@@ -37,7 +37,7 @@ static int cavsvideo_probe(const AVProbeData *p)
     const uint8_t *ptr = p->buf, *end = p->buf + p->buf_size;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &code);
+        ptr = avpriv_find_start_code(ptr, end, &code, 1);
         if (avpriv_start_code_is_valid(code)) {
             if(code < CAVS_SEQ_START_CODE) {
                 /* slices have to be consecutive */
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 92b4cc8087..483c65be17 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -1881,7 +1881,7 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
             extradd = 0;
 
         do {
-            p = avpriv_find_start_code(p, buf_end, &state);
+            p = avpriv_find_start_code(p, buf_end, &state, 1);
             av_log(s, AV_LOG_TRACE, "nal %"PRId32"\n", state & 0x1f);
             if ((state & 0x1f) == 7)
                 extradd = 0;
@@ -1947,7 +1947,7 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
             extradd = 0;
 
         do {
-            p = avpriv_find_start_code(p, buf_end, &state);
+            p = avpriv_find_start_code(p, buf_end, &state, 1);
             av_log(s, AV_LOG_TRACE, "nal %"PRId32"\n", (state & 0x7e)>>1);
             if ((state & 0x7e) == 2*32)
                 extradd = 0;
diff --git a/libavformat/mpegvideodec.c b/libavformat/mpegvideodec.c
index a9829dc1df..840d3565af 100644
--- a/libavformat/mpegvideodec.c
+++ b/libavformat/mpegvideodec.c
@@ -43,7 +43,7 @@ static int mpegvideo_probe(const AVProbeData *p)
     int j;
 
     while (ptr < end) {
-        ptr = avpriv_find_start_code(ptr, end, &code);
+        ptr = avpriv_find_start_code(ptr, end, &code, 1);
         if (avpriv_start_code_is_valid(code)) {
             switch(code){
             case     SEQ_START_CODE:
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 5e068c8220..8cec44e89c 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -2242,7 +2242,7 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st,
     int i, frame_size, slice_type, has_sps = 0, intra_only = 0, ret;
 
     for (;;) {
-        buf = avpriv_find_start_code(buf, buf_end, &state);
+        buf = avpriv_find_start_code(buf, buf_end, &state, 1);
         if (buf >= buf_end)
             break;
 
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index 91c07574bb..3c7168c482 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -54,8 +54,8 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
 
             r1 = buf1;
             while (1) {
-                uint32_t start_code = ~0;
-                r = avpriv_find_start_code(r1, end, &start_code);
+                uint32_t start_code;
+                r = avpriv_find_start_code(r1, end, &start_code, 1);
                 if (avpriv_start_code_is_valid(start_code)) {
                     /* New start code found */
                     if (start_code == 0x100) {
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 08/13] avpriv_find_start_code(): add parameter for ignoring history

[Andreas Rheinhardt]

Scott Theisen:
> If true, this skips the for loop at the beginning of avpriv_find_start_code().
> 
> 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.
> 
> cbs_mpeg2.c needs further changes to use output_only = true.
> 
> Use output_only = 0 for no functional change.  For example, if the state/start_code
> is passed into the function calling avpriv_find_start_code().
> ---
>  libavcodec/cavsdec.c                  |  2 +-
>  libavcodec/cbs_mpeg2.c                |  4 ++--
>  libavcodec/extract_extradata_bsf.c    |  4 ++--
>  libavcodec/h264_parser.c              |  2 +-
>  libavcodec/internal.h                 |  9 ++++++++-
>  libavcodec/mpeg12.c                   |  2 +-
>  libavcodec/mpeg12dec.c                |  9 ++++-----
>  libavcodec/mpeg4_unpack_bframes_bsf.c |  3 +--
>  libavcodec/mpegvideo_parser.c         |  5 ++---
>  libavcodec/remove_extradata_bsf.c     |  8 ++++----
>  libavcodec/utils.c                    | 14 +++++++++++++-
>  libavcodec/vc1_common.h               |  4 ++--
>  libavformat/avidec.c                  |  6 +++---
>  libavformat/avs2dec.c                 |  2 +-
>  libavformat/avs3dec.c                 |  2 +-
>  libavformat/cavsvideodec.c            |  2 +-
>  libavformat/mpegtsenc.c               |  4 ++--
>  libavformat/mpegvideodec.c            |  2 +-
>  libavformat/mxfenc.c                  |  2 +-
>  libavformat/rtpenc_mpv.c              |  4 ++--
>  20 files changed, 53 insertions(+), 37 deletions(-)
> 
> diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
> index a62177d520..c4c78cd534 100644
> --- a/libavcodec/cavsdec.c
> +++ b/libavcodec/cavsdec.c
> @@ -1248,7 +1248,7 @@ static int cavs_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>      buf_ptr = buf;
>      buf_end = buf + buf_size;
>      for(;;) {
> -        buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
> +        buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc, 1);
>          if (!avpriv_start_code_is_valid(stc) || buf_ptr == buf_end) {
>              if (!h->stc)
>                  av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index eb45929132..d41477620e 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -151,7 +151,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>      int err, i, final = 0;
>  
>      start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
> -                                   &start_code);
> +                                   &start_code, 1);
>      if (!avpriv_start_code_is_valid(start_code)) {
>          // No start code found.
>          return AVERROR_INVALIDDATA;
> @@ -169,7 +169,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>          }
>  
>          end = avpriv_find_start_code(start--, frag->data + frag->data_size,
> -                                     &start_code);
> +                                     &start_code, 0);
>  
>          // start points to the byte containing the start_code_identifier
>          // (may be the last byte of fragment->data); end points to the byte
> diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c
> index 4df1c97139..1a3ddceff2 100644
> --- a/libavcodec/extract_extradata_bsf.c
> +++ b/libavcodec/extract_extradata_bsf.c
> @@ -237,7 +237,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt,
>      int has_extradata = 0, extradata_size = 0;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &state);
> +        ptr = avpriv_find_start_code(ptr, end, &state, 1);
>          if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
>              has_extradata = 1;
>          } else if (has_extradata && avpriv_start_code_is_valid(state)) {
> @@ -300,7 +300,7 @@ static int extract_extradata_mpeg4(AVBSFContext *ctx, AVPacket *pkt,
>      uint32_t state = UINT32_MAX;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &state);
> +        ptr = avpriv_find_start_code(ptr, end, &state, 1);
>          if (state == 0x1B3 || state == 0x1B6) {
>              if (ptr - pkt->data > 4) {
>                  *size = ptr - 4 - pkt->data;
> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
> index 4002bcad77..0ca411c592 100644
> --- a/libavcodec/h264_parser.c
> +++ b/libavcodec/h264_parser.c
> @@ -72,7 +72,7 @@ static int find_start_code(const uint8_t *buf, int buf_size,
>  {
>      uint32_t state = -1;
>  
> -    buf_index = avpriv_find_start_code(buf + buf_index, buf + next_avc + 1, &state) - buf - 1;
> +    buf_index = avpriv_find_start_code(buf + buf_index, buf + next_avc + 1, &state, 1) - buf - 1;
>  
>      return FFMIN(buf_index, buf_size);
>  }
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index dadd8d4a10..57e2816a95 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -309,6 +309,9 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
>   * By preserving the <b>@p start_code</b> value between subsequent calls, the caller can
>   * detect start codes across buffer boundaries.
>   *
> + * If <b>@p output_only</b> is true, <b>@p start_code</b> is reset to <b>@c ~0 </b>
> + * if @f$ p - end < 4 @f$.
> + *
>   * @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>.
> @@ -321,12 +324,16 @@ static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
>   *                     start code (the 4 bytes prior to the returned value,
>   *                     using the input history if @f$ p - end < 4 @f$).
>   *
> + * @param[in] output_only   Boolean that if true makes <b>@p start_code</b> an
> + *                          output only parameter.
> + *
>   * @return A pointer to the memory address following the found start code, or <b>@p end</b>
>   *         if no start code was found.
>   */
>  const uint8_t *avpriv_find_start_code(const uint8_t *p,
>                                        const uint8_t * const end,
> -                                      uint32_t * const start_code);
> +                                      uint32_t * const start_code,
> +                                      int output_only);
>  
>  int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec);
>  
> diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
> index e45bc74479..d788aeb9e2 100644
> --- a/libavcodec/mpeg12.c
> +++ b/libavcodec/mpeg12.c
> @@ -203,7 +203,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 = avpriv_find_start_code(buf + i, buf + buf_size, &state, 0) - 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/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 6b0b84ae64..cad17d1b1a 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -1771,7 +1771,7 @@ static int mpeg_decode_slice(MpegEncContext *s, int mb_y,
>      if (avctx->hwaccel && avctx->hwaccel->decode_slice) {
>          const uint8_t *buf_end, *buf_start = *buf - 4; /* include start_code */
>          uint32_t start_code = ~0;
> -        buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code);
> +        buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code, 1);
>          if (buf_end < *buf + buf_size)
>              buf_end -= 4;
>          s->mb_y = mb_y;
> @@ -2020,8 +2020,7 @@ static int slice_decode_thread(AVCodecContext *c, void *arg)
>          if (s->mb_y == s->end_mb_y)
>              return 0;
>  
> -        start_code = -1;
> -        buf        = avpriv_find_start_code(buf, s->gb.buffer_end, &start_code);
> +        buf        = avpriv_find_start_code(buf, s->gb.buffer_end, &start_code, 1);
>          if (start_code < SLICE_MIN_START_CODE || start_code > SLICE_MAX_START_CODE)
>              return AVERROR_INVALIDDATA;
>          mb_y       = start_code - SLICE_MIN_START_CODE;
> @@ -2475,8 +2474,8 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
>  
>      for (;;) {
>          /* find next start code */
> -        uint32_t start_code = -1;
> -        buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
> +        uint32_t start_code;
> +        buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code, 1);
>          if (!avpriv_start_code_is_valid(start_code)) {
>              if (!skip_frame) {
>                  if (HAVE_THREADS &&
> diff --git a/libavcodec/mpeg4_unpack_bframes_bsf.c b/libavcodec/mpeg4_unpack_bframes_bsf.c
> index 6f8595713d..37fb0e4b66 100644
> --- a/libavcodec/mpeg4_unpack_bframes_bsf.c
> +++ b/libavcodec/mpeg4_unpack_bframes_bsf.c
> @@ -36,8 +36,7 @@ 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);
> +        pos = avpriv_find_start_code(pos, end, &startcode, 1);
>  
>          if (startcode == USER_DATA_STARTCODE && pos_p) {
>              /* check if the (DivX) userdata string ends with 'p' (packed) */
> diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
> index f0897e7e2c..14e350eaea 100644
> --- a/libavcodec/mpegvideo_parser.c
> +++ b/libavcodec/mpegvideo_parser.c
> @@ -68,7 +68,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 = avpriv_find_start_code(buf + i, buf + buf_size, &state, 0) - 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,8 +120,7 @@ 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);
> +        buf = avpriv_find_start_code(buf, buf_end, &start_code, 1);
>          bytes_left = buf_end - buf;
>          switch(start_code) {
>          case PICTURE_START_CODE:
> diff --git a/libavcodec/remove_extradata_bsf.c b/libavcodec/remove_extradata_bsf.c
> index 0e42174912..46e914053d 100644
> --- a/libavcodec/remove_extradata_bsf.c
> +++ b/libavcodec/remove_extradata_bsf.c
> @@ -70,7 +70,7 @@ static int h264_split(const uint8_t *buf, int buf_size)
>      int nalu_type;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &state);
> +        ptr = avpriv_find_start_code(ptr, end, &state, 1);
>          if (!avpriv_start_code_is_valid(state))
>              break;
>          nalu_type = state & 0x1F;
> @@ -108,7 +108,7 @@ static int hevc_split(const uint8_t *buf, int buf_size)
>      int nut;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &state);
> +        ptr = avpriv_find_start_code(ptr, end, &state, 1);
>          if (!avpriv_start_code_is_valid(state))
>              break;
>          nut = (state >> 1) & 0x3F;
> @@ -151,7 +151,7 @@ static int mpeg4video_split(const uint8_t *buf, int buf_size)
>      uint32_t state = -1;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &state);
> +        ptr = avpriv_find_start_code(ptr, end, &state, 1);
>          if (state == 0x1B3 || state == 0x1B6)
>              return ptr - 4 - buf;
>      }
> @@ -166,7 +166,7 @@ static int vc1_split(const uint8_t *buf, int buf_size)
>      int charged = 0;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &state);
> +        ptr = avpriv_find_start_code(ptr, end, &state, 1);
>          if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
>              charged = 1;
>          } else if (charged && avpriv_start_code_is_valid(state))
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index cf92d29f67..da057bad3e 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -942,9 +942,20 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
>  
>  const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
>                                        const uint8_t * const end,
> -                                      uint32_t * const av_restrict start_code)
> +                                      uint32_t * const av_restrict start_code,
> +                                      const int output_only)
>  {
>      av_assert0(p <= end);
> +    if (output_only) {
> +        // minimum length for a start code
> +        if (p + 4 > end) {
> +            *start_code = ~0; // set to an invalid start code
> +            return end;
> +        }
> +
> +        p += 3; // offset for negative indices in while loop
> +    }
> +    else {
>      if (p >= end)
>          return end;
>  
> @@ -958,6 +969,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
>              return p;
>      }
>      // p is now properly incremented for the negative indices in the 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 483f86a4ee..453dec34c6 100644
> --- a/libavcodec/vc1_common.h
> +++ b/libavcodec/vc1_common.h
> @@ -57,8 +57,8 @@ 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;
> -        src = avpriv_find_start_code(src, end, &mrk);
> +        uint32_t mrk;
> +        src = avpriv_find_start_code(src, end, &mrk, 1);
>          if (avpriv_start_code_is_valid(mrk))
>              return src - 4;
>      }
> diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> index 86f857b1e3..674f313e84 100644
> --- a/libavformat/avidec.c
> +++ b/libavformat/avidec.c
> @@ -1530,12 +1530,12 @@ resync:
>                  if (index >= 0 && e->timestamp == ast->frame_offset) {
>                      if (index == sti->nb_index_entries-1) {
>                          int key=1;
> -                        uint32_t state=-1;
>                          if (st->codecpar->codec_id == AV_CODEC_ID_MPEG4) {
>                              const uint8_t *ptr = pkt->data, *end = ptr + FFMIN(size, 256);
>                              while (ptr < end) {
> -                                ptr = avpriv_find_start_code(ptr, end, &state);
> -                                if (state == 0x1B6 && ptr < end) {
> +                                uint32_t start_code;
> +                                ptr = avpriv_find_start_code(ptr, end, &start_code, 1);
> +                                if (start_code == 0x1B6 && ptr < end) {
>                                      key = !(*ptr & 0xC0);
>                                      break;
>                                  }
> diff --git a/libavformat/avs2dec.c b/libavformat/avs2dec.c
> index bdeb746fc7..f2349dde45 100644
> --- a/libavformat/avs2dec.c
> +++ b/libavformat/avs2dec.c
> @@ -41,7 +41,7 @@ static int avs2_probe(const AVProbeData *p)
>      }
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &code);
> +        ptr = avpriv_find_start_code(ptr, end, &code, 1);
>          if (avpriv_start_code_is_valid(code)) {
>              state = code & 0xFF;
>              if (AVS2_ISUNIT(state)) {
> diff --git a/libavformat/avs3dec.c b/libavformat/avs3dec.c
> index 2daccd3d15..2238193ff4 100644
> --- a/libavformat/avs3dec.c
> +++ b/libavformat/avs3dec.c
> @@ -35,7 +35,7 @@ static int avs3video_probe(const AVProbeData *p)
>      int ret = 0;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &code);
> +        ptr = avpriv_find_start_code(ptr, end, &code, 1);
>          if (avpriv_start_code_is_valid(code)) {
>              state = code & 0xFF;
>              if (state < AVS3_SEQ_START_CODE) {
> diff --git a/libavformat/cavsvideodec.c b/libavformat/cavsvideodec.c
> index 1d007708cc..8f4004acea 100644
> --- a/libavformat/cavsvideodec.c
> +++ b/libavformat/cavsvideodec.c
> @@ -37,7 +37,7 @@ static int cavsvideo_probe(const AVProbeData *p)
>      const uint8_t *ptr = p->buf, *end = p->buf + p->buf_size;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &code);
> +        ptr = avpriv_find_start_code(ptr, end, &code, 1);
>          if (avpriv_start_code_is_valid(code)) {
>              if(code < CAVS_SEQ_START_CODE) {
>                  /* slices have to be consecutive */
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index 92b4cc8087..483c65be17 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -1881,7 +1881,7 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
>              extradd = 0;
>  
>          do {
> -            p = avpriv_find_start_code(p, buf_end, &state);
> +            p = avpriv_find_start_code(p, buf_end, &state, 1);
>              av_log(s, AV_LOG_TRACE, "nal %"PRId32"\n", state & 0x1f);
>              if ((state & 0x1f) == 7)
>                  extradd = 0;
> @@ -1947,7 +1947,7 @@ static int mpegts_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
>              extradd = 0;
>  
>          do {
> -            p = avpriv_find_start_code(p, buf_end, &state);
> +            p = avpriv_find_start_code(p, buf_end, &state, 1);
>              av_log(s, AV_LOG_TRACE, "nal %"PRId32"\n", (state & 0x7e)>>1);
>              if ((state & 0x7e) == 2*32)
>                  extradd = 0;
> diff --git a/libavformat/mpegvideodec.c b/libavformat/mpegvideodec.c
> index a9829dc1df..840d3565af 100644
> --- a/libavformat/mpegvideodec.c
> +++ b/libavformat/mpegvideodec.c
> @@ -43,7 +43,7 @@ static int mpegvideo_probe(const AVProbeData *p)
>      int j;
>  
>      while (ptr < end) {
> -        ptr = avpriv_find_start_code(ptr, end, &code);
> +        ptr = avpriv_find_start_code(ptr, end, &code, 1);
>          if (avpriv_start_code_is_valid(code)) {
>              switch(code){
>              case     SEQ_START_CODE:
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 5e068c8220..8cec44e89c 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -2242,7 +2242,7 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st,
>      int i, frame_size, slice_type, has_sps = 0, intra_only = 0, ret;
>  
>      for (;;) {
> -        buf = avpriv_find_start_code(buf, buf_end, &state);
> +        buf = avpriv_find_start_code(buf, buf_end, &state, 1);
>          if (buf >= buf_end)
>              break;
>  
> diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
> index 91c07574bb..3c7168c482 100644
> --- a/libavformat/rtpenc_mpv.c
> +++ b/libavformat/rtpenc_mpv.c
> @@ -54,8 +54,8 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
>  
>              r1 = buf1;
>              while (1) {
> -                uint32_t start_code = ~0;
> -                r = avpriv_find_start_code(r1, end, &start_code);
> +                uint32_t start_code;
> +                r = avpriv_find_start_code(r1, end, &start_code, 1);
>                  if (avpriv_start_code_is_valid(start_code)) {
>                      /* New start code found */
>                      if (start_code == 0x100) {

1. This patch is absolutely unacceptable: It breaks ABI.
2. I am surprised that there is apparently only one user that actually
wants this feature of state being an input parameter, namely
(ff_)mpeg1_find_frame_end(). This means that this loop done before the
new check should actually be made by this caller alone, obviating the
need to change the signature.
3. Given that no user of this in libavformat wants this feature,
changing it is IMO acceptable from an ABI-point of view.
4. There might be some slight changes introduced by this though:
Consider 00 00 01 00 00 01 xy. If the initial state is -1, a call to
avpriv_find_start_code() will treat the initial four bytes as start code
and return a pointer to the byte preceding the second 0x01. If the user
does not reset the start code between calls to avpriv_find_start_code(),
the second call will combine the last zero byte of the start code with
the rest to form another start code that partially overlaps with the
earlier one. This is (probably) invalid data (definitely for H.262,
H.264 and HEVC).

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 08/13] avpriv_find_start_code(): add parameter for ignoring history

[Scott Theisen]

On 2/5/22 01:10, Andreas Rheinhardt wrote:
> 1. This patch is absolutely unacceptable: It breaks ABI.
> 2. I am surprised that there is apparently only one user that actually
> wants this feature of state being an input parameter, namely
> (ff_)mpeg1_find_frame_end(). This means that this loop done before the
> new check should actually be made by this caller alone, obviating the
> need to change the signature.
> 3. Given that no user of this in libavformat wants this feature,
> changing it is IMO acceptable from an ABI-point of view.

I'll look into it.

> 4. There might be some slight changes introduced by this though:
> Consider 00 00 01 00 00 01 xy. If the initial state is -1, a call to
> avpriv_find_start_code() will treat the initial four bytes as start code
> and return a pointer to the byte preceding the second 0x01. If the user
> does not reset the start code between calls to avpriv_find_start_code(),
> the second call will combine the last zero byte of the start code with
> the rest to form another start code that partially overlaps with the
> earlier one. This is (probably) invalid data (definitely for H.262,
> H.264 and HEVC).

With input buffer 00 00 01 00 00 01 xy ...
The code in master will (incorrectly) find a second start code at offset 7.
It would need to check if (*start_code == 0x100) and invalidate it.

-Scott

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 09/13] avpriv_find_start_code(): fix indent from previous commit

[Scott Theisen]

Whitespace change only.  Separate so the previous diff is clearer.
---
 libavcodec/utils.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index da057bad3e..255c133737 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -956,19 +956,19 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
         p += 3; // offset for negative indices in while loop
     }
     else {
-    if (p >= end)
-        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 (avpriv_start_code_is_valid(*start_code) || p == end)
-            return p;
-    }
-    // p is now properly incremented for the negative indices in the while loop
+        if (p >= end)
+            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 (avpriv_start_code_is_valid(*start_code) || p == end)
+                return p;
+        }
+        // p is now properly incremented for the negative indices in the while loop
     }
 
     /* with memory address increasing left to right, we are looking for (in hexadecimal):
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop

[Scott Theisen]

This enhances the clarity of the code.
---
 libavcodec/cbs_mpeg2.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index d41477620e..afe78eef9a 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -148,7 +148,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
     CodedBitstreamUnitType unit_type;
     uint32_t start_code = -1;
     size_t unit_size;
-    int err, i, final = 0;
+    int err, i = 0, final = 0;
 
     start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
                                    &start_code, 1);
@@ -157,7 +157,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         return AVERROR_INVALIDDATA;
     }
 
-    for (i = 0;; i++) {
+    while (!final) {
         unit_type = start_code & 0xff;
 
         if (start == frag->data + frag->data_size) {
@@ -190,10 +190,8 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         if (err < 0)
             return err;
 
-        if (final)
-            break;
-
         start = end;
+        i++;
     }
 
     return 0;
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop

[Andreas Rheinhardt]

Scott Theisen:
> This enhances the clarity of the code.
> ---
>  libavcodec/cbs_mpeg2.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index d41477620e..afe78eef9a 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -148,7 +148,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>      CodedBitstreamUnitType unit_type;
>      uint32_t start_code = -1;
>      size_t unit_size;
> -    int err, i, final = 0;
> +    int err, i = 0, final = 0;
>  
>      start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
>                                     &start_code, 1);
> @@ -157,7 +157,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>          return AVERROR_INVALIDDATA;
>      }
>  
> -    for (i = 0;; i++) {
> +    while (!final) {
>          unit_type = start_code & 0xff;
>  
>          if (start == frag->data + frag->data_size) {
> @@ -190,10 +190,8 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>          if (err < 0)
>              return err;
>  
> -        if (final)
> -            break;
> -
>          start = end;
> +        i++;
>      }
>  
>      return 0;

I disagree that this enhances clarity: When using a counter, a for loop
is the most natural.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop

[Scott Theisen]

On 2/4/22 20:48, Andreas Rheinhardt wrote:
> I disagree that this enhances clarity: When using a counter, a for loop
> is the most natural.

The counter is not used as the loop condition, so it is not natural.  
Also, you removed the counter and made this a do while loop in your own 
patch series.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop

[Andreas Rheinhardt]

Scott Theisen:
> On 2/4/22 20:48, Andreas Rheinhardt wrote:
>> I disagree that this enhances clarity: When using a counter, a for loop
>> is the most natural.
> 
> The counter is not used as the loop condition, so it is not natural. 
> Also, you removed the counter and made this a do while loop in your own
> patch series.

Yes, after the counter has been removed, using a for-loop stopped being
natural.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop

[Scott Theisen]

On 2/4/22 23:06, Andreas Rheinhardt wrote:
> Scott Theisen:
>> On 2/4/22 20:48, Andreas Rheinhardt wrote:
>>> I disagree that this enhances clarity: When using a counter, a for loop
>>> is the most natural.
>> The counter is not used as the loop condition, so it is not natural.
>> Also, you removed the counter and made this a do while loop in your own
>> patch series.
> Yes, after the counter has been removed, using a for-loop stopped being
> natural.

It is not really a for-loop, though, since it has no loop condition and 
is thus an infinite loop that just happens to be abusing the for-loop 
syntax.

-Scott
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 11/13] cbs_mpeg2.c: improve readability of start code search (part 1)

[Scott Theisen]

ff_cbs_insert_unit_data() does not modify the data or data_size fields of
the CodedBitstreamFragment.  It also does not modify the value pointed to
by start.  (We don't need that byte in this function anymore, anyway.)
---
 libavcodec/cbs_mpeg2.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index afe78eef9a..f2efedde5d 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -144,23 +144,24 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
                                     CodedBitstreamFragment *frag,
                                     int header)
 {
-    const uint8_t *start, *end;
+    const uint8_t *start = frag->data, *end;
+    const uint8_t * const buf_end = frag->data + frag->data_size;
     CodedBitstreamUnitType unit_type;
     uint32_t start_code = -1;
     size_t unit_size;
-    int err, i = 0, final = 0;
+    int err, final = 0;
+    int i = -1; // offset for pre-increment
 
-    start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
-                                   &start_code, 1);
+    start = avpriv_find_start_code(start, buf_end, &start_code, 1);
     if (!avpriv_start_code_is_valid(start_code)) {
         // No start code found.
         return AVERROR_INVALIDDATA;
     }
 
-    while (!final) {
+    do {
         unit_type = start_code & 0xff;
 
-        if (start == frag->data + frag->data_size) {
+        if (start == buf_end) {
             // The last four bytes form a start code which constitutes
             // a unit of its own.  In this situation avpriv_find_start_code
             // won't modify start_code at all so modify start_code so that
@@ -168,10 +169,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
             start_code = 0;
         }
 
-        end = avpriv_find_start_code(start--, frag->data + frag->data_size,
-                                     &start_code, 0);
-
-        // start points to the byte containing the start_code_identifier
+        end = avpriv_find_start_code(start, buf_end, &start_code, 0);
+        start--;
+        // decrement so start points to the byte containing the start_code_identifier
         // (may be the last byte of fragment->data); end points to the byte
         // following the byte containing the start code identifier (or to
         // the end of fragment->data).
@@ -185,14 +185,14 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
            final     = 1;
         }
 
-        err = ff_cbs_insert_unit_data(frag, i, unit_type, (uint8_t*)start,
+        err = ff_cbs_insert_unit_data(frag, ++i, unit_type,
+                                      (uint8_t*)start /* cast away the const to match parameter type */,
                                       unit_size, frag->data_ref);
         if (err < 0)
             return err;
 
         start = end;
-        i++;
-    }
+    } while (!final);
 
     return 0;
 }
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 11/13] cbs_mpeg2.c: improve readability of start code search (part 1)

[Andreas Rheinhardt]

Scott Theisen:
> ff_cbs_insert_unit_data() does not modify the data or data_size fields of
> the CodedBitstreamFragment.  It also does not modify the value pointed to
> by start.  (We don't need that byte in this function anymore, anyway.)
> ---
>  libavcodec/cbs_mpeg2.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index afe78eef9a..f2efedde5d 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -144,23 +144,24 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>                                      CodedBitstreamFragment *frag,
>                                      int header)
>  {
> -    const uint8_t *start, *end;
> +    const uint8_t *start = frag->data, *end;
> +    const uint8_t * const buf_end = frag->data + frag->data_size;
>      CodedBitstreamUnitType unit_type;
>      uint32_t start_code = -1;
>      size_t unit_size;
> -    int err, i = 0, final = 0;
> +    int err, final = 0;
> +    int i = -1; // offset for pre-increment

Using a pre-increment is unnatural (i is supposed to be the number of
units of the fragment and so it should naturally be incremented after a
unit has been successfully added to the fragment) and impairs clarity.

>  
> -    start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
> -                                   &start_code, 1);
> +    start = avpriv_find_start_code(start, buf_end, &start_code, 1);
>      if (!avpriv_start_code_is_valid(start_code)) {
>          // No start code found.
>          return AVERROR_INVALIDDATA;
>      }
>  
> -    while (!final) {
> +    do {
>          unit_type = start_code & 0xff;
>  
> -        if (start == frag->data + frag->data_size) {
> +        if (start == buf_end) {
>              // The last four bytes form a start code which constitutes
>              // a unit of its own.  In this situation avpriv_find_start_code
>              // won't modify start_code at all so modify start_code so that
> @@ -168,10 +169,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>              start_code = 0;
>          }
>  
> -        end = avpriv_find_start_code(start--, frag->data + frag->data_size,
> -                                     &start_code, 0);
> -
> -        // start points to the byte containing the start_code_identifier
> +        end = avpriv_find_start_code(start, buf_end, &start_code, 0);
> +        start--;
> +        // decrement so start points to the byte containing the start_code_identifier
>          // (may be the last byte of fragment->data); end points to the byte
>          // following the byte containing the start code identifier (or to
>          // the end of fragment->data).
> @@ -185,14 +185,14 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>             final     = 1;
>          }
>  
> -        err = ff_cbs_insert_unit_data(frag, i, unit_type, (uint8_t*)start,
> +        err = ff_cbs_insert_unit_data(frag, ++i, unit_type,
> +                                      (uint8_t*)start /* cast away the const to match parameter type */,

redundant comment

>                                        unit_size, frag->data_ref);
>          if (err < 0)
>              return err;
>  
>          start = end;
> -        i++;
> -    }
> +    } while (!final);
>  
>      return 0;
>  }

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 11/13] cbs_mpeg2.c: improve readability of start code search (part 1)

[Scott Theisen]

On 2/4/22 21:16, Andreas Rheinhardt wrote:
> Using a pre-increment is unnatural (i is supposed to be the number of
> units of the fragment and so it should naturally be incremented after a
> unit has been successfully added to the fragment) and impairs clarity.
>

It *is* incremented after a unit has been successfully added, on the 
*next* iteration.

> redundant comment
>

Debatable, the variable is already a uint8_t*, albeit const qualified.  
The purpose of comments is to explain.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 12/13] cbs_mpeg2.c: improve readability of start code search (part 2)

[Scott Theisen]

If the last four bytes form a valid start code, the loop would run another time.
Since (start == buf_end), start_code is invalidated so the loop terminates.

However, calling avpriv_find_start_code() with p == end, it will immediately
return due to the zero size buffer.  Thus the history is not needed.
---
 libavcodec/cbs_mpeg2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index f2efedde5d..bf95fb7546 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -168,8 +168,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
             // the next unit will be treated as the last unit.
             start_code = 0;
         }
-
-        end = avpriv_find_start_code(start, buf_end, &start_code, 0);
+        else {
+            end = avpriv_find_start_code(start, buf_end, &start_code, 1);
+        }
         start--;
         // decrement so start points to the byte containing the start_code_identifier
         // (may be the last byte of fragment->data); end points to the byte
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 12/13] cbs_mpeg2.c: improve readability of start code search (part 2)

[Andreas Rheinhardt]

Scott Theisen:
> If the last four bytes form a valid start code, the loop would run another time.
> Since (start == buf_end), start_code is invalidated so the loop terminates.
> 
> However, calling avpriv_find_start_code() with p == end, it will immediately
> return due to the zero size buffer.  Thus the history is not needed.
> ---
>  libavcodec/cbs_mpeg2.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index f2efedde5d..bf95fb7546 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -168,8 +168,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>              // the next unit will be treated as the last unit.
>              start_code = 0;
>          }
> -
> -        end = avpriv_find_start_code(start, buf_end, &start_code, 0);
> +        else {
> +            end = avpriv_find_start_code(start, buf_end, &start_code, 1);
> +        }
>          start--;
>          // decrement so start points to the byte containing the start_code_identifier
>          // (may be the last byte of fragment->data); end points to the byte

end is now uninitialized in case of a unit consisting solely of a
Sequence End.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 12/13] cbs_mpeg2.c: improve readability of start code search (part 2)

[Scott Theisen]

On 2/4/22 21:24, Andreas Rheinhardt wrote:
> end is now uninitialized in case of a unit consisting solely of a
> Sequence End.

I hadn't noticed that.  However, I immediately restored it in the next 
patch.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 13/13] cbs_mpeg2.c: improve readability of start code search (part 3)

[Scott Theisen]

Separate from part 2 for a clearer diff.

Now the true loop condition has been revealed: start < buf_end

Clarify loop by moving the detection of sequence_end_codes out of the loop.
See also commit fd93d5efe64206d5f1bce8c702602353444c0c1a regarding sequence_end_codes
---
 libavcodec/cbs_mpeg2.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index bf95fb7546..53aa0ed3f6 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -149,7 +149,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
     CodedBitstreamUnitType unit_type;
     uint32_t start_code = -1;
     size_t unit_size;
-    int err, final = 0;
+    int err;
     int i = -1; // offset for pre-increment
 
     start = avpriv_find_start_code(start, buf_end, &start_code, 1);
@@ -161,16 +161,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
     do {
         unit_type = start_code & 0xff;
 
-        if (start == buf_end) {
-            // The last four bytes form a start code which constitutes
-            // a unit of its own.  In this situation avpriv_find_start_code
-            // won't modify start_code at all so modify start_code so that
-            // the next unit will be treated as the last unit.
-            start_code = 0;
-        }
-        else {
-            end = avpriv_find_start_code(start, buf_end, &start_code, 1);
-        }
+        end = avpriv_find_start_code(start, buf_end, &start_code, 1);
         start--;
         // decrement so start points to the byte containing the start_code_identifier
         // (may be the last byte of fragment->data); end points to the byte
@@ -182,8 +173,8 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
             unit_size = (end - 4) - start;
         } else {
            // We didn't find a start code, so this is the final unit.
+           // There is no start code to remove from end, hence not (end - 4).
            unit_size = end - start;
-           final     = 1;
         }
 
         err = ff_cbs_insert_unit_data(frag, ++i, unit_type,
@@ -193,7 +184,21 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
             return err;
 
         start = end;
-    } while (!final);
+    } while (start < buf_end);
+
+    if (avpriv_start_code_is_valid(start_code)) {
+        // The last four bytes form a start code which constitutes
+        // a unit of its own, with size 1.
+
+        start--; // since start == buf_end because of the loop condition,
+        // decrement so start points to the byte containing the start_code_identifier
+        err = ff_cbs_insert_unit_data(frag, ++i, start_code & 0xFF,
+                                      (uint8_t*)start /* cast away the const to match parameter type */,
+                                      1, frag->data_ref);
+        if (err < 0) {
+            return err;
+        }
+    }
 
     return 0;
 }
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 13/13] cbs_mpeg2.c: improve readability of start code search (part 3)

[Andreas Rheinhardt]

Scott Theisen:
> Separate from part 2 for a clearer diff.
> 
> Now the true loop condition has been revealed: start < buf_end
> 
> Clarify loop by moving the detection of sequence_end_codes out of the loop.
> See also commit fd93d5efe64206d5f1bce8c702602353444c0c1a regarding sequence_end_codes
> ---
>  libavcodec/cbs_mpeg2.c | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index bf95fb7546..53aa0ed3f6 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -149,7 +149,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>      CodedBitstreamUnitType unit_type;
>      uint32_t start_code = -1;
>      size_t unit_size;
> -    int err, final = 0;
> +    int err;
>      int i = -1; // offset for pre-increment
>  
>      start = avpriv_find_start_code(start, buf_end, &start_code, 1);
> @@ -161,16 +161,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>      do {
>          unit_type = start_code & 0xff;
>  
> -        if (start == buf_end) {
> -            // The last four bytes form a start code which constitutes
> -            // a unit of its own.  In this situation avpriv_find_start_code
> -            // won't modify start_code at all so modify start_code so that
> -            // the next unit will be treated as the last unit.
> -            start_code = 0;
> -        }
> -        else {
> -            end = avpriv_find_start_code(start, buf_end, &start_code, 1);
> -        }
> +        end = avpriv_find_start_code(start, buf_end, &start_code, 1);
>          start--;
>          // decrement so start points to the byte containing the start_code_identifier
>          // (may be the last byte of fragment->data); end points to the byte
> @@ -182,8 +173,8 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>              unit_size = (end - 4) - start;
>          } else {
>             // We didn't find a start code, so this is the final unit.
> +           // There is no start code to remove from end, hence not (end - 4).
>             unit_size = end - start;
> -           final     = 1;
>          }
>  
>          err = ff_cbs_insert_unit_data(frag, ++i, unit_type,
> @@ -193,7 +184,21 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
>              return err;
>  
>          start = end;
> -    } while (!final);
> +    } while (start < buf_end);
> +
> +    if (avpriv_start_code_is_valid(start_code)) {
> +        // The last four bytes form a start code which constitutes
> +        // a unit of its own, with size 1.
> +
> +        start--; // since start == buf_end because of the loop condition,
> +        // decrement so start points to the byte containing the start_code_identifier
> +        err = ff_cbs_insert_unit_data(frag, ++i, start_code & 0xFF,
> +                                      (uint8_t*)start /* cast away the const to match parameter type */,
> +                                      1, frag->data_ref);
> +        if (err < 0) {
> +            return err;
> +        }
> +    }
>  
>      return 0;
>  }

I disagree that this is the true loop condition that just needs to be
revealed; in fact, this is basically the loop condition from before
fd93d5efe64206d5f1bce8c702602353444c0c1a, just with an added block to
deal with size one units at the end. I considered this and chose the
current one because it leads to less code duplication for this special case.
Anyway, now that I have taken another look at this and I finally found
the true loop condition: Is there a unit that we have not added to the
fragment yet? This is easily implementable if one always resets the
start code, so that there being an outstanding unit is equivalent to the
start code being valid.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 13/13] cbs_mpeg2.c: improve readability of start code search (part 3)

[Scott Theisen]

On 2/4/22 22:54, Andreas Rheinhardt wrote:
> I disagree that this is the true loop condition that just needs to be
> revealed; in fact, this is basically the loop condition from before
> fd93d5efe64206d5f1bce8c702602353444c0c1a, just with an added block to
> deal with size one units at the end. I considered this and chose the
> current one because it leads to less code duplication for this special case.
> Anyway, now that I have taken another look at this and I finally found
> the true loop condition: Is there a unit that we have not added to the
> fragment yet? This is easily implementable if one always resets the
> start code, so that there being an outstanding unit is equivalent to the
> start code being valid.

Looking at your patch series again, I agree your changes to cbs_mpeg2.c 
are clearer.

However, I think my changes from patch 11 are a further helpful 
clarification (ignoring the index and loop changes (since you already 
did that) and the "redundant" comment):

@@ -144,23 +144,24 @@  static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
                                      CodedBitstreamFragment *frag,
                                      int header)
  {
- const uint8_t *start, *end;
+ const uint8_t *start = frag->data, *end;
+ const uint8_t * const buf_end = frag->data + frag->data_size;
      CodedBitstreamUnitType unit_type;
      uint32_t start_code = -1;
      size_t unit_size;
- int err, i = 0, final = 0;
+ int err, final = 0;
+ int i = -1; // offset for pre-increment
  
- start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
- &start_code, 1);
+ start = avpriv_find_start_code(start, buf_end, &start_code, 1);
      if (!avpriv_start_code_is_valid(start_code)) {
          // No start code found.
          return AVERROR_INVALIDDATA;
      }
  
- while (!final) {
+ do {
          unit_type = start_code & 0xff;
  
- if (start == frag->data + frag->data_size) {
+ if (start == buf_end) {
              // The last four bytes form a start code which constitutes
              // a unit of its own.  In this situation avpriv_find_start_code
              // won't modify start_code at all so modify start_code so that
@@ -168,10 +169,9 @@  static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
              start_code = 0;
          }
  
- end = avpriv_find_start_code(start--, frag->data + frag->data_size,
- &start_code, 0);
-
- // start points to the byte containing the start_code_identifier
+ end = avpriv_find_start_code(start, buf_end, &start_code, 0);
+ start--;
+ // decrement so start points to the byte containing the 
start_code_identifier
          // (may be the last byte of fragment->data); end points to the byte
          // following the byte containing the start code identifier (or to
          // the end of fragment->data).



Should I submit a v3 patch series which only includes patches 1-9? (That 
is only the avpriv_find_start_code() changes, since 10-13 were 
cbs_mpeg2.c and separate but related.)

Regards,
Scott

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 0/8] rewrite avpriv_find_start_code() for clarity

[Scott Theisen]

I have removed most of my added comments.

Rebased to account for Andreas Rheinhardt moving the prototype to
startcode.h.

Per his suggestion, the function signature is now unmodified.
(ff_)mpeg1_find_frame_end() now has a wrapper function preserving
the original behavior allowing start codes across buffer boundaries.

-Scott

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 1/8] avcodec/startcode.h: create start_code_is_valid()

[Scott Theisen]

This slightly changes what is considered valid in the following cases:
cavsdec.c:   0x000000XX is now considered invalid.
mpeg12dec.c: 0x000000XX is now considered invalid.
(where X is any value)

IS_MARKER is equivalent since VC1_CODE_RES0 = 0x00000100
---
 libavcodec/cavsdec.c               |  2 +-
 libavcodec/cbs_mpeg2.c             |  6 +++---
 libavcodec/extract_extradata_bsf.c |  2 +-
 libavcodec/mpeg12.c                |  2 +-
 libavcodec/mpeg12dec.c             |  2 +-
 libavcodec/mpegvideo_parser.c      |  2 +-
 libavcodec/remove_extradata_bsf.c  |  8 +++-----
 libavcodec/startcode.h             | 17 +++++++++++++++++
 libavcodec/vaapi_vc1.c             |  2 +-
 libavcodec/vc1_common.h            |  4 +---
 libavcodec/vc1dec.c                |  2 +-
 libavformat/avs2dec.c              |  4 ++--
 libavformat/avs3dec.c              |  4 ++--
 libavformat/cavsvideodec.c         |  2 +-
 libavformat/mpegvideodec.c         |  2 +-
 libavformat/rtpenc_mpv.c           |  2 +-
 16 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 894aa1b54a..75cf17e271 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -1250,7 +1250,7 @@ static int cavs_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
     buf_end = buf + buf_size;
     for(;;) {
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
-        if ((stc & 0xFFFFFE00) || buf_ptr == buf_end) {
+        if (!start_code_is_valid(stc) || buf_ptr == buf_end) {
             if (!h->stc)
                 av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
             return FFMAX(0, buf_ptr - buf);
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 33bd3e0998..f0a2265938 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -150,7 +150,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
 
     start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
                                    &start_code);
-    if (start_code >> 8 != 0x000001) {
+    if (!start_code_is_valid(start_code)) {
         // No start code found.
         return AVERROR_INVALIDDATA;
     }
@@ -172,7 +172,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         // (may be the last byte of fragment->data); end points to the byte
         // following the byte containing the start code identifier (or to
         // the end of fragment->data).
-        if (start_code >> 8 == 0x000001) {
+        if (start_code_is_valid(start_code)) {
             // Unit runs from start to the beginning of the start code
             // pointed to by end (including any padding zeroes).
             unit_size = (end - 4) - start;
@@ -189,7 +189,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         start = end;
 
         // Do we have a further unit to add to the fragment?
-    } while ((start_code >> 8) == 0x000001);
+    } while (start_code_is_valid(start_code));
 
     return 0;
 }
diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c
index 027a578af1..9c639933ee 100644
--- a/libavcodec/extract_extradata_bsf.c
+++ b/libavcodec/extract_extradata_bsf.c
@@ -239,7 +239,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt,
         ptr = avpriv_find_start_code(ptr, end, &state);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             has_extradata = 1;
-        } else if (has_extradata && IS_MARKER(state)) {
+        } else if (has_extradata && start_code_is_valid(state)) {
             extradata_size = ptr - 4 - pkt->data;
             break;
         }
diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index 5520960b74..85f4f432bd 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -213,7 +213,7 @@ int ff_mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size,
                 pc->frame_start_found = 0;
             if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
                 pc->frame_start_found++;
-            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
+            if (pc->frame_start_found == 4 && start_code_is_valid(state)) {
                 if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
                     pc->frame_start_found = 0;
                     pc->state             = -1;
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 860e86aa74..27fd61e848 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2475,7 +2475,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
         /* find next start code */
         uint32_t start_code = -1;
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
-        if (start_code > 0x1ff) {
+        if (!start_code_is_valid(start_code)) {
             if (!skip_frame) {
                 if (HAVE_THREADS &&
                     (avctx->active_thread_type & FF_THREAD_SLICE) &&
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index 22666c85d9..82fdd1a1bb 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -82,7 +82,7 @@ static int mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf,
                 pc->frame_start_found = 0;
             if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
                 pc->frame_start_found++;
-            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
+            if (pc->frame_start_found == 4 && start_code_is_valid(state)) {
                 if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
                     pc->frame_start_found = 0;
                     pc->state             = -1;
diff --git a/libavcodec/remove_extradata_bsf.c b/libavcodec/remove_extradata_bsf.c
index c698d1d7f9..55e4ceb662 100644
--- a/libavcodec/remove_extradata_bsf.c
+++ b/libavcodec/remove_extradata_bsf.c
@@ -35,8 +35,6 @@ enum RemoveFreq {
     REMOVE_FREQ_NONKEYFRAME,
 };
 
-#define START_CODE 0x000001
-
 typedef struct RemoveExtradataContext {
     const AVClass *class;
     int freq;
@@ -73,7 +71,7 @@ static int h264_split(const uint8_t *buf, int buf_size)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &state);
-        if ((state & 0xFFFFFF00) != 0x100)
+        if (!start_code_is_valid(state))
             break;
         nalu_type = state & 0x1F;
         if (nalu_type == H264_NAL_SPS) {
@@ -111,7 +109,7 @@ static int hevc_split(const uint8_t *buf, int buf_size)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &state);
-        if ((state >> 8) != START_CODE)
+        if (!start_code_is_valid(state))
             break;
         nut = (state >> 1) & 0x3F;
         if (nut == HEVC_NAL_VPS)
@@ -171,7 +169,7 @@ static int vc1_split(const uint8_t *buf, int buf_size)
         ptr = avpriv_find_start_code(ptr, end, &state);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             charged = 1;
-        } else if (charged && IS_MARKER(state))
+        } else if (charged && start_code_is_valid(state))
             return ptr - 4 - buf;
     }
 
diff --git a/libavcodec/startcode.h b/libavcodec/startcode.h
index 8b75832aaf..833754af09 100644
--- a/libavcodec/startcode.h
+++ b/libavcodec/startcode.h
@@ -27,6 +27,23 @@
 
 #include <stdint.h>
 
+#include "libavutil/attributes.h"
+
+/**
+ * @brief Test whether a start code found by avpriv_find_start_code() is valid.
+ *
+ * Use this to test the validity of a start code especially if a start code can
+ * be at the end of the buffer, where testing the return value of avpriv_find_start_code()
+ * would incorrectly imply that the start code is invalid (since the returned value
+ * equals <b>@c end </b>).
+ *
+ * @param[in] start_code The start code to test.
+ * @return A boolean that is true if and only if <b>@p start_code</b> is valid
+ */
+static av_always_inline int start_code_is_valid(uint32_t start_code) {
+    return (start_code & 0xFFFFFF00) == 0x100;
+}
+
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t *end,
                                       uint32_t *state);
diff --git a/libavcodec/vaapi_vc1.c b/libavcodec/vaapi_vc1.c
index d82b5b31f8..cd3ef16982 100644
--- a/libavcodec/vaapi_vc1.c
+++ b/libavcodec/vaapi_vc1.c
@@ -470,7 +470,7 @@ static int vaapi_vc1_decode_slice(AVCodecContext *avctx, const uint8_t *buffer,
     int err;
 
     /* Current bit buffer is beyond any marker for VC-1, so skip it */
-    if (avctx->codec_id == AV_CODEC_ID_VC1 && IS_MARKER(AV_RB32(buffer))) {
+    if (avctx->codec_id == AV_CODEC_ID_VC1 && start_code_is_valid(AV_RB32(buffer))) {
         buffer += 4;
         size -= 4;
     }
diff --git a/libavcodec/vc1_common.h b/libavcodec/vc1_common.h
index c0f0c8c2eb..8ff9802a51 100644
--- a/libavcodec/vc1_common.h
+++ b/libavcodec/vc1_common.h
@@ -41,8 +41,6 @@ enum VC1Code {
 };
 //@}
 
-#define IS_MARKER(x) (((x) & ~0xFF) == VC1_CODE_RES0)
-
 /** Available Profiles */
 //@{
 enum Profile {
@@ -61,7 +59,7 @@ static av_always_inline const uint8_t* find_next_marker(const uint8_t *src, cons
     if (end - src >= 4) {
         uint32_t mrk = 0xFFFFFFFF;
         src = avpriv_find_start_code(src, end, &mrk);
-        if (IS_MARKER(mrk))
+        if (start_code_is_valid(mrk))
             return src - 4;
     }
     return end;
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index 7ed5133cfa..6dcc28463e 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -664,7 +664,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, void *data,
         if (!buf2)
             return AVERROR(ENOMEM);
 
-        if (IS_MARKER(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
+        if (start_code_is_valid(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
             const uint8_t *start, *end, *next;
             int size;
 
diff --git a/libavformat/avs2dec.c b/libavformat/avs2dec.c
index 0d5b23b65e..a5a7cee7d0 100644
--- a/libavformat/avs2dec.c
+++ b/libavformat/avs2dec.c
@@ -42,8 +42,8 @@ static int avs2_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        state = code & 0xFF;
-        if ((code & 0xffffff00) == 0x100) {
+        if (start_code_is_valid(code)) {
+            state = code & 0xFF;
             if (AVS2_ISUNIT(state)) {
                 if (sqb && !hds) {
                     hds = ptr - sqb;
diff --git a/libavformat/avs3dec.c b/libavformat/avs3dec.c
index 2395df171b..112b28efa8 100644
--- a/libavformat/avs3dec.c
+++ b/libavformat/avs3dec.c
@@ -36,8 +36,8 @@ static int avs3video_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        state = code & 0xFF;
-        if ((code & 0xFFFFFF00) == 0x100) {
+        if (start_code_is_valid(code)) {
+            state = code & 0xFF;
             if (state < AVS3_SEQ_START_CODE) {
                 if (code < slice_pos)
                     return 0;
diff --git a/libavformat/cavsvideodec.c b/libavformat/cavsvideodec.c
index 1fa3aa5e92..01cf069f5e 100644
--- a/libavformat/cavsvideodec.c
+++ b/libavformat/cavsvideodec.c
@@ -38,7 +38,7 @@ static int cavsvideo_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        if ((code & 0xffffff00) == 0x100) {
+        if (start_code_is_valid(code)) {
             if(code < CAVS_SEQ_START_CODE) {
                 /* slices have to be consecutive */
                 if(code < slice_pos)
diff --git a/libavformat/mpegvideodec.c b/libavformat/mpegvideodec.c
index 33c5d79794..88966af72a 100644
--- a/libavformat/mpegvideodec.c
+++ b/libavformat/mpegvideodec.c
@@ -44,7 +44,7 @@ static int mpegvideo_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        if ((code & 0xffffff00) == 0x100) {
+        if (start_code_is_valid(code)) {
             switch(code){
             case     SEQ_START_CODE:
                 if (!(ptr[3 + 1 + 2] & 0x20))
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index c18c75082f..8b6987b7f2 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -57,7 +57,7 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
             while (1) {
                 start_code = -1;
                 r = avpriv_find_start_code(r1, end, &start_code);
-                if((start_code & 0xFFFFFF00) == 0x100) {
+                if (start_code_is_valid(start_code)) {
                     /* New start code found */
                     if (start_code == 0x100) {
                         frame_type = (r[1] & 0x38) >> 3;
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 2/8] avpriv_find_start_code(): readability enhancement part 1

[Scott Theisen]

No functional change.
---
 libavcodec/utils.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index c7c7323351..42a1885d61 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -963,10 +963,11 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
         }
     }
 
-    p = FFMIN(p, end) - 4;
-    *state = AV_RB32(p);
-
-    return p + 4;
+    if (p > end)
+        p = end;
+    // read the previous 4 bytes, i.e. bytes {p - 4, p - 3, p - 2, p - 1}
+    *state = AV_RB32(p - 4);
+    return p;
 }
 
 AVCPBProperties *av_cpb_properties_alloc(size_t *size)
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 3/8] avpriv_find_start_code(): rewrite while loop

[Scott Theisen]

The expected number of iterations may increase by one for an input
of alternating 0 and 1 bytes.  Instead of incrementing by 2 everytime,
it now alternates between incrementing by 1 and by 3.

For the check p[-2] != 0:
This slightly reduces the number of iterations by starting with three
new bytes on the next iteration, instead of keeping byte p[-3] which
is invalid, since it is now known to be 01 when it must be 00.

The comparisons (p[-1]  > 1) and (p[-1] == 0) should be one
comparison against 1 since p is unsigned, which makes
(p[-1] == 0) equivalent to (p[-1] < 1)

No other observable change.
---
 libavcodec/utils.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 42a1885d61..5b49657b44 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -953,12 +953,27 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
             return p;
     }
 
+    /* with memory address increasing left to right, we are looking for (in hexadecimal):
+     * 00 00 01 XX
+     * p points at the address which should have the value of XX
+     */
     while (p < end) {
-        if      (p[-1] > 1      ) p += 3;
-        else if (p[-2]          ) p += 2;
-        else if (p[-3]|(p[-1]-1)) p++;
-        else {
+        // UU UU UU
+        if      (p[-1]  > 1) {
+            p += 3;
+            // start check over with 3 new bytes
+        }
+        else if (p[-1] == 0) {
+            p++;
+            // could be in a start code, so check next byte
+        }
+        else if (/* UU UU 01 */ p[-2] != 0 ||
+                 /* UU 00 01 */ p[-3] != 0) {
+            p += 3;
+        }
+        else { /* 00 00 01 */
             p++;
+            // p now points at the address following the start code value XX
             break;
         }
     }
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 4/8] avpriv_find_start_code(): rewrite for loop for clarity

[Scott Theisen]

---
 libavcodec/utils.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 5b49657b44..bdafdaa355 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -940,18 +940,20 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
                                       uint32_t *av_restrict state)
 {
-    int i;
-
     av_assert0(p <= end);
     if (p >= end)
         return end;
 
-    for (i = 0; i < 3; i++) {
-        uint32_t tmp = *state << 8;
-        *state = tmp + *(p++);
-        if (tmp == 0x100 || p == 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++) {
+        *state <<= 8;
+        *state += *p;
+        p++;
+        if (start_code_is_valid(*state) || p == end)
             return p;
     }
+    // p is now properly incremented for the negative indices in the while loop
 
     /* with memory address increasing left to right, we are looking for (in hexadecimal):
      * 00 00 01 XX
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 5/8] avpriv_find_start_code(): add doxygen comment and rename a parameter

[Scott Theisen]

---
 libavcodec/startcode.h | 26 +++++++++++++++++++++++++-
 libavcodec/utils.c     | 10 +++++-----
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/libavcodec/startcode.h b/libavcodec/startcode.h
index 833754af09..69389c729c 100644
--- a/libavcodec/startcode.h
+++ b/libavcodec/startcode.h
@@ -44,9 +44,33 @@ static av_always_inline int start_code_is_valid(uint32_t start_code) {
     return (start_code & 0xFFFFFF00) == 0x100;
 }
 
+/**
+ * @brief Find the first start code in the buffer @p p.
+ *
+ * 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$).
+ *
+ * @return A pointer to the memory address following the found start code, or <b>@p end</b>
+ *         if no start code was found.
+ */
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t *end,
-                                      uint32_t *state);
+                                      uint32_t *start_code);
 
 int ff_startcode_find_candidate_c(const uint8_t *buf, int size);
 
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index bdafdaa355..68d126acd8 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -938,7 +938,7 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
 
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
-                                      uint32_t *av_restrict state)
+                                      uint32_t *av_restrict start_code)
 {
     av_assert0(p <= end);
     if (p >= end)
@@ -947,10 +947,10 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     // 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++) {
-        *state <<= 8;
-        *state += *p;
+        *start_code <<= 8;
+        *start_code += *p;
         p++;
-        if (start_code_is_valid(*state) || p == end)
+        if (start_code_is_valid(*start_code) || p == end)
             return p;
     }
     // p is now properly incremented for the negative indices in the while loop
@@ -983,7 +983,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     if (p > end)
         p = end;
     // read the previous 4 bytes, i.e. bytes {p - 4, p - 3, p - 2, p - 1}
-    *state = AV_RB32(p - 4);
+    *start_code = AV_RB32(p - 4);
     return p;
 }
 
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 6/8] avpriv_find_start_code(): correct type of start_code parameter

[Scott Theisen]

---
 libavcodec/mpeg12dec.c   | 2 +-
 libavformat/rtpenc_mpv.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 27fd61e848..24cd6aac77 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1768,7 +1768,7 @@ static int mpeg_decode_slice(MpegEncContext *s, int mb_y,
 
     if (avctx->hwaccel && avctx->hwaccel->decode_slice) {
         const uint8_t *buf_end, *buf_start = *buf - 4; /* include start_code */
-        int start_code = -1;
+        uint32_t start_code = ~0;
         buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code);
         if (buf_end < *buf + buf_size)
             buf_end -= 4;
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index 8b6987b7f2..9c0816ef95 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -51,11 +51,10 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
             end_of_slice = 1;
         } else {
             const uint8_t *r, *r1;
-            int start_code;
 
             r1 = buf1;
             while (1) {
-                start_code = -1;
+                uint32_t start_code = ~0;
                 r = avpriv_find_start_code(r1, end, &start_code);
                 if (start_code_is_valid(start_code)) {
                     /* New start code found */
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 7/8] avpriv_find_start_code(): constify pointer parameters

[Scott Theisen]

Have the compiler enforce not changing the addresses these parameters point to.

No functional change.
---
 libavcodec/utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 68d126acd8..d11d92d21e 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -937,8 +937,8 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
 #endif
 
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
-                                      const uint8_t *end,
-                                      uint32_t *av_restrict start_code)
+                                      const uint8_t * const end,
+                                      uint32_t * const av_restrict start_code)
 {
     av_assert0(p <= end);
     if (p >= end)
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Scott Theisen]

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 --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 */
-- 
2.32.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 0/8] rewrite avpriv_find_start_code() for clarity

[Scott Theisen]

On 2/8/22 22:28, Scott Theisen wrote:
> I have removed most of my added comments.
>
> Rebased to account for Andreas Rheinhardt moving the prototype to
> startcode.h.
>
> Per his suggestion, the function signature is now unmodified.
> (ff_)mpeg1_find_frame_end() now has a wrapper function preserving
> the original behavior allowing start codes across buffer boundaries.
>
> -Scott
>

Andreas,

Have you had a chance to look at v3?

Thanks,

Scott
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 0/8] rewrite avpriv_find_start_code() for clarity

[Scott Theisen]

My modified versions of avpriv_find_start_code were accepted into MythTV,
which caused clang-tidy to raise a bugprone-branch-clone warning.  The
for loop's if statement has therefore been modified to not duplicate
the branch payload.

There are no other changes from v3.

Regards,

Scott Theisen



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 1/8] avcodec/startcode.h: create start_code_is_valid()

[Scott Theisen]

This slightly changes what is considered valid in the following cases:
cavsdec.c:   0x000000XX is now considered invalid.
mpeg12dec.c: 0x000000XX is now considered invalid.
(where X is any value)

IS_MARKER is equivalent since VC1_CODE_RES0 = 0x00000100
---
 libavcodec/cavsdec.c               |  2 +-
 libavcodec/cbs_mpeg2.c             |  6 +++---
 libavcodec/extract_extradata_bsf.c |  2 +-
 libavcodec/mpeg12.c                |  2 +-
 libavcodec/mpeg12dec.c             |  2 +-
 libavcodec/mpegvideo_parser.c      |  2 +-
 libavcodec/remove_extradata_bsf.c  |  8 +++-----
 libavcodec/startcode.h             | 17 +++++++++++++++++
 libavcodec/vaapi_vc1.c             |  2 +-
 libavcodec/vc1_common.h            |  4 +---
 libavcodec/vc1dec.c                |  2 +-
 libavformat/avs2dec.c              |  4 ++--
 libavformat/avs3dec.c              |  4 ++--
 libavformat/cavsvideodec.c         |  2 +-
 libavformat/mpegvideodec.c         |  2 +-
 libavformat/rtpenc_mpv.c           |  2 +-
 16 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 3e8be65968..0c19d0e3f6 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -1257,7 +1257,7 @@ static int cavs_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
     buf_end = buf + buf_size;
     for(;;) {
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
-        if ((stc & 0xFFFFFE00) || buf_ptr == buf_end) {
+        if (!start_code_is_valid(stc) || buf_ptr == buf_end) {
             if (!h->stc)
                 av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
             return FFMAX(0, buf_ptr - buf);
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 04b0c7f87d..23839ca47b 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -150,7 +150,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
 
     start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
                                    &start_code);
-    if (start_code >> 8 != 0x000001) {
+    if (!start_code_is_valid(start_code)) {
         // No start code found.
         return AVERROR_INVALIDDATA;
     }
@@ -172,7 +172,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         // (may be the last byte of fragment->data); end points to the byte
         // following the byte containing the start code identifier (or to
         // the end of fragment->data).
-        if (start_code >> 8 == 0x000001) {
+        if (start_code_is_valid(start_code)) {
             // Unit runs from start to the beginning of the start code
             // pointed to by end (including any padding zeroes).
             unit_size = (end - 4) - start;
@@ -189,7 +189,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
         start = end;
 
         // Do we have a further unit to add to the fragment?
-    } while ((start_code >> 8) == 0x000001);
+    } while (start_code_is_valid(start_code));
 
     return 0;
 }
diff --git a/libavcodec/extract_extradata_bsf.c b/libavcodec/extract_extradata_bsf.c
index 329b1a6174..e698a2632e 100644
--- a/libavcodec/extract_extradata_bsf.c
+++ b/libavcodec/extract_extradata_bsf.c
@@ -239,7 +239,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx, AVPacket *pkt,
         ptr = avpriv_find_start_code(ptr, end, &state);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             has_extradata = 1;
-        } else if (has_extradata && IS_MARKER(state)) {
+        } else if (has_extradata && start_code_is_valid(state)) {
             extradata_size = ptr - 4 - pkt->data;
             break;
         }
diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index d78e25a777..5ff1830496 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -214,7 +214,7 @@ int ff_mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf, int buf_size,
                 pc->frame_start_found = 0;
             if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
                 pc->frame_start_found++;
-            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
+            if (pc->frame_start_found == 4 && start_code_is_valid(state)) {
                 if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
                     pc->frame_start_found = 0;
                     pc->state             = -1;
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 7133696f3c..b857353ab8 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2446,7 +2446,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
         /* find next start code */
         uint32_t start_code = -1;
         buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &start_code);
-        if (start_code > 0x1ff) {
+        if (!start_code_is_valid(start_code)) {
             if (!skip_frame) {
                 if (HAVE_THREADS &&
                     (avctx->active_thread_type & FF_THREAD_SLICE) &&
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index ac6efb6909..f5afa95981 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -83,7 +83,7 @@ static int mpeg1_find_frame_end(ParseContext *pc, const uint8_t *buf,
                 pc->frame_start_found = 0;
             if (pc->frame_start_found  < 4 && state == EXT_START_CODE)
                 pc->frame_start_found++;
-            if (pc->frame_start_found == 4 && (state & 0xFFFFFF00) == 0x100) {
+            if (pc->frame_start_found == 4 && start_code_is_valid(state)) {
                 if (state < SLICE_MIN_START_CODE || state > SLICE_MAX_START_CODE) {
                     pc->frame_start_found = 0;
                     pc->state             = -1;
diff --git a/libavcodec/remove_extradata_bsf.c b/libavcodec/remove_extradata_bsf.c
index 66b7d00bd8..1914cf19d5 100644
--- a/libavcodec/remove_extradata_bsf.c
+++ b/libavcodec/remove_extradata_bsf.c
@@ -35,8 +35,6 @@ enum RemoveFreq {
     REMOVE_FREQ_NONKEYFRAME,
 };
 
-#define START_CODE 0x000001
-
 typedef struct RemoveExtradataContext {
     const AVClass *class;
     int freq;
@@ -73,7 +71,7 @@ static int h264_split(const uint8_t *buf, int buf_size)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &state);
-        if ((state & 0xFFFFFF00) != 0x100)
+        if (!start_code_is_valid(state))
             break;
         nalu_type = state & 0x1F;
         if (nalu_type == H264_NAL_SPS) {
@@ -111,7 +109,7 @@ static int hevc_split(const uint8_t *buf, int buf_size)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &state);
-        if ((state >> 8) != START_CODE)
+        if (!start_code_is_valid(state))
             break;
         nut = (state >> 1) & 0x3F;
         if (nut == HEVC_NAL_VPS)
@@ -171,7 +169,7 @@ static int vc1_split(const uint8_t *buf, int buf_size)
         ptr = avpriv_find_start_code(ptr, end, &state);
         if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
             charged = 1;
-        } else if (charged && IS_MARKER(state))
+        } else if (charged && start_code_is_valid(state))
             return ptr - 4 - buf;
     }
 
diff --git a/libavcodec/startcode.h b/libavcodec/startcode.h
index 8b75832aaf..833754af09 100644
--- a/libavcodec/startcode.h
+++ b/libavcodec/startcode.h
@@ -27,6 +27,23 @@
 
 #include <stdint.h>
 
+#include "libavutil/attributes.h"
+
+/**
+ * @brief Test whether a start code found by avpriv_find_start_code() is valid.
+ *
+ * Use this to test the validity of a start code especially if a start code can
+ * be at the end of the buffer, where testing the return value of avpriv_find_start_code()
+ * would incorrectly imply that the start code is invalid (since the returned value
+ * equals <b>@c end </b>).
+ *
+ * @param[in] start_code The start code to test.
+ * @return A boolean that is true if and only if <b>@p start_code</b> is valid
+ */
+static av_always_inline int start_code_is_valid(uint32_t start_code) {
+    return (start_code & 0xFFFFFF00) == 0x100;
+}
+
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t *end,
                                       uint32_t *state);
diff --git a/libavcodec/vaapi_vc1.c b/libavcodec/vaapi_vc1.c
index d82336a3b3..33de4a63c8 100644
--- a/libavcodec/vaapi_vc1.c
+++ b/libavcodec/vaapi_vc1.c
@@ -473,7 +473,7 @@ static int vaapi_vc1_decode_slice(AVCodecContext *avctx, const uint8_t *buffer,
     int err;
 
     /* Current bit buffer is beyond any marker for VC-1, so skip it */
-    if (avctx->codec_id == AV_CODEC_ID_VC1 && IS_MARKER(AV_RB32(buffer))) {
+    if (avctx->codec_id == AV_CODEC_ID_VC1 && start_code_is_valid(AV_RB32(buffer))) {
         buffer += 4;
         size -= 4;
     }
diff --git a/libavcodec/vc1_common.h b/libavcodec/vc1_common.h
index c0f0c8c2eb..8ff9802a51 100644
--- a/libavcodec/vc1_common.h
+++ b/libavcodec/vc1_common.h
@@ -41,8 +41,6 @@ enum VC1Code {
 };
 //@}
 
-#define IS_MARKER(x) (((x) & ~0xFF) == VC1_CODE_RES0)
-
 /** Available Profiles */
 //@{
 enum Profile {
@@ -61,7 +59,7 @@ static av_always_inline const uint8_t* find_next_marker(const uint8_t *src, cons
     if (end - src >= 4) {
         uint32_t mrk = 0xFFFFFFFF;
         src = avpriv_find_start_code(src, end, &mrk);
-        if (IS_MARKER(mrk))
+        if (start_code_is_valid(mrk))
             return src - 4;
     }
     return end;
diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index 9f32e82f9e..d98c15f2e4 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -667,7 +667,7 @@ static int vc1_decode_frame(AVCodecContext *avctx, AVFrame *pict,
         if (!buf2)
             return AVERROR(ENOMEM);
 
-        if (IS_MARKER(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
+        if (start_code_is_valid(AV_RB32(buf))) { /* frame starts with marker and needs to be parsed */
             const uint8_t *start, *end, *next;
             int size;
 
diff --git a/libavformat/avs2dec.c b/libavformat/avs2dec.c
index 0d5b23b65e..a5a7cee7d0 100644
--- a/libavformat/avs2dec.c
+++ b/libavformat/avs2dec.c
@@ -42,8 +42,8 @@ static int avs2_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        state = code & 0xFF;
-        if ((code & 0xffffff00) == 0x100) {
+        if (start_code_is_valid(code)) {
+            state = code & 0xFF;
             if (AVS2_ISUNIT(state)) {
                 if (sqb && !hds) {
                     hds = ptr - sqb;
diff --git a/libavformat/avs3dec.c b/libavformat/avs3dec.c
index 2395df171b..112b28efa8 100644
--- a/libavformat/avs3dec.c
+++ b/libavformat/avs3dec.c
@@ -36,8 +36,8 @@ static int avs3video_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        state = code & 0xFF;
-        if ((code & 0xFFFFFF00) == 0x100) {
+        if (start_code_is_valid(code)) {
+            state = code & 0xFF;
             if (state < AVS3_SEQ_START_CODE) {
                 if (code < slice_pos)
                     return 0;
diff --git a/libavformat/cavsvideodec.c b/libavformat/cavsvideodec.c
index b89851fbbb..398f1192dd 100644
--- a/libavformat/cavsvideodec.c
+++ b/libavformat/cavsvideodec.c
@@ -39,7 +39,7 @@ static int cavsvideo_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        if ((code & 0xffffff00) == 0x100) {
+        if (start_code_is_valid(code)) {
             if(code < CAVS_SEQ_START_CODE) {
                 /* slices have to be consecutive */
                 if(code < slice_pos)
diff --git a/libavformat/mpegvideodec.c b/libavformat/mpegvideodec.c
index 33c5d79794..88966af72a 100644
--- a/libavformat/mpegvideodec.c
+++ b/libavformat/mpegvideodec.c
@@ -44,7 +44,7 @@ static int mpegvideo_probe(const AVProbeData *p)
 
     while (ptr < end) {
         ptr = avpriv_find_start_code(ptr, end, &code);
-        if ((code & 0xffffff00) == 0x100) {
+        if (start_code_is_valid(code)) {
             switch(code){
             case     SEQ_START_CODE:
                 if (!(ptr[3 + 1 + 2] & 0x20))
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index c18c75082f..8b6987b7f2 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -57,7 +57,7 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
             while (1) {
                 start_code = -1;
                 r = avpriv_find_start_code(r1, end, &start_code);
-                if((start_code & 0xFFFFFF00) == 0x100) {
+                if (start_code_is_valid(start_code)) {
                     /* New start code found */
                     if (start_code == 0x100) {
                         frame_type = (r[1] & 0x38) >> 3;
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 2/8] avpriv_find_start_code(): readability enhancement part 1

[Scott Theisen]

No functional change.
---
 libavcodec/utils.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 2f57418ff7..d6ab21b1a0 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1006,10 +1006,11 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
         }
     }
 
-    p = FFMIN(p, end) - 4;
-    *state = AV_RB32(p);
-
-    return p + 4;
+    if (p > end)
+        p = end;
+    // read the previous 4 bytes, i.e. bytes {p - 4, p - 3, p - 2, p - 1}
+    *state = AV_RB32(p - 4);
+    return p;
 }
 
 AVCPBProperties *av_cpb_properties_alloc(size_t *size)
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 3/8] avpriv_find_start_code(): rewrite while loop

[Scott Theisen]

The expected number of iterations may increase by one for an input
of alternating 0 and 1 bytes.  Instead of incrementing by 2 everytime,
it now alternates between incrementing by 1 and by 3.

For the check p[-2] != 0:
This slightly reduces the number of iterations by starting with three
new bytes on the next iteration, instead of keeping byte p[-3] which
is invalid, since it is now known to be 01 when it must be 00.

No other observable change.
---
 libavcodec/utils.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index d6ab21b1a0..fc8cd87366 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -996,12 +996,24 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
             return p;
     }
 
+    /* with memory address increasing left to right, we are looking for (in hexadecimal):
+     * 00 00 01 XX
+     * p points at the address which should have the value of XX
+     */
     while (p < end) {
-        if      (p[-1] > 1      ) p += 3;
-        else if (p[-2]          ) p += 2;
-        else if (p[-3]|(p[-1]-1)) p++;
-        else {
+        if      (/* UU UU UU */ p[-1]  < 1) { // equivalently p[-1] == 0
             p++;
+            // could be in a start code, so check next byte
+        }
+        else if (/* UU UU UN */ p[-1]  > 1 ||
+                 /* UU UU 01 */ p[-2] != 0 ||
+                 /* UU 00 01 */ p[-3] != 0) {
+            // start check over with 3 new bytes
+            p += 3;
+        }
+        else { /* 00 00 01 */
+            p++;
+            // p now points at the address following the start code value XX
             break;
         }
     }
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 4/8] avpriv_find_start_code(): rewrite for loop for clarity

[Scott Theisen]

---
 libavcodec/utils.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index fc8cd87366..83f7d8a01a 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -983,18 +983,20 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
                                       uint32_t *av_restrict state)
 {
-    int i;
-
     av_assert0(p <= end);
     if (p >= end)
         return end;
 
-    for (i = 0; i < 3; i++) {
-        uint32_t tmp = *state << 8;
-        *state = tmp + *(p++);
-        if (tmp == 0x100 || p == 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++) {
+        *state <<= 8;
+        *state += *p;
+        p++;
+        if (start_code_is_valid(*state) || p == end)
             return p;
     }
+    // p is now properly incremented for the negative indices in the while loop
 
     /* with memory address increasing left to right, we are looking for (in hexadecimal):
      * 00 00 01 XX
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 5/8] avpriv_find_start_code(): add doxygen comment and rename a parameter

[Scott Theisen]

---
 libavcodec/startcode.h | 26 +++++++++++++++++++++++++-
 libavcodec/utils.c     | 10 +++++-----
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/libavcodec/startcode.h b/libavcodec/startcode.h
index 833754af09..69389c729c 100644
--- a/libavcodec/startcode.h
+++ b/libavcodec/startcode.h
@@ -44,9 +44,33 @@ static av_always_inline int start_code_is_valid(uint32_t start_code) {
     return (start_code & 0xFFFFFF00) == 0x100;
 }
 
+/**
+ * @brief Find the first start code in the buffer @p p.
+ *
+ * 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$).
+ *
+ * @return A pointer to the memory address following the found start code, or <b>@p end</b>
+ *         if no start code was found.
+ */
 const uint8_t *avpriv_find_start_code(const uint8_t *p,
                                       const uint8_t *end,
-                                      uint32_t *state);
+                                      uint32_t *start_code);
 
 int ff_startcode_find_candidate_c(const uint8_t *buf, int size);
 
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 83f7d8a01a..63bf4b8cb7 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -981,7 +981,7 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
 
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
                                       const uint8_t *end,
-                                      uint32_t *av_restrict state)
+                                      uint32_t *av_restrict start_code)
 {
     av_assert0(p <= end);
     if (p >= end)
@@ -990,10 +990,10 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     // 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++) {
-        *state <<= 8;
-        *state += *p;
+        *start_code <<= 8;
+        *start_code += *p;
         p++;
-        if (start_code_is_valid(*state) || p == end)
+        if (start_code_is_valid(*start_code) || p == end)
             return p;
     }
     // p is now properly incremented for the negative indices in the while loop
@@ -1023,7 +1023,7 @@ const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
     if (p > end)
         p = end;
     // read the previous 4 bytes, i.e. bytes {p - 4, p - 3, p - 2, p - 1}
-    *state = AV_RB32(p - 4);
+    *start_code = AV_RB32(p - 4);
     return p;
 }
 
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 6/8] avpriv_find_start_code(): correct type of start_code parameter

[Scott Theisen]

---
 libavcodec/mpeg12dec.c   | 2 +-
 libavformat/rtpenc_mpv.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index b857353ab8..6a7af91fad 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1746,7 +1746,7 @@ static int mpeg_decode_slice(MpegEncContext *s, int mb_y,
 
     if (avctx->hwaccel && avctx->hwaccel->decode_slice) {
         const uint8_t *buf_end, *buf_start = *buf - 4; /* include start_code */
-        int start_code = -1;
+        uint32_t start_code = ~0;
         buf_end = avpriv_find_start_code(buf_start + 2, *buf + buf_size, &start_code);
         if (buf_end < *buf + buf_size)
             buf_end -= 4;
diff --git a/libavformat/rtpenc_mpv.c b/libavformat/rtpenc_mpv.c
index 8b6987b7f2..9c0816ef95 100644
--- a/libavformat/rtpenc_mpv.c
+++ b/libavformat/rtpenc_mpv.c
@@ -51,11 +51,10 @@ void ff_rtp_send_mpegvideo(AVFormatContext *s1, const uint8_t *buf1, int size)
             end_of_slice = 1;
         } else {
             const uint8_t *r, *r1;
-            int start_code;
 
             r1 = buf1;
             while (1) {
-                start_code = -1;
+                uint32_t start_code = ~0;
                 r = avpriv_find_start_code(r1, end, &start_code);
                 if (start_code_is_valid(start_code)) {
                     /* New start code found */
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 7/8] avpriv_find_start_code(): constify pointer parameters

[Scott Theisen]

Have the compiler enforce not changing the addresses these parameters point to.

No functional change.
---
 libavcodec/utils.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 63bf4b8cb7..0635c5dcaa 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -980,8 +980,8 @@ void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, in
 #endif
 
 const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
-                                      const uint8_t *end,
-                                      uint32_t *av_restrict start_code)
+                                      const uint8_t * const end,
+                                      uint32_t * const av_restrict start_code)
 {
     av_assert0(p <= end);
     if (p >= end)
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 8/8] avpriv_find_start_code(): make start_code output only

[Scott Theisen]

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 --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 23839ca47b..fd235a43a7 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 5ff1830496..9e3c7e5814 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -166,6 +166,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
@@ -200,7 +239,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 3a3aad795f..dd351d9d0f 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 f5afa95981..d76e8ba069 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -34,6 +34,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
@@ -69,7 +108,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;
@@ -121,7 +160,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 0635c5dcaa..2a6067ca4e 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -984,19 +984,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 */
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 0/8] rewrite avpriv_find_start_code() for clarity

[Scott Theisen]

On 9/16/22 14:19, Scott Theisen wrote:
> My modified versions of avpriv_find_start_code were accepted into MythTV,
> which caused clang-tidy to raise a bugprone-branch-clone warning.  The
> for loop's if statement has therefore been modified to not duplicate

This should say while loop.

> the branch payload.
>
> There are no other changes from v3.
>
> Regards,
>
> Scott Theisen
>
>
>

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".