[FFmpeg-devel] [PATCH] doc/decoders: Clear up description of ac3's drc_scale option

[Aman Verma]

On Fri, Aug 28, 2020 at 11:15 PM Jim DeLaHunt <list+ffmpeg-dev at jdlh.com> wrote:
> I don't know anything about the AC-3 format or the ac3 decoder in
> FFmpeg. However, I can read technical writing. I don't understand what
> you are trying to say differently with this change. But even more, I
> don't understand what the existing documentation is trying to say.
> I like that you are adding the information that the default value of
> -drc_scale is 1. That is not presently documented. Good.
>
> It looks like the other changes are to change "factor" to "number", and
> to convert a sentence "This factor is applied exponentially." to a
> dashed phrase "---exponentially---". I don't feel strongly that this
> makes things worth, but I also don't see how this makes things clearer.
>
> However, both the old and the new text leave me with a lot of questions.
> No doubt some of my questions are due to my ignorance of the AC-3 format
> and of the ac3 decoder code in FFmpeg. But some are due to the text
> leaving a lot of information unstated.

That is a fair judgement, I mostly reworded it because I didn't like the
sentence structure and I didn't look too closely at the source beyond
finding the default value. Looking at it now, it seems that the comments
in ac3dec_fixed.c, ac3dec_float.c, and ac3dec.h claim that the value is
applied as a linear scaler, contradicting the claims in the CLI
documentation [1][2][3].

> What does it mean to "apply — exponentially — to" a number? What
> mathematical operation is that? This is not at all clear to me.

That definitely could have been made clearer haha. I was referring to a
function f(x,d) = x^d, where x is the number and d is the drc_scale
value. I think that is also what the original documentation meant.

> So, I'm not in a position to approve or reject this patch.  And fixing
> deficiencies in the ac3 decoder documentation overall is probably not
> the scope of change which you wanted to make.

For now, I'll revise the patch to only include the mention of the
default value but I will do a closer reading of the ac3dec source when I
have more time.

As an aside, am I supposed to submit the revised patch in reply to this
thread or should I submit it as a new thread? This is my first time
contributing to a project that uses mailing lists.

[1]: https://github.com/FFmpeg/FFmpeg/blob/5ff2ff6bd9cd9e08729060d330e381a09972c498/libavcodec/ac3dec_fixed.c#L159
[2]: https://github.com/FFmpeg/FFmpeg/blob/5ff2ff6bd9cd9e08729060d330e381a09972c498/libavcodec/ac3dec_float.c#L36
[3]: https://github.com/FFmpeg/FFmpeg/blob/5ff2ff6bd9cd9e08729060d330e381a09972c498/libavcodec/ac3dec.h#L175