From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id A791C459E1 for ; Tue, 28 Feb 2023 21:34:28 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F040968ACC3; Tue, 28 Feb 2023 23:34:26 +0200 (EET) Received: from mail8.parnet.fi (mail8.parnet.fi [77.234.108.134]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 332E7689213 for ; Tue, 28 Feb 2023 23:34:20 +0200 (EET) Received: from mail9.parnet.fi (mail9.parnet.fi [77.234.108.21]) by mail8.parnet.fi with ESMTP id 31SLYJYE026062-31SLYJYF026062; Tue, 28 Feb 2023 23:34:19 +0200 Received: from foo.martin.st (host-97-187.parnet.fi [77.234.97.187]) by mail9.parnet.fi (Postfix) with ESMTPS id 28AFAA143A; Tue, 28 Feb 2023 23:34:18 +0200 (EET) Date: Tue, 28 Feb 2023 23:34:18 +0200 (EET) From: =?ISO-8859-15?Q?Martin_Storsj=F6?= To: FFmpeg development discussions and patches In-Reply-To: <20230228212513.3976220-1-JonHGee@gmail.com> Message-ID: References: <826ff2b1-6de8-4811-5ad5-6f1c3b30662c@martin.st> <20230228212513.3976220-1-JonHGee@gmail.com> MIME-Version: 1.0 X-FE-Policy-ID: 3:14:2:SYSTEM Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/libfdk-aacenc: Enable writing DRC metadata X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: JonHGee Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Tue, 28 Feb 2023, JonHGee wrote: > Signed-off-by: JonHGee > --- > libavcodec/libfdk-aacenc.c | 69 +++++++++++++++++++++++++++++++------- > 1 file changed, 56 insertions(+), 13 deletions(-) > > diff --git a/libavcodec/libfdk-aacenc.c b/libavcodec/libfdk-aacenc.c > index 54549de473..123dabf3ae 100644 > --- a/libavcodec/libfdk-aacenc.c > +++ b/libavcodec/libfdk-aacenc.c > @@ -46,6 +46,13 @@ typedef struct AACContext { > int latm; > int header_period; > int vbr; > + int drc_profile; > + int drc_target_ref; > + int comp_profile; > + int comp_target_ref; > + int prog_ref; > + int metadata_mode; > + AACENC_MetaData metaDataSetup; > > AudioFrameQueue afq; > } AACContext; > @@ -64,6 +71,11 @@ static const AVOption aac_enc_options[] = { > { "latm", "Output LATM/LOAS encapsulated data", offsetof(AACContext, latm), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, > { "header_period", "StreamMuxConfig and PCE repetition period (in frames)", offsetof(AACContext, header_period), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 0xffff, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, > { "vbr", "VBR mode (1-5)", offsetof(AACContext, vbr), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 5, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, > + { "drc_profile", "The desired compression profile for AAC DRC", offsetof(AACContext, drc_profile), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 256, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, The indentation is still off here > + { "drc_target_ref", "Expected target reference level at decoder side in dB (for clipping prevention/limiter)", offsetof(AACContext, drc_target_ref), AV_OPT_TYPE_INT, { .i64 = 0.0 }, -31.75, 0, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, > + { "comp_profile", "The desired compression profile for AAC DRC", offsetof(AACContext, comp_profile), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 256, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, > + { "comp_target_ref", "Expected target reference level at decoder side in dB (for clipping prevention/limiter)", offsetof(AACContext, comp_target_ref), AV_OPT_TYPE_INT, { .i64 = 0.0 }, -31.75, 0, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, > + { "prog_ref", "The program reference level or dialog level in dB", offsetof(AACContext, prog_ref), AV_OPT_TYPE_INT, { .i64 = 0.0 }, -31.75, 0, AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM }, > FF_AAC_PROFILE_OPTS > { NULL } > }; > @@ -319,6 +331,30 @@ static av_cold int aac_encode_init(AVCodecContext *avctx) > } > } > > + s->metadata_mode = 0; > + if (s->prog_ref) { > + s->metadata_mode = 1; > + s->metaDataSetup.prog_ref_level_present = 1; > + s->metaDataSetup.prog_ref_level = s->prog_ref << 16; > + } > + if (s->drc_profile) { > + s->metadata_mode = 1; > + s->metaDataSetup.drc_profile = s->drc_profile; > + s->metaDataSetup.drc_TargetRefLevel = s->drc_target_ref << 16; > + if (s->comp_profile) { > + /* Including the comp_profile means that we need to set the mode to ETSI */ > + s->metadata_mode = 2; > + s->metaDataSetup.comp_profile = s->comp_profile; > + s->metaDataSetup.comp_TargetRefLevel = s->comp_target_ref << 16; > + } > + } > + > + if ((err = aacEncoder_SetParam(s->handle, AACENC_METADATA_MODE, s->metadata_mode)) != AACENC_OK) { > + av_log(avctx, AV_LOG_ERROR, "Unable to set metadata mode to %d: %s\n", > + s->metadata_mode, aac_get_error(err)); > + goto error; > + } > + > if ((err = aacEncEncode(s->handle, NULL, NULL, NULL, NULL)) != AACENC_OK) { > av_log(avctx, AV_LOG_ERROR, "Unable to initialize the encoder: %s\n", > aac_get_error(err)); > @@ -363,11 +399,13 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, > AACENC_BufDesc in_buf = { 0 }, out_buf = { 0 }; > AACENC_InArgs in_args = { 0 }; > AACENC_OutArgs out_args = { 0 }; > - int in_buffer_identifier = IN_AUDIO_DATA; > - int in_buffer_size, in_buffer_element_size; > + void* inBuffer[] = { 0, &s->metaDataSetup }; > + int in_buffer_identifiers[] = { IN_AUDIO_DATA, IN_METADATA_SETUP }; > + int in_buffer_element_sizes[] = { 2, sizeof(AACENC_MetaData) }; Still have the extra weird whitespace here > + int in_buffer_sizes[] = { 0 , sizeof(s->metaDataSetup) }; Extra space before comma > + void *out_ptr; > int out_buffer_identifier = OUT_BITSTREAM_DATA; > int out_buffer_size, out_buffer_element_size; > - void *in_ptr, *out_ptr; > int ret; > uint8_t dummy_buf[1]; > AACENC_ERROR err; > @@ -376,13 +414,12 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, > if (!frame) { > /* Must be a non-null pointer, even if it's a dummy. We could use > * the address of anything else on the stack as well. */ > - in_ptr = dummy_buf; > - in_buffer_size = 0; > + inBuffer[0] = dummy_buf; > > in_args.numInSamples = -1; > } else { > - in_ptr = frame->data[0]; > - in_buffer_size = 2 * avctx->ch_layout.nb_channels * frame->nb_samples; > + inBuffer[0] = frame->data[0]; > + in_buffer_sizes[0] = 2 * avctx->ch_layout.nb_channels * frame->nb_samples; > > in_args.numInSamples = avctx->ch_layout.nb_channels * frame->nb_samples; > > @@ -391,12 +428,18 @@ static int aac_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, > return ret; > } > > - in_buffer_element_size = 2; > - in_buf.numBufs = 1; > - in_buf.bufs = &in_ptr; > - in_buf.bufferIdentifiers = &in_buffer_identifier; > - in_buf.bufSizes = &in_buffer_size; > - in_buf.bufElSizes = &in_buffer_element_size; > + /* Only use audio input data if metadata mode is none. */ > + if (s->metadata_mode == 0) { > + in_buf.numBufs = 1; > + Weird leftover empty line here. And the comment above feels oddly worded - I don't feel it's needed at all anymore. > + } else { > + in_buf.numBufs = 2; > + } > + > + in_buf.bufs = (void**)inBuffer; > + in_buf.bufferIdentifiers = in_buffer_identifiers; > + in_buf.bufSizes = in_buffer_sizes; > + in_buf.bufElSizes = in_buffer_element_sizes; > > /* The maximum packet size is 6144 bits aka 768 bytes per channel. */ > ret = ff_alloc_packet(avctx, avpkt, FFMAX(8192, 768 * avctx->ch_layout.nb_channels)); > -- > 2.39.2.722.g9855ee24e9-goog Looks mostly good to me now, thanks. I guess I can clean up the remaining few nitpicks without you having to resubmit the patch. So unless you disagree with the comments, I can push it with the minor touch-ups. // Martin _______________________________________________ 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".