On Fri, Aug 28, 2020 at 11:15 PM Jim DeLaHunt <list+ffmpeg-...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
