diff mbox series

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

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

Checks

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

Commit Message

Scott Theisen Feb. 3, 2022, 6:44 p.m. UTC 
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(-)

Comments

Andreas Rheinhardt Feb. 5, 2022, 6:10 a.m. UTC | #1 
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
Scott Theisen Feb. 5, 2022, 9 a.m. UTC | #2 
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
diff mbox series

Patch

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) {