[FFmpeg-devel] [RFC] encoder profile validation

[Marton Balint]

On Sat, 16 May 2020, Fu, Linjie wrote:

>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Marton Balint
>> Sent: Saturday, May 16, 2020 21:52
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: [FFmpeg-devel] [RFC] encoder profile validation
>>
>> Hi,
>>
>> As you may know, a recent patchset enabled AVCodecContext->profile
>> constants to reside in encoders.
>>
>> In order to make a full transition to avctx->profile even in existing
>> encoders which might use a private profile setting, we have to make sure
>> only supported avctx->profile values are passed to encoders.
>>
>> The fact that avctx->profile is not validated is already an issue, and
>> assertions/segmentation faults can already happen in existing encoders
>> (e.g.: aac, mpeg) if unsupported values are passed.
>>
>> AVCodec have a .profiles attribute which supposed to contain the list of
>> supported profiles. However this is problematic because
>> - AVCodecContext->profile is not validated against this list
>> - not all encoders define the list
>> - even if there is a list it might be defined as NULL_IF_CONFIG_SMALL
>> - some encoders support more or less than its currently defined list
>> - AVCodec->profiles contains AVProfiles which supposed to have a textual
>>    representation of each profile, which can cause profile name
>>    duplications/inconsistencies against libavcodec/profiles.c if the list
>>    is different to the one in codecs profile list.
>>
>> So I'd rather not user AVCodec->profiles for this validation. I am
>
> The validation would help from my perspective. Meanwhile got some questions:
>
> IIRC, AVCodec->profiles probably didn't cover the HW specific profiles for encoders
> Like nvenc/qsv/amfenc. Are you intending to add them in this profile list  and keep
> Them in common or did the map/verify internally in specific hw encoder?

No, I'd leave AVCodec->profiles alone, because for the reasons I 
described above I plan to NOT use it for validation.

>
>> thinking about two possible solutions:
>>
>> 1) Add a new AVCodec->supported_profiles attribute which is a simple
>> integer list and validate against this list
>>
>> or
>>
>> 2) Introdce a new OPT type, AV_OPT_TYPE_ENUM which validates against
>> the
>> values of its named constants. This has the benefit that the supported
>> profiles only appear once in the AVClass option list, but it also
>> means that encoders might not share the supported profiles list via a
>> #define in libavcodec/profile.h, because they might support different
>> profiles.
>>
>> I like the second approach better, but let me know what you think.
>>
> Regardless of the validation itself, as to " make a full transition to avctx->profile",
> do we still need to add a prefix like "-profile:v h264_main " in your designs?
> (I'd prefer a more natural usage)

No, my previous patchset makes it possible for encoders to define their 
own profile names and those names do not have to be unique.

https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1193

Regards,
Marton