[FFmpeg-devel] [FFmpeg-cvslog] lavc/qsvenc: add VDENC support for H264

[Carl Eugen Hoyos]

2019-01-14 9:03 GMT+01:00, Fu, Linjie <linjie.fu at intel.com>:
>> -----Original Message-----
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>> Of Li, Zhong
>> Sent: Friday, January 11, 2019 11:43
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] lavc/qsvenc: add VDENC
>> support for H264
>>
>> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
>> Behalf
>> > Of Carl Eugen Hoyos
>> > Sent: Friday, January 11, 2019 10:39 AM
>> > To: FFmpeg development discussions and patches
>> > <ffmpeg-devel at ffmpeg.org>
>> > Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] lavc/qsvenc: add VDENC
>> > support for H264
>> >
>> > 2019-01-11 3:28 GMT+01:00, Li, Zhong <zhong.li at intel.com>:
>> > >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
>> > Behalf
>> > >> Of Carl Eugen Hoyos
>> > >> Sent: Friday, January 11, 2019 1:06 AM
>> > >> To: ffmpeg-devel at ffmpeg.org
>> > >> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] lavc/qsvenc: add VDENC
>> > >> support for H264
>> > >>
>> > >> 2019-01-10 14:51 GMT+01:00, Linjie Fu <git at videolan.org>:
>> > >>
>> > >> > +#if QSV_HAVE_VDENC
>> > >> > +    { "low_power", "enable low power mode(experimental: many
>> > >> > +limitations by
>> > >> > mfx version, BRC modes, etc.)", OFFSET(qsv.low_power),
>> > >> > AV_OPT_TYPE_BOOL, {
>> > >> > .i64 =  0 }, 0, 1, VE},
>> > >> > +#endif
>> > >>
>> > >> This seems wrong to me: The visibility of an option should probably
>> > >> not depend on configuration details (the effect can of course depend
>> > >> on many details).
>> > >>
>> > >> Carl Eugen
>> > >
>> > > That's is just a remind that this feature has some limitations. The
>> > > configures are just to make driver to support this feature.
>> > > In ffmpeg level, the only thing we should do IMHO is to query
>> > > MSDK/Driver capability and report an error message if not supported.
>> > > And it was done:
>> > >
>> > https://github.com/FFmpeg/FFmpeg/blob/395e8a53fa0266f26581f3e9752b
>> > 0dbc
>> > > 93998a90/libavcodec/qsvenc.c#L376
>> >
>> > That is probably helpful, but the issue I try to explain is unrelated to
>> > the
>> > brokenness of the option:
>> > Now, in some cases FFmpeg shows the option and in other cases it does
>> not.
>> > I believe that instead, the option should always be shown, even if it
>> > does
>> not
>> > work.
>>
>> That is a big topic since I see many components are doing this. E.g; we
>> can
>> find some options of libx264 and nvenc are hided too.
>> Personally speaking I like current way: if the implementation of an option
>> is
>> not built-in, it is not necessary to show the option, else it is really
>> confusing
>> that this option can't work at any case.
>> But I am open to your suggestion, and looking forward more comments from
>> you and others. (If we get alignment, we need to clear up many options,
>> not
>> only this one).
>
> Is it a possible way to show "not supported" info and set the option to
> default off instead of  just hiding the option?
>
> Like:
> #if QSV_HAVE_VDENC
>     { "low_power", "enable low power mode (experimental, many limitations by
> mfx version, HW platform, BRC modes, etc.)", OFFSET(qsv.low_power),
> AV_OPT_TYPE_BOOL, { .i64 =  0 }, 0, 1, VE},
> # else
>     { "low_power", "not supported", OFFSET(qsv.low_power), AV_OPT_TYPE_BOOL,
> { .i64 =  0 }, 0, 0, VE},
> #endif

This is also an option but I wonder if a failure message wouldn't help
users even more.

Carl Eugen