[FFmpeg-devel] [PATCH v6] avcodec/mpeg12enc: support mpeg2 encoder const profile

[Marton Balint]

On Sat, 20 Jun 2020, lance.lmwang at gmail.com wrote:

> On Fri, Jun 19, 2020 at 06:50:03PM +0200, Carl Eugen Hoyos wrote:
>> Am Sa., 30. Mai 2020 um 15:29 Uhr schrieb <lance.lmwang at gmail.com>:
>> >
>> > From: Limin Wang <lance.lmwang at gmail.com>
>> >
>> > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
>> > ---
>> >  doc/encoders.texi      | 14 ++++++++++++++
>> >  libavcodec/mpeg12enc.c |  2 ++
>> >  libavcodec/profiles.h  |  8 ++++++++
>> >  3 files changed, 24 insertions(+)
>> >
>> > diff --git a/doc/encoders.texi b/doc/encoders.texi
>> > index 954f0f6..1331b79 100644
>> > --- a/doc/encoders.texi
>> > +++ b/doc/encoders.texi
>> > @@ -2740,6 +2740,20 @@ MPEG-2 video encoder.
>> >  @subsection Options
>> >
>> >  @table @option
>> > + at item profile @var{integer}
>> > +Select the mpeg2 profile to encode:
>> > +
>> > + at table @samp
>> > + at item 422
>> > + at item main
>> > + at item ss
>> > +Spatially Scalable
>> > + at item snr
>> > +SNR Scalable
>> > + at item high
>> > + at item simple
>> > + at end table
>> 
>> Above looks wrong (as reported on the user mailing list):
>> high and main have to be swapped, no?
>
> User can't assume the integer value by the order, it's const 
> string.

I agree. As I explaned in the ffmpeg-user thread, the only confusing thing 
might be that @var{integer} is listed, maybe the @var{integer} parameter 
should be removed to avoid the confusion.

> I recall ffmpeg -h full can give the detail integer,
> now it's missing for the profile options, what's the problem?
>
> But I'll fix the order to avoid such confusing.

Fix it if you want, I don't really mind either way, but I think its not 
needed, because the user can't assume any mapping in general for 
constant options and their integer mappings.

Regards,
Marton