[FFmpeg-devel] [PATCH 1/3] avcodec/decode: move processing discard samples to its own function

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Sat Jul 8 22:38:22 EEST 2023


James Almer:
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/decode.c | 182 +++++++++++++++++++++++---------------------
>  1 file changed, 94 insertions(+), 88 deletions(-)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 269633ce10..793ab975f6 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -289,69 +289,14 @@ static int64_t guess_correct_pts(AVCodecContext *ctx,
>      return pts;
>  }
>  
> -/*
> - * The core of the receive_frame_wrapper for the decoders implementing
> - * the simple API. Certain decoders might consume partial packets without
> - * returning any output, so this function needs to be called in a loop until it
> - * returns EAGAIN.
> - **/
> -static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
> +static int discard_samples(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
>  {
> -    AVCodecInternal   *avci = avctx->internal;
> -    AVPacket     *const pkt = avci->in_pkt;
> -    const FFCodec *const codec = ffcodec(avctx->codec);
> -    int got_frame, actual_got_frame;
> -    int ret;
> -
> -    if (!pkt->data && !avci->draining) {
> -        av_packet_unref(pkt);
> -        ret = ff_decode_get_packet(avctx, pkt);
> -        if (ret < 0 && ret != AVERROR_EOF)
> -            return ret;
> -    }
> -
> -    // Some codecs (at least wma lossless) will crash when feeding drain packets
> -    // after EOF was signaled.
> -    if (avci->draining_done)
> -        return AVERROR_EOF;
> -
> -    if (!pkt->data &&
> -        !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
> -          avctx->active_thread_type & FF_THREAD_FRAME))
> -        return AVERROR_EOF;
> -
> -    got_frame = 0;
> -
> -    if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
> -        ret = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
> -    } else {
> -        ret = codec->cb.decode(avctx, frame, &got_frame, pkt);
> -
> -        if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
> -            frame->pkt_dts = pkt->dts;
> -        if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> -#if FF_API_FRAME_PKT
> -FF_DISABLE_DEPRECATION_WARNINGS
> -            if(!avctx->has_b_frames)
> -                frame->pkt_pos = pkt->pos;
> -FF_ENABLE_DEPRECATION_WARNINGS
> -#endif
> -            //FIXME these should be under if(!avctx->has_b_frames)
> -            /* get_buffer is supposed to set frame parameters */
> -            if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
> -                if (!frame->sample_aspect_ratio.num)  frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
> -                if (!frame->width)                    frame->width               = avctx->width;
> -                if (!frame->height)                   frame->height              = avctx->height;
> -                if (frame->format == AV_PIX_FMT_NONE) frame->format              = avctx->pix_fmt;
> -            }
> -        }
> -    }
> -    emms_c();
> -    actual_got_frame = got_frame;
> +    AVCodecInternal *avci = avctx->internal;
> +    int ret = 0;
>  
>      if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
>          if (frame->flags & AV_FRAME_FLAG_DISCARD)
> -            got_frame = 0;
> +            ret = AVERROR(EAGAIN);
>      } else if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
>          uint8_t *side;
>          size_t side_size;
> @@ -359,16 +304,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          uint8_t skip_reason = 0;
>          uint8_t discard_reason = 0;
>  
> -        if (ret >= 0 && got_frame) {
>              if (frame->format == AV_SAMPLE_FMT_NONE)
>                  frame->format = avctx->sample_fmt;
> -            if (!frame->ch_layout.nb_channels) {
> -                int ret2 = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
> -                if (ret2 < 0) {
> -                    ret = ret2;
> -                    got_frame = 0;
> -                }
> -            }
> +            if (!frame->ch_layout.nb_channels)
> +                ret = av_channel_layout_copy(&frame->ch_layout, &avctx->ch_layout);
>  #if FF_API_OLD_CHANNEL_LAYOUT
>  FF_DISABLE_DEPRECATION_WARNINGS
>              if (!frame->channel_layout)
> @@ -380,7 +319,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  #endif
>              if (!frame->sample_rate)
>                  frame->sample_rate = avctx->sample_rate;
> -        }
>  
>          side= av_packet_get_side_data(avci->last_pkt_props, AV_PKT_DATA_SKIP_SAMPLES, &side_size);
>          if(side && side_size>=10) {
> @@ -393,21 +331,21 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              discard_reason = AV_RL8(side + 9);
>          }
>  
> -        if ((frame->flags & AV_FRAME_FLAG_DISCARD) && got_frame &&
> +        if ((frame->flags & AV_FRAME_FLAG_DISCARD) && !ret &&
>              !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
>              avci->skip_samples = FFMAX(0, avci->skip_samples - frame->nb_samples);
> -            got_frame = 0;
>              *discarded_samples += frame->nb_samples;
> +            ret = AVERROR(EAGAIN);
>          }
>  
> -        if (avci->skip_samples > 0 && got_frame &&
> +        if (avci->skip_samples > 0 && !ret &&
>              !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
>              if(frame->nb_samples <= avci->skip_samples){
> -                got_frame = 0;
>                  *discarded_samples += frame->nb_samples;
>                  avci->skip_samples -= frame->nb_samples;
>                  av_log(avctx, AV_LOG_DEBUG, "skip whole frame, skip left: %d\n",
>                         avci->skip_samples);
> +                ret = AVERROR(EAGAIN);
>              } else {
>                  av_samples_copy(frame->extended_data, frame->extended_data, 0, avci->skip_samples,
>                                  frame->nb_samples - avci->skip_samples, avctx->ch_layout.nb_channels, frame->format);
> @@ -432,11 +370,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              }
>          }
>  
> -        if (discard_padding > 0 && discard_padding <= frame->nb_samples && got_frame &&
> +        if (discard_padding > 0 && discard_padding <= frame->nb_samples && !ret &&
>              !(avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL)) {
>              if (discard_padding == frame->nb_samples) {
>                  *discarded_samples += frame->nb_samples;
> -                got_frame = 0;
> +                ret = AVERROR(EAGAIN);
>              } else {
>                  if(avctx->pkt_timebase.num && avctx->sample_rate) {
>                      int64_t diff_ts = av_rescale_q(frame->nb_samples - discard_padding,
> @@ -452,7 +390,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              }
>          }
>  
> -        if ((avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL) && got_frame) {
> +        if ((avctx->flags2 & AV_CODEC_FLAG2_SKIP_MANUAL) && !ret) {
>              AVFrameSideData *fside = av_frame_new_side_data(frame, AV_FRAME_DATA_SKIP_SAMPLES, 10);
>              if (fside) {
>                  AV_WL32(fside->data, avci->skip_samples);
> @@ -464,15 +402,88 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          }
>      }
>  
> -    if (!got_frame)
> +    return ret;
> +}
> +
> +/*
> + * The core of the receive_frame_wrapper for the decoders implementing
> + * the simple API. Certain decoders might consume partial packets without
> + * returning any output, so this function needs to be called in a loop until it
> + * returns EAGAIN.
> + **/
> +static inline int decode_simple_internal(AVCodecContext *avctx, AVFrame *frame, int64_t *discarded_samples)
> +{
> +    AVCodecInternal   *avci = avctx->internal;
> +    AVPacket     *const pkt = avci->in_pkt;
> +    const FFCodec *const codec = ffcodec(avctx->codec);
> +    int got_frame;
> +    int consumed, ret;
> +
> +    if (!pkt->data && !avci->draining) {
> +        av_packet_unref(pkt);
> +        ret = ff_decode_get_packet(avctx, pkt);
> +        if (ret < 0 && ret != AVERROR_EOF)
> +            return ret;
> +    }
> +
> +    // Some codecs (at least wma lossless) will crash when feeding drain packets
> +    // after EOF was signaled.
> +    if (avci->draining_done)
> +        return AVERROR_EOF;
> +
> +    if (!pkt->data &&
> +        !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY ||
> +          avctx->active_thread_type & FF_THREAD_FRAME))
> +        return AVERROR_EOF;
> +
> +    got_frame = 0;
> +
> +    if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME) {
> +        consumed = ff_thread_decode_frame(avctx, frame, &got_frame, pkt);
> +    } else {
> +        consumed = codec->cb.decode(avctx, frame, &got_frame, pkt);
> +
> +        if (!(codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
> +            frame->pkt_dts = pkt->dts;
> +        if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> +#if FF_API_FRAME_PKT
> +FF_DISABLE_DEPRECATION_WARNINGS
> +            if(!avctx->has_b_frames)
> +                frame->pkt_pos = pkt->pos;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +            //FIXME these should be under if(!avctx->has_b_frames)
> +            /* get_buffer is supposed to set frame parameters */
> +            if (!(avctx->codec->capabilities & AV_CODEC_CAP_DR1)) {
> +                if (!frame->sample_aspect_ratio.num)  frame->sample_aspect_ratio = avctx->sample_aspect_ratio;
> +                if (!frame->width)                    frame->width               = avctx->width;
> +                if (!frame->height)                   frame->height              = avctx->height;
> +                if (frame->format == AV_PIX_FMT_NONE) frame->format              = avctx->pix_fmt;
> +            }
> +        }
> +    }
> +    emms_c();
> +
> +    if (got_frame)
> +        ret = discard_samples(avctx, frame, discarded_samples);
> +    else
> +        ret = AVERROR(EAGAIN);
> +
> +    if (consumed < 0)
> +        ret = consumed;
> +    if (consumed >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO)
> +        consumed = pkt->size;
> +
> +    if (!got_frame || ret == AVERROR(EAGAIN))
>          av_frame_unref(frame);
>  
> -    if (ret >= 0 && avctx->codec->type == AVMEDIA_TYPE_VIDEO)
> -        ret = pkt->size;
> +    if (!ret)
> +        av_assert0(frame->buf[0]);
> +    if (ret == AVERROR(EAGAIN))
> +        ret = 0;
>  
> -    /* do not stop draining when actual_got_frame != 0 or ret < 0 */
> -    /* got_frame == 0 but actual_got_frame != 0 when frame is discarded */
> -    if (avci->draining && !actual_got_frame) {
> +    /* do not stop draining when got_frame != 0 or ret < 0 */
> +    if (avci->draining && !got_frame) {
>          if (ret < 0) {
>              /* prevent infinite loop if a decoder wrongly always return error on draining */
>              /* reasonable nb_errors_max = maximum b frames + thread count */
> @@ -490,11 +501,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          }
>      }
>  
> -    if (ret >= pkt->size || ret < 0) {
> +    if (consumed >= pkt->size || ret < 0) {
>          av_packet_unref(pkt);
>      } else {
> -        int consumed = ret;
> -
>          pkt->data                += consumed;
>          pkt->size                -= consumed;
>          pkt->pts                  = AV_NOPTS_VALUE;
> @@ -509,10 +518,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          }
>      }
>  
> -    if (got_frame)
> -        av_assert0(frame->buf[0]);
> -
> -    return ret < 0 ? ret : 0;
> +    return ret;
>  }
>  
>  #if CONFIG_LCMS2

Would using --diff-algorithm=minimal (or patience or histogram) produce
a more readable diff (i.e. one where the code that ends up in
discard_samples() is moved instead of everything except it)?

- Andreas



More information about the ffmpeg-devel mailing list