[FFmpeg-devel] [PATCH] Document the preset files system

[Stefano Sabatini]

On date Wednesday 2008-10-01 19:20:28 -0400, The Wanderer encoded:
> Stefano Sabatini wrote:
> 
> > On date Sunday 2008-09-28 20:27:31 -0400, The Wanderer encoded:
> > 
> >> Stefano Sabatini wrote:
> >> 
> >>> Hi, as in subject.
> > 
> > [...]
> > 
> >> Beyond that I can't really improve this, largely because I don't
> >> understand it well enough for the degree of rewriting it would
> >> require. (I think it's talking about automatically choosing a
> >> preset based on the specified codec name and on something else, but
> >> I have no real idea what the else is from a user perspective, and I
> >> don't see where in the code this paragraph comes from in the first
> >> place.)
> >> 
> >> Honestly, I would prefer to rewrite (and reorganize) these last
> >> three paragraphs entirely, if I were certain that I understood them
> >> correctly.
> > 
> > I rephrased almost entirely the whole text, trying to apply the
> > proposed corrections, I think it should be more clear now, please
> > check it again.
> 
> Sorry for the delay. Here goes.
> 
> > +The argument of these options identifies a preset file according to
> > +the following rules.
> 
> The use of the singular "argument" and the plural "options" seems a
> little off, though I understand what you mean and can see how it could
> be said to make sense. Unfortunately just using both singular or both
> plural becomes either more-confusing or outright inaccurate. Something
> like "The argument passed to one of these options is used to identify",
> et cetera... still has problems, but avoids that *particular* problem.
> I'm not sure it's really better, either.
> 
> If anyone else has any suggestions for how to improve this, I'd
> appreciate hearing them.

Applied your suggestion:
"The argument passed to one of these options is used to identify the
preset file to use according to the following rules."
 
> > +First ffmpeg searches for a file named @var{ARG}.ffpreset in the
> > +directories @file{$HOME/.ffmpeg}, @file{/usr/local/share/ffmpeg} and
> > + at file{/usr/share/ffmpeg} in that order. For example if the argument is
> > + at code{libx264-max} it will search for the file
> > + at file{libx264-max.ffpreset}.
> 
> It feels as if there should be a comma after "First", but adding that
> would conflict with the use of a comma-separated list later in the
> sentence, so it's probably best to leave it alone.
> 
> Need two commas in the second sentence - after "example" and after the
> first @code section.

Done.

> > +If a file is not found applying the previous rule, then ffmpeg will
> 
> This seems awkward, but I don't have a good way to fix it while keeping
> the same approach. I think something like "If no such file is found,"
> might work, though.

Yes.

> > +search for a file named @var{CODEC_NAME}- at var{ARG}.ffpreset in the
> > +above-mentioned directories, where @var{CODEC_NAME} is the name of the
> > +currently selected codec with the type specified by the command
> > +option. For example if the command is @code{-vpre max} and the
> > +currently selected video codec is @code{libx264}, then it will search
> > +for the file @file{libx264-max.ffpreset}.
> 
> Again, need comma after "example".
> 
> Clarification question: does "with the type specified by the command
> option" (which should be "command-line option", naturally) refer to the
> type of codec (i.e., which type of codec is used depends on the
> command-line option, e.g. '-vcodec foobar'), or to the type of preset
> (e.g., "max" vs. "normal")?

The former, I didn't specify "*codec* type" because "codec" is used
few words before, but now I see that maybe is better to specify it.

> If the former, I would say something like "the currently selected codec
> as specified on the command line",

This would be slightly misleading, since the codec may be
automatically selected by ffmpeg itself, so the user doesn't
necessarily needs to specify it on command line.

> and possibly mention '-vcodec' explicitly at some point ("specified
> by the -vcodec option'?).

OK, I'm mentioning it in the example.

I also reworded other parts of the paragraph, please check it again.

> If the latter, I don't think I would include the part of the sentence
> after "codec" at all; the matter seems clearly enough explained by the
> example, and I mentioning it here doesn't seem to have any benefit and
> might be confusing.
> 
> > +Finally if the above rules failed and the argument specifies an
> > +absolute pathname, it will search for that filename. This way you can
> > +specify the absolute and complete filename of the preset file, for
> > +example @file{./ffpresets/libx264-max.ffpreset}.
> 
> Need a comma after "finally".
> 
> Other than that, looks good enough to me.

While I was at it I also lowcased the argument variables (ARG -> arg),
which looks more consistent with the file and less ugly.

New patch attached.

Thanks, regards.
-- 
FFmpeg = Fiendish and Faboulous Minimal Power Elastic Goblin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: document-preset-02.patch
Type: text/x-diff
Size: 1913 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081002/2990c988/attachment.patch>