[FFmpeg-devel] [PATCH v2] lavc/audiotoolboxenc: fix noise in encoded audio
James Almer
jamrial at gmail.com
Tue Jan 2 17:31:46 EET 2018
On 1/2/2018 12:03 PM, zhangjiejun1992 at gmail.com wrote:
> From: Jiejun Zhang <zhangjiejun1992 at gmail.com>
>
> This fixes #6940
>
> Although undocumented, AudioToolbox seems to require the data supplied
> by the callback (i.e. ffat_encode_callback) being unchanged until the
> next time the callback is called. In the old implementation, the
> AVBuffer backing the frame is recycled after the frame is freed, and
> somebody else (maybe the decoder) will write into the AVBuffer and
> change the data. AudioToolbox then encodes some wrong data and noise
> is produced. Copying the data to a separate buffer solves this
> problem.
> ---
> libavcodec/audiotoolboxenc.c | 32 +++++++++++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/audiotoolboxenc.c b/libavcodec/audiotoolboxenc.c
> index 71885d1530..dcac88cdde 100644
> --- a/libavcodec/audiotoolboxenc.c
> +++ b/libavcodec/audiotoolboxenc.c
> @@ -48,6 +48,9 @@ typedef struct ATDecodeContext {
> AudioFrameQueue afq;
> int eof;
> int frame_size;
> +
> + uint8_t* audio_data_buf;
> + uint32_t audio_data_buf_size;
> } ATDecodeContext;
>
> static UInt32 ffat_get_format_id(enum AVCodecID codec, int profile)
> @@ -442,6 +445,9 @@ static av_cold int ffat_init_encoder(AVCodecContext *avctx)
>
> ff_af_queue_init(avctx, &at->afq);
>
> + at->audio_data_buf_size = 0;
> + at->audio_data_buf = NULL;
The entire ATDecodeContext struct is zero initialized when allocated, so
this is not needed.
> +
> return 0;
> }
>
> @@ -465,13 +471,27 @@ static OSStatus ffat_encode_callback(AudioConverterRef converter, UInt32 *nb_pac
> }
>
> frame = ff_bufqueue_get(&at->frame_queue);
> -
> + int audio_data_size = frame->nb_samples *
> + av_get_bytes_per_sample(avctx->sample_fmt) *
> + avctx->channels;
> + if (at->audio_data_buf_size < audio_data_size) {
> + av_log(avctx, AV_LOG_INFO, "Increasing audio data buffer size to %d\n",
Verbose or debug level, please.
> + audio_data_size);
> + av_free(at->audio_data_buf);
> + at->audio_data_buf_size = audio_data_size;
> + at->audio_data_buf = av_malloc(at->audio_data_buf_size);
> + if (!at->audio_data_buf) {
> + at->audio_data_buf_size = 0;
> + data->mNumberBuffers = 0;
> + *nb_packets = 0;
> + return AVERROR(ENOMEM);
> + }
> + }
> data->mNumberBuffers = 1;
> data->mBuffers[0].mNumberChannels = avctx->channels;
> - data->mBuffers[0].mDataByteSize = frame->nb_samples *
> - av_get_bytes_per_sample(avctx->sample_fmt) *
> - avctx->channels;
> - data->mBuffers[0].mData = frame->data[0];
> + data->mBuffers[0].mDataByteSize = audio_data_size;
> + data->mBuffers[0].mData = at->audio_data_buf;
> + memcpy(at->audio_data_buf, frame->data[0], data->mBuffers[0].mDataByteSize);
Can't you instead create a new reference for the frame buffer? Or will
making it non writable break things further into the process? It would
save you a memcpy per frame.
> if (*nb_packets > frame->nb_samples)
> *nb_packets = frame->nb_samples;
>
> @@ -565,6 +585,8 @@ static av_cold int ffat_close_encoder(AVCodecContext *avctx)
> ff_bufqueue_discard_all(&at->frame_queue);
> ff_bufqueue_discard_all(&at->used_frame_queue);
> ff_af_queue_close(&at->afq);
> + at->audio_data_buf_size = 0;
> + av_freep(&at->audio_data_buf);
> return 0;
> }
>
>
More information about the ffmpeg-devel
mailing list