[FFmpeg-devel] [PATCH] doc/encoders: Add libopus encoder doc

[Timothy Gu]

On Jul 1, 2013 7:18 AM, "Stefano Sabatini" <stefasab at gmail.com> wrote:
>
> On date Friday 2013-06-28 16:46:21 -0700, Timothy Gu encoded:
> > ---
> >  doc/encoders.texi | 74
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 74 insertions(+)
> >
> > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > index 7da6376..c3db780 100644
> > --- a/doc/encoders.texi
> > +++ b/doc/encoders.texi
> > @@ -629,6 +629,80 @@ Same as 3, but with extra processing enabled -
corresponding to the wavpack
> >
> >  @end table
> >
> > + at section libopus
> > +
> > +libopus Opus Interactive Audio Codec encoder wrapper.
> > +
> > +Requires the presence of the libopus headers and library during
> > +configuration. You need to explicitly configure the build with
> > + at code{--enable-libopus}.
> > +
> > + at subsection Option Mapping
> > +
> > +Most libopus options are modeled after the @command{opusenc} utility
from
> > +opus-tools. The following is an option mapping chart describing options
> > +supported by the libopus wrapper, and their
@command{opusenc}-equivalent.
> > +
> > + at multitable @columnfractions .15 .15 .7
> > + at headitem FFmpeg                 @tab @command{opusenc}
>
> I would avoid the @columnfractions MAGIC altogether, which has a poor
> rendering in the HTML output (and thus I'd avoid it in
> libmp3lame/lib264 docs as well).

Okay, but you told me that you like two columns with separate options like
this better, and now I think you were right.

http://ffmpeg.org/pipermail/ffmpeg-devel/2013-May/144286.html

Try to actually build the HTML and you will see that this renders better
than you think. Either way I will send a separate patch to normalize all
the behaviors.

>
> What I suggest instead:
> @item b (bitrate)
>
> or if you prefer:
> @item b (@emph{bitrate})
>
> adopting the KISS principle.

The thing I did isn't simple?

>
>
> > + at tab Comments
> > +
> > + at item @option{b}                 @tab @option{bitrate}
>
> @item b ...
>
> should be more consistent with the rest of the docs (check also HTML
> output)

Again, now I don't want to hear "use the format other docs are using", I
want to __change__ the format.

>
> > + at tab Set the bit rate in bits/s. One thing to note is that FFmpeg's
> > + at option{b} option is expressed in bits/s, while @command{opusenc}'s
> > + at option{bitrate} in kilobits/s.
>
> Simpler:
>
> Set the bit rate in bits/s.  FFmpeg's @option{b} option is expressed
> in bits/s, while @command{opusenc}'s @option{bitrate} in
> kilobits/s.

Will fix

>
> > The default value in both encoders are
> > +the same, which is 64k per mono stream, 96k per coupled pair.
>
> I'd avoid this part, since this will result in wrong docs in case
> either is updated (and I bet this will happen).

Will fix

>
> > +
>
> > + at item @option{vbr}               @tab @option{vbr}, @option{hard-cbr},
and @option{cvbr}
> > + at tab Set VBR mode. The libavcodec @option{vbr} option has the following
> > +valid arguments, with the their @command{opusenc} equivalent in
>
> Please stick either with "FFmpeg" or "libavcodec".

I will use libavcodec for this, and will also normalize this in other parts
of the doc in a separate patch later.

>
> > +parentheses:
> > +
> > + at table @samp
> > + at item off
> > +(hard-cbr) Use constant bit rate encoding.
>
>
> I'd put them on the same line:
> @item off (hard-cbr)

But this will become ugly in the rendered result.

>
> same below
>
> > + at item on
> > +(vbr) Use variable bit rate encoding (the default).
> > + at item constrained
> > +(cvbr) Use constrained variable bit rate encoding.
> > + at end table
> > +
> > + at item @option{compression_level} @tab @option{comp}
>
> > + at tab Set encoding algorithm complexity. Valid options are integers
from 0-10.
>
> integers in the 0-10 range.
>

Will fix.

[...]

So I assume other stuff are OK, especially the listing of all the opus
bands?

I will submit the fixed patch after you decide will which format to go with.

Timothy