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

[James Almer <jamrial@gmail.com>]

On 7/8/2023 4:38 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Signed-off-by: James Almer <jamrial@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)?

No, "git format-patch --diff-algorithm={minimal,patience,histogram}" 
produces the same output.
_______________________________________________
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".