Message ID | 20200205130652.75949-1-martin@martin.st |
---|---|
State | Accepted |
Headers | show |
Series | None | expand |
On 2/5/2020 10:06 AM, Martin Storsjö wrote: > It was disabled by default in 2dbd35b00c6433e587d5f44d5dbc8972ebbaa88e > as it added delay, but now we compensate for the delay properly > by offsetting timestamps. > --- > libavcodec/libfdk-aacdec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/libfdk-aacdec.c b/libavcodec/libfdk-aacdec.c > index d9b080cf3e..b43226f515 100644 > --- a/libavcodec/libfdk-aacdec.c > +++ b/libavcodec/libfdk-aacdec.c > @@ -81,7 +81,8 @@ static const AVOption fdk_aac_dec_options[] = { > { "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 > - { "level_limit", "Signal level limiting", OFFSET(level_limit), AV_OPT_TYPE_INT, { .i64 = 0 }, -1, 1, AD }, > + { "level_limit", "Signal level limiting", > + OFFSET(level_limit), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, AD }, > #endif > #if FDKDEC_VER_AT_LEAST(3, 0) // 3.0.0 > { "drc_effect","Dynamic Range Control: effect type, where e.g. [0] is none and [6] is general", Meant to reply to this patch, sorry. Quoting here: > You need to check that level_limit != -1, same as all the drc_* options > next to it in fdk_aac_decode_init(). Otherwise you'll call > aacDecoder_SetParam() with that value that's only meant to be > interpreted by this wrapper as "Let the decoder use its default".
On Fri, 7 Feb 2020, James Almer wrote: > On 2/5/2020 10:06 AM, Martin Storsjö wrote: >> It was disabled by default in 2dbd35b00c6433e587d5f44d5dbc8972ebbaa88e >> as it added delay, but now we compensate for the delay properly >> by offsetting timestamps. >> --- >> libavcodec/libfdk-aacdec.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/libavcodec/libfdk-aacdec.c b/libavcodec/libfdk-aacdec.c >> index d9b080cf3e..b43226f515 100644 >> --- a/libavcodec/libfdk-aacdec.c >> +++ b/libavcodec/libfdk-aacdec.c >> @@ -81,7 +81,8 @@ static const AVOption fdk_aac_dec_options[] = { >> { "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 >> - { "level_limit", "Signal level limiting", OFFSET(level_limit), AV_OPT_TYPE_INT, { .i64 = 0 }, -1, 1, AD }, >> + { "level_limit", "Signal level limiting", >> + OFFSET(level_limit), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, AD }, >> #endif >> #if FDKDEC_VER_AT_LEAST(3, 0) // 3.0.0 >> { "drc_effect","Dynamic Range Control: effect type, where e.g. [0] is none and [6] is general", > > Meant to reply to this patch, sorry. Quoting here: > >> You need to check that level_limit != -1, same as all the drc_* options >> next to it in fdk_aac_decode_init(). Otherwise you'll call >> aacDecoder_SetParam() with that value that's only meant to be >> interpreted by this wrapper as "Let the decoder use its default". The code already has this treatment of -1, see line 296 in current master. // Martin
On 2/7/2020 3:04 PM, Martin Storsjö wrote: > On Fri, 7 Feb 2020, James Almer wrote: > >> On 2/5/2020 10:06 AM, Martin Storsjö wrote: >>> It was disabled by default in 2dbd35b00c6433e587d5f44d5dbc8972ebbaa88e >>> as it added delay, but now we compensate for the delay properly >>> by offsetting timestamps. >>> --- >>> libavcodec/libfdk-aacdec.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/libfdk-aacdec.c b/libavcodec/libfdk-aacdec.c >>> index d9b080cf3e..b43226f515 100644 >>> --- a/libavcodec/libfdk-aacdec.c >>> +++ b/libavcodec/libfdk-aacdec.c >>> @@ -81,7 +81,8 @@ static const AVOption fdk_aac_dec_options[] = { >>> { "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 >>> - { "level_limit", "Signal level limiting", OFFSET(level_limit), >>> AV_OPT_TYPE_INT, { .i64 = 0 }, -1, 1, AD }, >>> + { "level_limit", "Signal level limiting", >>> + OFFSET(level_limit), AV_OPT_TYPE_BOOL, { >>> .i64 = -1 }, -1, 1, AD }, >>> #endif >>> #if FDKDEC_VER_AT_LEAST(3, 0) // 3.0.0 >>> { "drc_effect","Dynamic Range Control: effect type, where e.g. >>> [0] is none and [6] is general", >> >> Meant to reply to this patch, sorry. Quoting here: >> >>> You need to check that level_limit != -1, same as all the drc_* options >>> next to it in fdk_aac_decode_init(). Otherwise you'll call >>> aacDecoder_SetParam() with that value that's only meant to be >>> interpreted by this wrapper as "Let the decoder use its default". > > The code already has this treatment of -1, see line 296 in current master. That's for drc_level. level_limit is line 311.
On Fri, 7 Feb 2020, Martin Storsjö wrote: > On Fri, 7 Feb 2020, James Almer wrote: > >> On 2/5/2020 10:06 AM, Martin Storsjö wrote: >>> It was disabled by default in > 2dbd35b00c6433e587d5f44d5dbc8972ebbaa88e >>> as it added delay, but now we compensate for the delay > properly >>> by offsetting timestamps. >>> --- >>> libavcodec/libfdk-aacdec.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/libfdk-aacdec.c > b/libavcodec/libfdk-aacdec.c >>> index d9b080cf3e..b43226f515 100644 >>> --- a/libavcodec/libfdk-aacdec.c >>> +++ b/libavcodec/libfdk-aacdec.c >>> @@ -81,7 +81,8 @@ static const AVOption > fdk_aac_dec_options[] = { >>> { "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 >>> - { "level_limit", "Signal level limiting", > OFFSET(level_limit), AV_OPT_TYPE_INT, { .i64 = 0 }, -1, 1, AD > }, >>> + { "level_limit", "Signal level limiting", >>> + OFFSET(level_limit), > AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, AD }, >>> #endif >>> #if FDKDEC_VER_AT_LEAST(3, 0) // 3.0.0 >>> { "drc_effect","Dynamic Range Control: effect type, > where e.g. [0] is none and [6] is general", >> >> Meant to reply to this patch, sorry. Quoting here: >> >>> You need to check that level_limit != -1, same as all the > drc_* options >>> next to it in fdk_aac_decode_init(). Otherwise you'll call >>> aacDecoder_SetParam() with that value that's only meant to > be >>> interpreted by this wrapper as "Let the decoder use its > default". > > The code already has this treatment of -1, see line 296 in > current master. Sorry, I misread, that was a different option. But -1 is actually treated by the decoder lib as explicit auto mode, which probably also is the default setting if the setting isn't set. // Martin
On 2/7/2020 3:08 PM, Martin Storsjö wrote: > On Fri, 7 Feb 2020, Martin Storsjö wrote: > >> On Fri, 7 Feb 2020, James Almer wrote: >> >>> On 2/5/2020 10:06 AM, Martin Storsjö wrote: >>>> It was disabled by default in >> 2dbd35b00c6433e587d5f44d5dbc8972ebbaa88e >>>> as it added delay, but now we compensate for the delay >> properly >>>> by offsetting timestamps. >>>> --- >>>> libavcodec/libfdk-aacdec.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/libavcodec/libfdk-aacdec.c >> b/libavcodec/libfdk-aacdec.c >>>> index d9b080cf3e..b43226f515 100644 >>>> --- a/libavcodec/libfdk-aacdec.c >>>> +++ b/libavcodec/libfdk-aacdec.c >>>> @@ -81,7 +81,8 @@ static const AVOption >> fdk_aac_dec_options[] = { >>>> { "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 >>>> - { "level_limit", "Signal level limiting", >> OFFSET(level_limit), AV_OPT_TYPE_INT, { .i64 = 0 }, -1, 1, AD }, >>>> + { "level_limit", "Signal level limiting", >>>> + OFFSET(level_limit), >> AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, AD }, >>>> #endif >>>> #if FDKDEC_VER_AT_LEAST(3, 0) // 3.0.0 >>>> { "drc_effect","Dynamic Range Control: effect type, >> where e.g. [0] is none and [6] is general", >>> >>> Meant to reply to this patch, sorry. Quoting here: >>> >>>> You need to check that level_limit != -1, same as all the >> drc_* options >>>> next to it in fdk_aac_decode_init(). Otherwise you'll call >>>> aacDecoder_SetParam() with that value that's only meant to >> be >>>> interpreted by this wrapper as "Let the decoder use its >> default". >> >> The code already has this treatment of -1, see line 296 in current >> master. > > Sorry, I misread, that was a different option. > > But -1 is actually treated by the decoder lib as explicit auto mode, > which probably also is the default setting if the setting isn't set. Ok, then LGTM. > > // 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 d9b080cf3e..b43226f515 100644 --- a/libavcodec/libfdk-aacdec.c +++ b/libavcodec/libfdk-aacdec.c @@ -81,7 +81,8 @@ static const AVOption fdk_aac_dec_options[] = { { "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 - { "level_limit", "Signal level limiting", OFFSET(level_limit), AV_OPT_TYPE_INT, { .i64 = 0 }, -1, 1, AD }, + { "level_limit", "Signal level limiting", + OFFSET(level_limit), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, AD }, #endif #if FDKDEC_VER_AT_LEAST(3, 0) // 3.0.0 { "drc_effect","Dynamic Range Control: effect type, where e.g. [0] is none and [6] is general",