diff mbox series

[FFmpeg-devel] lavc/ac3dec: disable DRC by default

Message ID 20200201193443.22419-1-rcombs@rcombs.me
State New
Headers show
Series [FFmpeg-devel] lavc/ac3dec: disable DRC by default
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

rcombs Feb. 1, 2020, 7:34 p.m. UTC
This issue has been argued before, with the status quo being preserved under
the logic that the spec says this parameter is supposed to default to 1,
and that we should follow the spec. The spec was misguided, and thus so was
blindly following it.

The (E-)AC3 DRC architecture is a clever optimization: the (relatively) heavy
lifting of analyzing the audio data and calculating the per-block gain needed
is offloaded to the encoder, so that the player only needs to apply the gain
coefficients (at the desired scale) to get the DRC effect. In theory, this
seems like an efficient way to reduce the computational complexity of playback.
In practice, this isn't actually the case.

Actual (E-)AC3 is not guaranteed to have useful DRC metadata at all. For
instance, ffmpeg's encoder has never supported generating it. This means that
a user who wants DRC can't merely use the internal filter for (E-)AC3 and an
external one for other codecs; they have to use an external filter at al
times! They could also apply the internal filter, but it's hard to see what
benefit this would give.

I've also seen it argued that (E-)AC3 DRC does a better job of reproducing the
intent of the audio mastering engineer than playback-time DRC can. I don't
believe this argument is credible either. The official encoder does support
passing in custom gain values, but it doesn't appear that this capability is
frequently used. I'm not aware of any tooling that actually passes custom data
into the encoder's loudness parameters, and Dolby's first-party GUI tools don't
appear to support doing so. Dolby's own tools expose the parameters available in
the encoder itself, which comprise a set of 5 presets with no customizability:
"Film Standard", "Film Light", "Music Standard", "Music Light", and "Speech",
along with the null profile. These profiles are documented (minus their time
constants) in this PDF on the Dolby website:
https://www.dolby.com/us/en/technologies/a-guide-to-dolby-metadata.pdf
These are not advanced filters; they don't provide any complex control to the
content producer, who can't even tune the details or _know_ the time constants.
Even if the mastering engineer did want to customize the DRC with some custom
tool, in most cases I don't think they'd have the ability to do so because they
don't have access to the point in the pipeline where audio encoding occurs.
In TV, audio is often encoded live rather than streaming a pre-compressed
bitstream. In web streaming, masters are usually delivered with uncompressed
PCM audio and no loudness metadata, and encoded using stock configurations.
Film for DVD or Blu-Ray is the only plausible case, but again, I'm not aware
of any widely-used tooling for this with any more configurability than Dolby's.

Additional context: the AudioToolbox (E-)AC3 decoder (which is Dolby's) seems
not to apply DRC; you can see this by comparing the output of ffmpeg's decoder
with drc_scale set to 0 and 1 with the output of the (e)ac3_at decoder.

So to summarize:
- (E-)AC3 DRC is designed as an optimization for the decoder
- There's no inherent reason (E-)AC3 _requires_ DRC for playback more than any
  other codec
- Due to inconsistent encoding, it can't be relied upon
- There's no reason to believe it's better than decode-side DRC when it works
- It's not universally applied by other software

I think this makes a compelling case to stop applying (E-)AC3 DRC by default.
The user can enable it on their own if they prefer, but I believe having it off
by default will improve the apparent quality of (E-)AC3 playback for the vast
majority of ffmpeg users, and improve the consistency between (E-)AC3 playback
and playback of any other codec. Thus, this patch.
---
 libavcodec/ac3dec_fixed.c |  2 +-
 libavcodec/ac3dec_float.c |  2 +-
 tests/fate/ac3.mak        | 32 ++++++++++++++++----------------
 3 files changed, 18 insertions(+), 18 deletions(-)

Comments

Derek Buitenhuis Feb. 1, 2020, 7:43 p.m. UTC | #1
On 01/02/2020 19:34, rcombs wrote:
> This issue has been argued before, with the status quo being preserved under
> the logic that the spec says this parameter is supposed to default to 1,
> and that we should follow the spec. The spec was misguided, and thus so was
> blindly following it.

If "I don't agree with the spec" or "this spec does something stupid" were valid
arguments, we may as well not have specs at all, and FFmpeg would have a lot more
garbage in it. You can write at length why you think it's bad, but that does not
negeate the fact that it's the spec, and the defacto way to handle decoding AC-3.

And I'd rather not give Dolby more ammunition for their anti-FOSS FUD campaign.

For whatever my opinion is worth nowadays on this list, it's a very, very hard
NAK from me.

- Derek
James Almer Feb. 1, 2020, 7:50 p.m. UTC | #2
On 2/1/2020 4:34 PM, rcombs wrote:
> This issue has been argued before, with the status quo being preserved under
> the logic that the spec says this parameter is supposed to default to 1,
> and that we should follow the spec. The spec was misguided, and thus so was
> blindly following it.
> 
> The (E-)AC3 DRC architecture is a clever optimization: the (relatively) heavy
> lifting of analyzing the audio data and calculating the per-block gain needed
> is offloaded to the encoder, so that the player only needs to apply the gain
> coefficients (at the desired scale) to get the DRC effect. In theory, this
> seems like an efficient way to reduce the computational complexity of playback.
> In practice, this isn't actually the case.
> 
> Actual (E-)AC3 is not guaranteed to have useful DRC metadata at all. For
> instance, ffmpeg's encoder has never supported generating it. This means that
> a user who wants DRC can't merely use the internal filter for (E-)AC3 and an
> external one for other codecs; they have to use an external filter at al
> times! They could also apply the internal filter, but it's hard to see what
> benefit this would give.
> 
> I've also seen it argued that (E-)AC3 DRC does a better job of reproducing the
> intent of the audio mastering engineer than playback-time DRC can. I don't
> believe this argument is credible either. The official encoder does support
> passing in custom gain values, but it doesn't appear that this capability is
> frequently used. I'm not aware of any tooling that actually passes custom data
> into the encoder's loudness parameters, and Dolby's first-party GUI tools don't
> appear to support doing so. Dolby's own tools expose the parameters available in
> the encoder itself, which comprise a set of 5 presets with no customizability:
> "Film Standard", "Film Light", "Music Standard", "Music Light", and "Speech",
> along with the null profile. These profiles are documented (minus their time
> constants) in this PDF on the Dolby website:
> https://www.dolby.com/us/en/technologies/a-guide-to-dolby-metadata.pdf
> These are not advanced filters; they don't provide any complex control to the
> content producer, who can't even tune the details or _know_ the time constants.
> Even if the mastering engineer did want to customize the DRC with some custom
> tool, in most cases I don't think they'd have the ability to do so because they
> don't have access to the point in the pipeline where audio encoding occurs.
> In TV, audio is often encoded live rather than streaming a pre-compressed
> bitstream. In web streaming, masters are usually delivered with uncompressed
> PCM audio and no loudness metadata, and encoded using stock configurations.
> Film for DVD or Blu-Ray is the only plausible case, but again, I'm not aware
> of any widely-used tooling for this with any more configurability than Dolby's.
> 
> Additional context: the AudioToolbox (E-)AC3 decoder (which is Dolby's) seems
> not to apply DRC; you can see this by comparing the output of ffmpeg's decoder
> with drc_scale set to 0 and 1 with the output of the (e)ac3_at decoder.
> 
> So to summarize:
> - (E-)AC3 DRC is designed as an optimization for the decoder
> - There's no inherent reason (E-)AC3 _requires_ DRC for playback more than any
>   other codec
> - Due to inconsistent encoding, it can't be relied upon
> - There's no reason to believe it's better than decode-side DRC when it works
> - It's not universally applied by other software
> 
> I think this makes a compelling case to stop applying (E-)AC3 DRC by default.
> The user can enable it on their own if they prefer, but I believe having it off
> by default will improve the apparent quality of (E-)AC3 playback for the vast
> majority of ffmpeg users, and improve the consistency between (E-)AC3 playback
> and playback of any other codec. Thus, this patch.
> ---
>  libavcodec/ac3dec_fixed.c |  2 +-
>  libavcodec/ac3dec_float.c |  2 +-
>  tests/fate/ac3.mak        | 32 ++++++++++++++++----------------
>  3 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/libavcodec/ac3dec_fixed.c b/libavcodec/ac3dec_fixed.c
> index bd66175d50..312616edfd 100644
> --- a/libavcodec/ac3dec_fixed.c
> +++ b/libavcodec/ac3dec_fixed.c
> @@ -169,7 +169,7 @@ static void ac3_downmix_c_fixed16(int16_t **samples, int16_t **matrix,
>  
>  static const AVOption options[] = {
>      { "cons_noisegen", "enable consistent noise generation", OFFSET(consistent_noise_generation), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR },
> -    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 1.0}, 0.0, 6.0, PAR },
> +    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 0.0}, 0.0, 6.0, PAR },
>      { "heavy_compr", "enable heavy dynamic range compression", OFFSET(heavy_compression), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR },
>      { NULL},
>  };
> diff --git a/libavcodec/ac3dec_float.c b/libavcodec/ac3dec_float.c
> index b85a4ce336..69bd47aa6a 100644
> --- a/libavcodec/ac3dec_float.c
> +++ b/libavcodec/ac3dec_float.c
> @@ -33,7 +33,7 @@
>  
>  static const AVOption options[] = {
>      { "cons_noisegen", "enable consistent noise generation", OFFSET(consistent_noise_generation), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR },
> -    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 1.0}, 0.0, 6.0, PAR },
> +    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 0.0}, 0.0, 6.0, PAR },
>      { "heavy_compr", "enable heavy dynamic range compression", OFFSET(heavy_compression), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR },
>      { "target_level", "target level in -dBFS (0 not applied)", OFFSET(target_level), AV_OPT_TYPE_INT, {.i64 = 0 }, -31, 0, PAR },
>  
> diff --git a/tests/fate/ac3.mak b/tests/fate/ac3.mak
> index bb02d3829d..c810b53ad2 100644
> --- a/tests/fate/ac3.mak
> +++ b/tests/fate/ac3.mak
> @@ -1,67 +1,67 @@
>  FATE_AC3 += fate-ac3-2.0
> -fate-ac3-2.0: CMD = pcm -i $(TARGET_SAMPLES)/ac3/monsters_inc_2.0_192_small.ac3
> +fate-ac3-2.0: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/ac3/monsters_inc_2.0_192_small.ac3
>  fate-ac3-2.0: REF = $(SAMPLES)/ac3/monsters_inc_2.0_192_small_v2.pcm
>  
>  FATE_AC3 += fate-ac3-4.0
> -fate-ac3-4.0: CMD = pcm -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
> +fate-ac3-4.0: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
>  fate-ac3-4.0: REF = $(SAMPLES)/ac3/millers_crossing_4.0_v2.pcm
>  
>  #request_channel_layout 4 -> front channel
>  FATE_AC3 += fate-ac3-4.0-downmix-mono
> -fate-ac3-4.0-downmix-mono: CMD = pcm -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
> +fate-ac3-4.0-downmix-mono: CMD = pcm -drc_scale 1 -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
>  fate-ac3-4.0-downmix-mono: REF = $(SAMPLES)/ac3/millers_crossing_4.0_mono_v2.pcm
>  
>  #request_channel_layout 3 -> left channel + right channel
>  FATE_AC3 += fate-ac3-4.0-downmix-stereo
> -fate-ac3-4.0-downmix-stereo: CMD = pcm -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
> +fate-ac3-4.0-downmix-stereo: CMD = pcm -drc_scale 1 -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
>  fate-ac3-4.0-downmix-stereo: REF = $(SAMPLES)/ac3/millers_crossing_4.0_stereo_v2.pcm
>  
>  FATE_AC3 += fate-ac3-5.1
> -fate-ac3-5.1: CMD = pcm -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
> +fate-ac3-5.1: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
>  fate-ac3-5.1: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_v2.pcm
>  
>  FATE_AC3 += fate-ac3-5.1-downmix-mono
> -fate-ac3-5.1-downmix-mono: CMD = pcm -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
> +fate-ac3-5.1-downmix-mono: CMD = pcm -drc_scale 1 -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
>  fate-ac3-5.1-downmix-mono: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_mono_v2.pcm
>  
>  FATE_AC3 += fate-ac3-5.1-downmix-stereo
> -fate-ac3-5.1-downmix-stereo: CMD = pcm -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
> +fate-ac3-5.1-downmix-stereo: CMD = pcm -drc_scale 1 -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
>  fate-ac3-5.1-downmix-stereo: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_stereo_v2.pcm
>  
>  FATE_AC3 += fate-ac3-fixed-2.0
> -fate-ac3-fixed-2.0: CMD = pcm -c ac3_fixed -i $(TARGET_SAMPLES)/ac3/monsters_inc_2.0_192_small.ac3
> +fate-ac3-fixed-2.0: CMD = pcm -drc_scale 1 -c ac3_fixed -i $(TARGET_SAMPLES)/ac3/monsters_inc_2.0_192_small.ac3
>  fate-ac3-fixed-2.0: REF = $(SAMPLES)/ac3/monsters_inc_2.0_192_small_v2.pcm
>  
>  FATE_AC3 += fate-ac3-fixed-4.0-downmix-mono
> -fate-ac3-fixed-4.0-downmix-mono: CMD = pcm -c ac3_fixed -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
> +fate-ac3-fixed-4.0-downmix-mono: CMD = pcm -drc_scale 1 -c ac3_fixed -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
>  fate-ac3-fixed-4.0-downmix-mono: REF = $(SAMPLES)/ac3/millers_crossing_4.0_mono_v2.pcm
>  
>  FATE_AC3 += fate-ac3-fixed-5.1-downmix-mono
> -fate-ac3-fixed-5.1-downmix-mono: CMD = pcm -c ac3_fixed -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
> +fate-ac3-fixed-5.1-downmix-mono: CMD = pcm -drc_scale 1 -c ac3_fixed -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
>  fate-ac3-fixed-5.1-downmix-mono: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_mono_v2.pcm
>  
>  FATE_AC3 += fate-ac3-fixed-5.1-downmix-stereo
> -fate-ac3-fixed-5.1-downmix-stereo: CMD = pcm -c ac3_fixed -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
> +fate-ac3-fixed-5.1-downmix-stereo: CMD = pcm -drc_scale 1 -c ac3_fixed -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
>  fate-ac3-fixed-5.1-downmix-stereo: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_stereo_v2.pcm
>  
>  FATE_EAC3 += fate-eac3-1
> -fate-eac3-1: CMD = pcm -i $(TARGET_SAMPLES)/eac3/csi_miami_5.1_256_spx_small.eac3
> +fate-eac3-1: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/csi_miami_5.1_256_spx_small.eac3
>  fate-eac3-1: REF = $(SAMPLES)/eac3/csi_miami_5.1_256_spx_small_v2.pcm
>  
>  FATE_EAC3 += fate-eac3-2
> -fate-eac3-2: CMD = pcm -i $(TARGET_SAMPLES)/eac3/csi_miami_stereo_128_spx_small.eac3
> +fate-eac3-2: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/csi_miami_stereo_128_spx_small.eac3
>  fate-eac3-2: REF = $(SAMPLES)/eac3/csi_miami_stereo_128_spx_small_v2.pcm
>  
>  FATE_EAC3 += fate-eac3-3
> -fate-eac3-3: CMD = pcm -i $(TARGET_SAMPLES)/eac3/matrix2_commentary1_stereo_192_small.eac3
> +fate-eac3-3: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/matrix2_commentary1_stereo_192_small.eac3
>  fate-eac3-3: REF = $(SAMPLES)/eac3/matrix2_commentary1_stereo_192_small_v2.pcm
>  
>  FATE_EAC3 += fate-eac3-4
> -fate-eac3-4: CMD = pcm -i $(TARGET_SAMPLES)/eac3/serenity_english_5.1_1536_small.eac3
> +fate-eac3-4: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/serenity_english_5.1_1536_small.eac3
>  fate-eac3-4: REF = $(SAMPLES)/eac3/serenity_english_5.1_1536_small_v2.pcm
>  
>  FATE_EAC3 += fate-eac3-5
> -fate-eac3-5: CMD = pcm -i $(TARGET_SAMPLES)/eac3/the_great_wall_7.1.eac3
> +fate-eac3-5: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/the_great_wall_7.1.eac3
>  fate-eac3-5: REF = $(SAMPLES)/eac3/the_great_wall_7.1.pcm
>  
>  $(FATE_AC3) $(FATE_EAC3): CMP = oneoff

May want to rename these to fate-*-drc-1 and add new tests for drc 0
while at it.
I can create the reference pcm files for them and add them to the fate
sample suit, if you don't have access.
Paul B Mahol Feb. 1, 2020, 8:18 p.m. UTC | #3
On 2/1/20, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> On 01/02/2020 19:34, rcombs wrote:
>> This issue has been argued before, with the status quo being preserved
>> under
>> the logic that the spec says this parameter is supposed to default to 1,
>> and that we should follow the spec. The spec was misguided, and thus so
>> was
>> blindly following it.
>
> If "I don't agree with the spec" or "this spec does something stupid" were
> valid
> arguments, we may as well not have specs at all, and FFmpeg would have a lot
> more
> garbage in it. You can write at length why you think it's bad, but that does
> not
> negeate the fact that it's the spec, and the defacto way to handle decoding
> AC-3.
>
> And I'd rather not give Dolby more ammunition for their anti-FOSS FUD
> campaign.
>
> For whatever my opinion is worth nowadays on this list, it's a very, very
> hard
> NAK from me.

This does not have much logic or valid reasoning.

>
> - Derek
> _______________________________________________
> 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".
Derek Buitenhuis Feb. 1, 2020, 8:25 p.m. UTC | #4
On 01/02/2020 20:18, Paul B Mahol wrote:
> This does not have much logic or valid reasoning.

Please stop your constant vague troll replies and ad hominem attacks.

They are rampant on this mailing list; practically every thread, but
specifically against some people. It's getting ridiculous. It makes
ffmpeg-devel unwelcoming to others, and appear childish in nature.

I will not reply to any further mails from you of this nature, and I
would encourage also not to.

- Derek
Paul B Mahol Feb. 1, 2020, 8:37 p.m. UTC | #5
On 2/1/20, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> On 01/02/2020 20:18, Paul B Mahol wrote:
>> This does not have much logic or valid reasoning.
>
> Please stop your constant vague troll replies and ad hominem attacks.

I was not vague, your reasoning that because of dolby we need to comply to some
standards is not good.
Even dolby does not follow this as already mentioned on this same thread.

>
> They are rampant on this mailing list; practically every thread, but
> specifically against some people. It's getting ridiculous. It makes
> ffmpeg-devel unwelcoming to others, and appear childish in nature.
>

It is easy to deny truth. I'm not rampant at least not yet.

> I will not reply to any further mails from you of this nature, and I
> would encourage also not to.
>
> - Derek
> _______________________________________________
> 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".
Derek Buitenhuis Feb. 1, 2020, 8:47 p.m. UTC | #6
On 01/02/2020 20:37, Paul B Mahol wrote:
> I was not vague, your reasoning that because of dolby we need to comply to some
> standards is not good.
> Even dolby does not follow this as already mentioned on this same thread.

So write this in the email instead of one completely devoid of any reasoning.
Replies that say something is wrong or that you disagree, and not /why/ are not
useful to the author, nor are they effective at persuading others. I'm certainly
reminded of why I stopped frequenting this list.

Anyway, your argument is basically that we shouln't conform to specs depending
on who wrote them? That's arbitrary, and a poor precedent to set, in my opinion.
Further, AudioToolBox, while it may use an official decoder, is not distributed by
Dolby, nor, as far as I know, endorsed by them (they won't even tell you if it's
theirs inside, I believe). I suspect Apple set it up to not apply DRC, not Dolby.

The rest of your mail is more trolling, which I will not reply to.

So, that's my two cents on the subject - the rest of the list can decide if they
have merit or not - I am done replying to you, since you cannot act like an adult.

- Derek
Paul B Mahol Feb. 1, 2020, 8:53 p.m. UTC | #7
On 2/1/20, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> On 01/02/2020 20:37, Paul B Mahol wrote:
>> I was not vague, your reasoning that because of dolby we need to comply to
>> some
>> standards is not good.
>> Even dolby does not follow this as already mentioned on this same thread.
>
> So write this in the email instead of one completely devoid of any
> reasoning.
> Replies that say something is wrong or that you disagree, and not /why/ are
> not
> useful to the author, nor are they effective at persuading others. I'm
> certainly
> reminded of why I stopped frequenting this list.
>
> Anyway, your argument is basically that we shouln't conform to specs
> depending
> on who wrote them? That's arbitrary, and a poor precedent to set, in my
> opinion.
> Further, AudioToolBox, while it may use an official decoder, is not
> distributed by
> Dolby, nor, as far as I know, endorsed by them (they won't even tell you if
> it's
> theirs inside, I believe). I suspect Apple set it up to not apply DRC, not
> Dolby.
>
> The rest of your mail is more trolling, which I will not reply to.
>
> So, that's my two cents on the subject - the rest of the list can decide if
> they
> have merit or not - I am done replying to you, since you cannot act like an
> adult.

Easy pick, to stop discussion by ridiculing opponent.
rcombs Feb. 1, 2020, 9:03 p.m. UTC | #8
> On Feb 1, 2020, at 13:43, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> 
> On 01/02/2020 19:34, rcombs wrote:
>> This issue has been argued before, with the status quo being preserved under
>> the logic that the spec says this parameter is supposed to default to 1,
>> and that we should follow the spec. The spec was misguided, and thus so was
>> blindly following it.
> 
> If "I don't agree with the spec" or "this spec does something stupid" were valid
> arguments, we may as well not have specs at all, and FFmpeg would have a lot more
> garbage in it. You can write at length why you think it's bad, but that does not
> negeate the fact that it's the spec,

I'm not sure why the spec should have any bearing on the defaults of configurable options, particularly when those defaults result in unexpected behavior that's unlike that used for any other codec.

> and the defacto way to handle decoding AC-3.

Is it, though? My understanding is that AV receivers usually only enable AC3 DRC in "night mode" or the like, and few even provide a fully-tunable setting for it. Apple's decoder in AudioToolbox (implemented on top of Dolby's) doesn't provide the feature at all (I haven't checked what Win32 Media Foundation does yet). So where does this "de facto" situation come from, other than lavc itself?

Meanwhile, Dolby's own decoder applies digital dialog normalization by default (contrary to how the spec says it should be applied downstream), and I haven't seen anyone arguing that lavc should do that.

Additionally, TrueHD has a very similar DRC feature that lavc doesn't support whatsoever, and nobody seems to mind.

One other thing I neglected to mention in the commit message: the actual audio encoded in an (E)AC3 track often has levels similar to those of a track in another codec (PCM, AAC, etc) from the same or other sources, but sounds very different when both are decoded with lavc. For instance, Netflix's EAC3 sounds about the same as BD audio of the same content when decoded without DRC, but in lavc-based players by default, it sounds, well... compressed. This had led to an impression in the anime community that Netflix applies excessive DRC to their EAC3 streams at encode-time, and thus that those streams were low-quality and should be avoided.

> 
> And I'd rather not give Dolby more ammunition for their anti-FOSS FUD campaign.
> 
> For whatever my opinion is worth nowadays on this list, it's a very, very hard
> NAK from me.
> 
> - Derek
> _______________________________________________
> 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".
Derek Buitenhuis Feb. 1, 2020, 9:55 p.m. UTC | #9
On 01/02/2020 21:03, rcombs wrote:
> 
>> If "I don't agree with the spec" or "this spec does something stupid" were valid
>> arguments, we may as well not have specs at all, and FFmpeg would have a lot more
>> garbage in it. You can write at length why you think it's bad, but that does not
>> negeate the fact that it's the spec,
> 
> I'm not sure why the spec should have any bearing on the defaults of configurable options, particularly when those defaults result in unexpected behavior that's unlike that used for any other codec.

Because it's the spec, and it defines that. It is the source of truth,
and, in my opinion, the expected default behavior as a result.

>> and the defacto way to handle decoding AC-3.
> 
> Is it, though? My understanding is that AV receivers usually only enable AC3 DRC in "night mode" or the like, and few even provide a fully-tunable setting for it. Apple's decoder in AudioToolbox (implemented on top of Dolby's) doesn't provide the feature at all (I haven't checked what Win32 Media Foundation does yet). So where does this "de facto" situation come from, other than lavc itself?

Because the spec defines what is de facto - it is what defines what AC-3 *is*.
You are choosing to ignore it here because you don't like what's in it.

I am of the opinion that a spec is better than a survey of a one software decoder
(not endorsed or acknowledged by Dolby afaik), and some unconfirmed info on some
recievers.

Is the spec dumb? Yeah probably, but lots of specs have dumb stuff in them. But
we follow them - which is the entire point of a spec.

> Meanwhile, Dolby's own decoder applies digital dialog normalization by default (contrary to how the spec says it should be applied downstream), and I haven't seen anyone arguing that lavc should do that.

Own decoder from where? Is it officially Dolby branded, or is this Apple again?

> Additionally, TrueHD has a very similar DRC feature that lavc doesn't support whatsoever, and nobody seems to mind.

Missing support in another decoder is not a good argument in favour or changing
behavior in one that is not missing support.

(As an aside, I have heard this used as an argument against using FFmpeg's
TrueHD decoder in the past, in Profession Broadcast Vendor Trade Shows(TM))

> One other thing I neglected to mention in the commit message: the actual audio encoded in an (E)AC3 track often has levels similar to those of a track in another codec (PCM, AAC, etc) from the same or other sources, but sounds very different when both are decoded with lavc. For instance, Netflix's EAC3 sounds about the same as BD audio of the same content when decoded without DRC, but in lavc-based players by default, it sounds, well... compressed. This had led to an impression in the anime community that Netflix applies excessive DRC to their EAC3 streams at encode-time, and thus that those streams were low-quality and should be avoided.

Netflix is the origin of that audio, not the master. I suspect it's a byproduct of how they
transcode, rather than inherent to the show/BD/master/whatever itself. I don't think it's
a good argument in favour of changing the default behavior to be contrary to the spec,
but YMMV, I'm only one person. If people on the list decide they prefer it that way, then
it'll be that way - this is just my opinion as an API user and someone who has dealt with
too much vendor FUD.

- Derek
rcombs Feb. 1, 2020, 10:02 p.m. UTC | #10
> On Feb 1, 2020, at 15:55, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> 
>>> and the defacto way to handle decoding AC-3.
>> 
>> Is it, though? My understanding is that AV receivers usually only enable AC3 DRC in "night mode" or the like, and few even provide a fully-tunable setting for it. Apple's decoder in AudioToolbox (implemented on top of Dolby's) doesn't provide the feature at all (I haven't checked what Win32 Media Foundation does yet). So where does this "de facto" situation come from, other than lavc itself?
> 
> Because the spec defines what is de facto - it is what defines what AC-3 *is*.
> You are choosing to ignore it here because you don't like what's in it.

This is "de jure"; "de facto" means how it's actually implemented in practice.

>> Meanwhile, Dolby's own decoder applies digital dialog normalization by default (contrary to how the spec says it should be applied downstream), and I haven't seen anyone arguing that lavc should do that.
> 
> Own decoder from where? Is it officially Dolby branded, or is this Apple again?

Their own decoder IP and reference tool; it's non-public, and it's what Apple's is built on top of.

> 
> _______________________________________________
> 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".
Derek Buitenhuis Feb. 1, 2020, 10:09 p.m. UTC | #11
On 01/02/2020 22:02, rcombs wrote:
>> Because the spec defines what is de facto - it is what defines what AC-3 *is*.
>> You are choosing to ignore it here because you don't like what's in it.
> 
> This is "de jure"; "de facto" means how it's actually implemented in practice.

Woops, yes, you are correct. I misused the phrase.

I stand by my reasoning, however.

>> Own decoder from where? Is it officially Dolby branded, or is this Apple again?
> 
> Their own decoder IP and reference tool; it's non-public, and it's what Apple's is built on top of.

Most proprietary AC-3 decoders are, to my knowledge - it comes with the IP licensing
package or something. But the vendor (Apple) in this case, may change things. This is
why I specifically asked for Dolby branded or endorsed decoders - to my knowledge,
Apple's AudioToolbox is neither of those.

- Derek
diff mbox series

Patch

diff --git a/libavcodec/ac3dec_fixed.c b/libavcodec/ac3dec_fixed.c
index bd66175d50..312616edfd 100644
--- a/libavcodec/ac3dec_fixed.c
+++ b/libavcodec/ac3dec_fixed.c
@@ -169,7 +169,7 @@  static void ac3_downmix_c_fixed16(int16_t **samples, int16_t **matrix,
 
 static const AVOption options[] = {
     { "cons_noisegen", "enable consistent noise generation", OFFSET(consistent_noise_generation), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR },
-    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 1.0}, 0.0, 6.0, PAR },
+    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 0.0}, 0.0, 6.0, PAR },
     { "heavy_compr", "enable heavy dynamic range compression", OFFSET(heavy_compression), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR },
     { NULL},
 };
diff --git a/libavcodec/ac3dec_float.c b/libavcodec/ac3dec_float.c
index b85a4ce336..69bd47aa6a 100644
--- a/libavcodec/ac3dec_float.c
+++ b/libavcodec/ac3dec_float.c
@@ -33,7 +33,7 @@ 
 
 static const AVOption options[] = {
     { "cons_noisegen", "enable consistent noise generation", OFFSET(consistent_noise_generation), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR },
-    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 1.0}, 0.0, 6.0, PAR },
+    { "drc_scale", "percentage of dynamic range compression to apply", OFFSET(drc_scale), AV_OPT_TYPE_FLOAT, {.dbl = 0.0}, 0.0, 6.0, PAR },
     { "heavy_compr", "enable heavy dynamic range compression", OFFSET(heavy_compression), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, PAR },
     { "target_level", "target level in -dBFS (0 not applied)", OFFSET(target_level), AV_OPT_TYPE_INT, {.i64 = 0 }, -31, 0, PAR },
 
diff --git a/tests/fate/ac3.mak b/tests/fate/ac3.mak
index bb02d3829d..c810b53ad2 100644
--- a/tests/fate/ac3.mak
+++ b/tests/fate/ac3.mak
@@ -1,67 +1,67 @@ 
 FATE_AC3 += fate-ac3-2.0
-fate-ac3-2.0: CMD = pcm -i $(TARGET_SAMPLES)/ac3/monsters_inc_2.0_192_small.ac3
+fate-ac3-2.0: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/ac3/monsters_inc_2.0_192_small.ac3
 fate-ac3-2.0: REF = $(SAMPLES)/ac3/monsters_inc_2.0_192_small_v2.pcm
 
 FATE_AC3 += fate-ac3-4.0
-fate-ac3-4.0: CMD = pcm -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
+fate-ac3-4.0: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
 fate-ac3-4.0: REF = $(SAMPLES)/ac3/millers_crossing_4.0_v2.pcm
 
 #request_channel_layout 4 -> front channel
 FATE_AC3 += fate-ac3-4.0-downmix-mono
-fate-ac3-4.0-downmix-mono: CMD = pcm -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
+fate-ac3-4.0-downmix-mono: CMD = pcm -drc_scale 1 -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
 fate-ac3-4.0-downmix-mono: REF = $(SAMPLES)/ac3/millers_crossing_4.0_mono_v2.pcm
 
 #request_channel_layout 3 -> left channel + right channel
 FATE_AC3 += fate-ac3-4.0-downmix-stereo
-fate-ac3-4.0-downmix-stereo: CMD = pcm -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
+fate-ac3-4.0-downmix-stereo: CMD = pcm -drc_scale 1 -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
 fate-ac3-4.0-downmix-stereo: REF = $(SAMPLES)/ac3/millers_crossing_4.0_stereo_v2.pcm
 
 FATE_AC3 += fate-ac3-5.1
-fate-ac3-5.1: CMD = pcm -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
+fate-ac3-5.1: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
 fate-ac3-5.1: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_v2.pcm
 
 FATE_AC3 += fate-ac3-5.1-downmix-mono
-fate-ac3-5.1-downmix-mono: CMD = pcm -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
+fate-ac3-5.1-downmix-mono: CMD = pcm -drc_scale 1 -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
 fate-ac3-5.1-downmix-mono: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_mono_v2.pcm
 
 FATE_AC3 += fate-ac3-5.1-downmix-stereo
-fate-ac3-5.1-downmix-stereo: CMD = pcm -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
+fate-ac3-5.1-downmix-stereo: CMD = pcm -drc_scale 1 -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
 fate-ac3-5.1-downmix-stereo: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_stereo_v2.pcm
 
 FATE_AC3 += fate-ac3-fixed-2.0
-fate-ac3-fixed-2.0: CMD = pcm -c ac3_fixed -i $(TARGET_SAMPLES)/ac3/monsters_inc_2.0_192_small.ac3
+fate-ac3-fixed-2.0: CMD = pcm -drc_scale 1 -c ac3_fixed -i $(TARGET_SAMPLES)/ac3/monsters_inc_2.0_192_small.ac3
 fate-ac3-fixed-2.0: REF = $(SAMPLES)/ac3/monsters_inc_2.0_192_small_v2.pcm
 
 FATE_AC3 += fate-ac3-fixed-4.0-downmix-mono
-fate-ac3-fixed-4.0-downmix-mono: CMD = pcm -c ac3_fixed -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
+fate-ac3-fixed-4.0-downmix-mono: CMD = pcm -drc_scale 1 -c ac3_fixed -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/millers_crossing_4.0.ac3
 fate-ac3-fixed-4.0-downmix-mono: REF = $(SAMPLES)/ac3/millers_crossing_4.0_mono_v2.pcm
 
 FATE_AC3 += fate-ac3-fixed-5.1-downmix-mono
-fate-ac3-fixed-5.1-downmix-mono: CMD = pcm -c ac3_fixed -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
+fate-ac3-fixed-5.1-downmix-mono: CMD = pcm -drc_scale 1 -c ac3_fixed -request_channel_layout 4 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
 fate-ac3-fixed-5.1-downmix-mono: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_mono_v2.pcm
 
 FATE_AC3 += fate-ac3-fixed-5.1-downmix-stereo
-fate-ac3-fixed-5.1-downmix-stereo: CMD = pcm -c ac3_fixed -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
+fate-ac3-fixed-5.1-downmix-stereo: CMD = pcm -drc_scale 1 -c ac3_fixed -request_channel_layout 3 -i $(TARGET_SAMPLES)/ac3/monsters_inc_5.1_448_small.ac3
 fate-ac3-fixed-5.1-downmix-stereo: REF = $(SAMPLES)/ac3/monsters_inc_5.1_448_small_stereo_v2.pcm
 
 FATE_EAC3 += fate-eac3-1
-fate-eac3-1: CMD = pcm -i $(TARGET_SAMPLES)/eac3/csi_miami_5.1_256_spx_small.eac3
+fate-eac3-1: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/csi_miami_5.1_256_spx_small.eac3
 fate-eac3-1: REF = $(SAMPLES)/eac3/csi_miami_5.1_256_spx_small_v2.pcm
 
 FATE_EAC3 += fate-eac3-2
-fate-eac3-2: CMD = pcm -i $(TARGET_SAMPLES)/eac3/csi_miami_stereo_128_spx_small.eac3
+fate-eac3-2: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/csi_miami_stereo_128_spx_small.eac3
 fate-eac3-2: REF = $(SAMPLES)/eac3/csi_miami_stereo_128_spx_small_v2.pcm
 
 FATE_EAC3 += fate-eac3-3
-fate-eac3-3: CMD = pcm -i $(TARGET_SAMPLES)/eac3/matrix2_commentary1_stereo_192_small.eac3
+fate-eac3-3: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/matrix2_commentary1_stereo_192_small.eac3
 fate-eac3-3: REF = $(SAMPLES)/eac3/matrix2_commentary1_stereo_192_small_v2.pcm
 
 FATE_EAC3 += fate-eac3-4
-fate-eac3-4: CMD = pcm -i $(TARGET_SAMPLES)/eac3/serenity_english_5.1_1536_small.eac3
+fate-eac3-4: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/serenity_english_5.1_1536_small.eac3
 fate-eac3-4: REF = $(SAMPLES)/eac3/serenity_english_5.1_1536_small_v2.pcm
 
 FATE_EAC3 += fate-eac3-5
-fate-eac3-5: CMD = pcm -i $(TARGET_SAMPLES)/eac3/the_great_wall_7.1.eac3
+fate-eac3-5: CMD = pcm -drc_scale 1 -i $(TARGET_SAMPLES)/eac3/the_great_wall_7.1.eac3
 fate-eac3-5: REF = $(SAMPLES)/eac3/the_great_wall_7.1.pcm
 
 $(FATE_AC3) $(FATE_EAC3): CMP = oneoff