[FFmpeg-devel] [PATCH 2/3] doc/examples/demuxing_decoding: convert to new decoding API

[James Almer]

On 4/11/2020 12:06 PM, Anton Khirnov wrote:
> ---
>  doc/examples/demuxing_decoding.c | 177 ++++++++++++++++---------------
>  1 file changed, 91 insertions(+), 86 deletions(-)
> 
> diff --git a/doc/examples/demuxing_decoding.c b/doc/examples/demuxing_decoding.c
> index 9bde927321..803e35d25c 100644
> --- a/doc/examples/demuxing_decoding.c
> +++ b/doc/examples/demuxing_decoding.c
> @@ -55,87 +55,93 @@ static AVPacket pkt;
>  static int video_frame_count = 0;
>  static int audio_frame_count = 0;
>  
> -static int decode_packet(int *got_frame, int cached)
> +static int output_video_frame(AVFrame *frame)
> +{
> +    if (frame->width != width || frame->height != height ||
> +        frame->format != pix_fmt) {
> +        /* To handle this change, one could call av_image_alloc again and
> +         * decode the following frames into another rawvideo file. */
> +        fprintf(stderr, "Error: Width, height and pixel format have to be "
> +                "constant in a rawvideo file, but the width, height or "
> +                "pixel format of the input video changed:\n"
> +                "old: width = %d, height = %d, format = %s\n"
> +                "new: width = %d, height = %d, format = %s\n",
> +                width, height, av_get_pix_fmt_name(pix_fmt),
> +                frame->width, frame->height,
> +                av_get_pix_fmt_name(frame->format));
> +        return -1;
> +    }
> +
> +    printf("video_frame n:%d coded_n:%d\n",
> +           video_frame_count++, frame->coded_picture_number);
> +
> +    /* copy decoded frame to destination buffer:
> +     * this is required since rawvideo expects non aligned data */
> +    av_image_copy(video_dst_data, video_dst_linesize,
> +                  (const uint8_t **)(frame->data), frame->linesize,
> +                  pix_fmt, width, height);
> +
> +    /* write to rawvideo file */
> +    fwrite(video_dst_data[0], 1, video_dst_bufsize, video_dst_file);
> +    return 0;
> +}
> +
> +static int output_audio_frame(AVFrame *frame)
> +{
> +    size_t unpadded_linesize = frame->nb_samples * av_get_bytes_per_sample(frame->format);
> +    printf("audio_frame n:%d nb_samples:%d pts:%s\n",
> +           audio_frame_count++, frame->nb_samples,
> +           av_ts2timestr(frame->pts, &audio_dec_ctx->time_base));
> +
> +    /* Write the raw audio data samples of the first plane. This works
> +     * fine for packed formats (e.g. AV_SAMPLE_FMT_S16). However,
> +     * most audio decoders output planar audio, which uses a separate
> +     * plane of audio samples for each channel (e.g. AV_SAMPLE_FMT_S16P).
> +     * In other words, this code will write only the first audio channel
> +     * in these cases.
> +     * You should use libswresample or libavfilter to convert the frame
> +     * to packed data. */
> +    fwrite(frame->extended_data[0], 1, unpadded_linesize, audio_dst_file);
> +
> +    return 0;
> +}
> +
> +static int decode_packet(AVCodecContext *dec, const AVPacket *pkt)
>  {
>      int ret = 0;
> -    int decoded = pkt.size;
>  
> -    *got_frame = 0;
> +    // submit the packet to the decoder
> +    ret = avcodec_send_packet(dec, pkt);
> +    if (ret < 0) {
> +        fprintf(stderr, "Error submitting a packet for decoding (%s)\n", av_err2str(ret));
> +        return ret;
> +    }
>  
> -    if (pkt.stream_index == video_stream_idx) {
> -        /* decode video frame */
> -        ret = avcodec_decode_video2(video_dec_ctx, frame, got_frame, &pkt);
> +    // get all the available frames from the decoder

Instead of this, it might be better to call receive_frame() one time
after each call to send_packet(), regardless of the latter consuming the
packet or not. If it doesn't, then you just keep it around until at some
point a call will consume it, then you can fetch the next one.

I say this because if you do

ret = avcodec_send_packet(pkt);
av_packet_unref(pkt);
if (ret < 0)
    return ret:
do {
    ret = avcodec_receive_frame(frame);
    if (!ret)
        output_frame(frame):
} while (!ret);

You'll be draining the decoders of all the frames it may have generated,
which may be detrimental in frame threading scenarios, versus something
like (Untested PoC):

do {
    ret = avcodec_send_packet(pkt);
    if (ret < 0) {
        if (ret != AVERROR(EAGAIN))
            return ret;
    } else
        av_packet_unref(pkt);
    ret = avcodec_receive_frame(frame);
    if (ret < 0) {
        if (ret != AVERROR(EAGAIN))
            return ret;
    } else
        output_frame(frame);
} while (!ret || pkt->size);

Which would constantly keep the decoder fed with packets as you retrieve
generated frames.

This is something I've noticed when writing the libdav1d decoder
wrapper. I don't know if it also applies to our frame threading logic,
or if it depends on the decoder used, but if it does, then the CLI would
also benefit from this.