| Message ID | 20200205110708.64282-3-martin@martin.st |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/3] libfdk-aacdec: Apply the decoder's output delay on timestamps | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On 2/5/2020 8:07 AM, Martin Storsjö wrote: > Previously, it was always left in the automatic mode, if the option > was set to the only special (negative) value. Now there's two separate > special values for this option, -1 for automatic (metadata based) > and -2 for explicitly disabled. > --- > libavcodec/libfdk-aacdec.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/libfdk-aacdec.c b/libavcodec/libfdk-aacdec.c > index 32a97958c4..cc50fdce2f 100644 > --- a/libavcodec/libfdk-aacdec.c > +++ b/libavcodec/libfdk-aacdec.c > @@ -76,8 +76,8 @@ static const AVOption fdk_aac_dec_options[] = { > OFFSET(drc_boost), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 127, AD, NULL }, > { "drc_cut", "Dynamic Range Control: attenuation factor, where [0] is none and [127] is max compression", > OFFSET(drc_cut), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 127, AD, NULL }, > - { "drc_level", "Dynamic Range Control: reference level, quantized to 0.25dB steps where [0] is 0dB and [127] is -31.75dB", > - OFFSET(drc_level), AV_OPT_TYPE_INT, { .i64 = -1}, -1, 127, AD, NULL }, > + { "drc_level", "Dynamic Range Control: reference level, quantized to 0.25dB steps where [0] is 0dB and [127] is -31.75dB, -1 for auto, and -2 for disabled", Not against adding this extra explanation here since this one is not a simple boolean option, but i see there's no entry for fdk-aacdec in decoders.texy, so could you add one with all the options described in it? Bump micro, and LGTM. > + OFFSET(drc_level), AV_OPT_TYPE_INT, { .i64 = -1}, -2, 127, AD, NULL }, > { "drc_heavy", "Dynamic Range Control: heavy compression, where [1] is on (RF mode) and [0] is off", > OFFSET(drc_heavy), AV_OPT_TYPE_INT, { .i64 = -1}, -1, 1, AD, NULL }, > #if FDKDEC_VER_AT_LEAST(2, 5) // 2.5.10 > @@ -298,6 +298,12 @@ static av_cold int fdk_aac_decode_init(AVCodecContext *avctx) > } > > if (s->drc_level != -1) { > + // This option defaults to -1, i.e. not calling > + // aacDecoder_SetParam(AAC_DRC_REFERENCE_LEVEL) at all, which defaults > + // to the level from DRC metadata, if available. The user can set > + // -drc_level -2, which calls aacDecoder_SetParam( > + // AAC_DRC_REFERENCE_LEVEL) with a negative value, which then > + // explicitly disables the feature. > if (aacDecoder_SetParam(s->handle, AAC_DRC_REFERENCE_LEVEL, s->drc_level) != AAC_DEC_OK) { > av_log(avctx, AV_LOG_ERROR, "Unable to set DRC reference level in the decoder\n"); > return AVERROR_UNKNOWN; >
On Wed, 5 Feb 2020, James Almer wrote: > On 2/5/2020 8:07 AM, Martin Storsjö wrote: >> Previously, it was always left in the automatic mode, if the option >> was set to the only special (negative) value. Now there's two separate >> special values for this option, -1 for automatic (metadata based) >> and -2 for explicitly disabled. >> --- >> libavcodec/libfdk-aacdec.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/libavcodec/libfdk-aacdec.c b/libavcodec/libfdk-aacdec.c >> index 32a97958c4..cc50fdce2f 100644 >> --- a/libavcodec/libfdk-aacdec.c >> +++ b/libavcodec/libfdk-aacdec.c >> @@ -76,8 +76,8 @@ static const AVOption fdk_aac_dec_options[] = { >> OFFSET(drc_boost), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 127, AD, NULL }, >> { "drc_cut", "Dynamic Range Control: attenuation factor, where [0] is none and [127] is max compression", >> OFFSET(drc_cut), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 127, AD, NULL }, >> - { "drc_level", "Dynamic Range Control: reference level, quantized to 0.25dB steps where [0] is 0dB and [127] is -31.75dB", >> - OFFSET(drc_level), AV_OPT_TYPE_INT, { .i64 = -1}, -1, 127, AD, NULL }, >> + { "drc_level", "Dynamic Range Control: reference level, quantized to 0.25dB steps where [0] is 0dB and [127] is -31.75dB, -1 for auto, and -2 for disabled", > > Not against adding this extra explanation here since this one is not a > simple boolean option, but i see there's no entry for fdk-aacdec in > decoders.texy, so could you add one with all the options described in it? I'd say that's a separate issue from these patches - I could look into doing that, but that would be a later step. // Martin
On 2/5/2020 10:12 AM, Martin Storsjö wrote: > On Wed, 5 Feb 2020, James Almer wrote: > >> On 2/5/2020 8:07 AM, Martin Storsjö wrote: >>> Previously, it was always left in the automatic mode, if the option >>> was set to the only special (negative) value. Now there's two separate >>> special values for this option, -1 for automatic (metadata based) >>> and -2 for explicitly disabled. >>> --- >>> libavcodec/libfdk-aacdec.c | 10 ++++++++-- >>> 1 file changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavcodec/libfdk-aacdec.c b/libavcodec/libfdk-aacdec.c >>> index 32a97958c4..cc50fdce2f 100644 >>> --- a/libavcodec/libfdk-aacdec.c >>> +++ b/libavcodec/libfdk-aacdec.c >>> @@ -76,8 +76,8 @@ static const AVOption fdk_aac_dec_options[] = { >>> OFFSET(drc_boost), AV_OPT_TYPE_INT, { >>> .i64 = -1 }, -1, 127, AD, NULL }, >>> { "drc_cut", "Dynamic Range Control: attenuation factor, where >>> [0] is none and [127] is max compression", >>> OFFSET(drc_cut), AV_OPT_TYPE_INT, { >>> .i64 = -1 }, -1, 127, AD, NULL }, >>> - { "drc_level", "Dynamic Range Control: reference level, >>> quantized to 0.25dB steps where [0] is 0dB and [127] is -31.75dB", >>> - OFFSET(drc_level), AV_OPT_TYPE_INT, { >>> .i64 = -1}, -1, 127, AD, NULL }, >>> + { "drc_level", "Dynamic Range Control: reference level, >>> quantized to 0.25dB steps where [0] is 0dB and [127] is -31.75dB, -1 >>> for auto, and -2 for disabled", >> >> Not against adding this extra explanation here since this one is not a >> simple boolean option, but i see there's no entry for fdk-aacdec in >> decoders.texy, so could you add one with all the options described in it? > > I'd say that's a separate issue from these patches - I could look into > doing that, but that would be a later step. Yes, it's a request, not a blocking comment. > > // 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".
diff --git a/libavcodec/libfdk-aacdec.c b/libavcodec/libfdk-aacdec.c index 32a97958c4..cc50fdce2f 100644 --- a/libavcodec/libfdk-aacdec.c +++ b/libavcodec/libfdk-aacdec.c @@ -76,8 +76,8 @@ static const AVOption fdk_aac_dec_options[] = { OFFSET(drc_boost), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 127, AD, NULL }, { "drc_cut", "Dynamic Range Control: attenuation factor, where [0] is none and [127] is max compression", OFFSET(drc_cut), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 127, AD, NULL }, - { "drc_level", "Dynamic Range Control: reference level, quantized to 0.25dB steps where [0] is 0dB and [127] is -31.75dB", - OFFSET(drc_level), AV_OPT_TYPE_INT, { .i64 = -1}, -1, 127, AD, NULL }, + { "drc_level", "Dynamic Range Control: reference level, quantized to 0.25dB steps where [0] is 0dB and [127] is -31.75dB, -1 for auto, and -2 for disabled", + OFFSET(drc_level), AV_OPT_TYPE_INT, { .i64 = -1}, -2, 127, AD, NULL }, { "drc_heavy", "Dynamic Range Control: heavy compression, where [1] is on (RF mode) and [0] is off", OFFSET(drc_heavy), AV_OPT_TYPE_INT, { .i64 = -1}, -1, 1, AD, NULL }, #if FDKDEC_VER_AT_LEAST(2, 5) // 2.5.10 @@ -298,6 +298,12 @@ static av_cold int fdk_aac_decode_init(AVCodecContext *avctx) } if (s->drc_level != -1) { + // This option defaults to -1, i.e. not calling + // aacDecoder_SetParam(AAC_DRC_REFERENCE_LEVEL) at all, which defaults + // to the level from DRC metadata, if available. The user can set + // -drc_level -2, which calls aacDecoder_SetParam( + // AAC_DRC_REFERENCE_LEVEL) with a negative value, which then + // explicitly disables the feature. if (aacDecoder_SetParam(s->handle, AAC_DRC_REFERENCE_LEVEL, s->drc_level) != AAC_DEC_OK) { av_log(avctx, AV_LOG_ERROR, "Unable to set DRC reference level in the decoder\n"); return AVERROR_UNKNOWN;