[FFmpeg-devel] [PATCH] lavc/audiotoolboxenc: fix dropped frames on iOS

[Rodger Combs]

One comment below; otherwise this LGTM.

> On Jun 2, 2016, at 01:32, Rick Kern <kernrj at gmail.com> wrote:
> 
> AudioConverterFillComplexBuffer() doesn't always call its callback. A frame
> queue is used to prevent skipped audio samples.
> 
> Signed-off-by: Rick Kern <kernrj at gmail.com>
> ---
> libavcodec/audiotoolboxenc.c | 78 +++++++++++++++++++++++++++++---------------
> 1 file changed, 52 insertions(+), 26 deletions(-)
> 
> diff --git a/libavcodec/audiotoolboxenc.c b/libavcodec/audiotoolboxenc.c
> index 855df0c..262308a 100644
> --- a/libavcodec/audiotoolboxenc.c
> +++ b/libavcodec/audiotoolboxenc.c
> @@ -22,6 +22,9 @@
> 
> #include <AudioToolbox/AudioToolbox.h>
> 
> +#define FF_BUFQUEUE_SIZE 256
> +#include "libavfilter/bufferqueue.h"
> +
> #include "config.h"
> #include "audio_frame_queue.h"
> #include "avcodec.h"
> @@ -38,8 +41,8 @@ typedef struct ATDecodeContext {
>     int quality;
> 
>     AudioConverterRef converter;
> -    AVFrame in_frame;
> -    AVFrame new_in_frame;
> +    struct FFBufQueue frame_queue;
> +    struct FFBufQueue used_frame_queue;
> 
>     unsigned pkt_size;
>     AudioFrameQueue afq;
> @@ -449,28 +452,30 @@ static OSStatus ffat_encode_callback(AudioConverterRef converter, UInt32 *nb_pac
> {
>     AVCodecContext *avctx = inctx;
>     ATDecodeContext *at = avctx->priv_data;
> +    AVFrame *frame;
> 
> -    if (at->eof) {
> -        *nb_packets = 0;
> -        return 0;
> +    if (!at->frame_queue.available) {
> +        if (at->eof) {
> +            *nb_packets = 0;
> +            return 0;
> +        } else {
> +            *nb_packets = 0;
> +            return 1;
> +        }
>     }
> 
> -    av_frame_unref(&at->in_frame);
> -    av_frame_move_ref(&at->in_frame, &at->new_in_frame);
> -
> -    if (!at->in_frame.data[0]) {
> -        *nb_packets = 0;
> -        return 1;
> -    }
> +    frame = ff_bufqueue_get(&at->frame_queue);
> 
>     data->mNumberBuffers              = 1;
>     data->mBuffers[0].mNumberChannels = avctx->channels;
> -    data->mBuffers[0].mDataByteSize   = at->in_frame.nb_samples *
> +    data->mBuffers[0].mDataByteSize   = frame->nb_samples *
>                                         av_get_bytes_per_sample(avctx->sample_fmt) *
>                                         avctx->channels;
> -    data->mBuffers[0].mData           = at->in_frame.data[0];
> -    if (*nb_packets > at->in_frame.nb_samples)
> -        *nb_packets = at->in_frame.nb_samples;
> +    data->mBuffers[0].mData           = frame->data[0];
> +    if (*nb_packets > frame->nb_samples)
> +        *nb_packets = frame->nb_samples;
> +
> +    ff_bufqueue_add(avctx, &at->used_frame_queue, frame);
> 
>     return 0;
> }
> @@ -492,20 +497,38 @@ static int ffat_encode(AVCodecContext *avctx, AVPacket *avpkt,
>     };
>     AudioStreamPacketDescription out_pkt_desc = {0};
> 
> -    if ((ret = ff_alloc_packet2(avctx, avpkt, at->pkt_size, 0)) < 0)
> -        return ret;
> -
> -    av_frame_unref(&at->new_in_frame);
> -
>     if (frame) {
> +        AVFrame *in_frame;
> +
> +        if (ff_bufqueue_is_full(&at->frame_queue)) {
> +            /*
> +             * The frame queue is significantly larger than needed in practice,
> +             * but no clear way to determine the minimum number of samples to
> +             * get output from AudioConverterFillComplexBuffer().
> +             */
> +            av_log(avctx, AV_LOG_ERROR, "Bug: frame queue is too small.\n");
> +            return AVERROR_BUG;
> +        }
> +
>         if ((ret = ff_af_queue_add(&at->afq, frame)) < 0)
>             return ret;
> -        if ((ret = av_frame_ref(&at->new_in_frame, frame)) < 0)
> +
> +        in_frame = av_frame_alloc();
> +        if (!in_frame)
> +            return AVERROR(ENOMEM);
> +
> +        if ((ret = av_frame_ref(in_frame, frame)) < 0)
>             return ret;
Looks like you can just use av_frame_clone(frame) here, which also avoids a leak if av_frame_ref() fails.

> +
> +        ff_bufqueue_add(avctx, &at->frame_queue, in_frame);
>     } else {
>         at->eof = 1;
>     }
> 
> +    if ((ret = ff_alloc_packet2(avctx, avpkt, at->pkt_size, 0)) < 0)
> +        return ret;
> +
> +
>     out_buffers.mBuffers[0].mData = avpkt->data;
> 
>     *got_packet_ptr = avctx->frame_size / at->frame_size;
> @@ -513,6 +536,9 @@ static int ffat_encode(AVCodecContext *avctx, AVPacket *avpkt,
>     ret = AudioConverterFillComplexBuffer(at->converter, ffat_encode_callback, avctx,
>                                           got_packet_ptr, &out_buffers,
>                                           (avctx->frame_size > at->frame_size) ? NULL : &out_pkt_desc);
> +
> +    ff_bufqueue_discard_all(&at->used_frame_queue);
> +
>     if ((!ret || ret == 1) && *got_packet_ptr) {
>         avpkt->size = out_buffers.mBuffers[0].mDataByteSize;
>         ff_af_queue_remove(&at->afq, out_pkt_desc.mVariableFramesInPacket ?
> @@ -531,16 +557,16 @@ static av_cold void ffat_encode_flush(AVCodecContext *avctx)
> {
>     ATDecodeContext *at = avctx->priv_data;
>     AudioConverterReset(at->converter);
> -    av_frame_unref(&at->new_in_frame);
> -    av_frame_unref(&at->in_frame);
> +    ff_bufqueue_discard_all(&at->frame_queue);
> +    ff_bufqueue_discard_all(&at->used_frame_queue);
> }
> 
> static av_cold int ffat_close_encoder(AVCodecContext *avctx)
> {
>     ATDecodeContext *at = avctx->priv_data;
>     AudioConverterDispose(at->converter);
> -    av_frame_unref(&at->new_in_frame);
> -    av_frame_unref(&at->in_frame);
> +    ff_bufqueue_discard_all(&at->frame_queue);
> +    ff_bufqueue_discard_all(&at->used_frame_queue);
>     ff_af_queue_close(&at->afq);
>     return 0;
> }
> -- 
> 2.7.4
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3579 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160609/3f598955/attachment.bin>