[FFmpeg-devel] [RFC] encoder profile validation

[Anton Khirnov]

Quoting Marton Balint (2020-05-20 20:26:53)
> 
> 
> You asked that why the AVProfiles list of the encoders should be different 
> to the AVProfile lists stored in libavcodec/profiles.c. Or at least that's 
> how I understood your question...
> 
> Anyway, I'd still argue against the usage of AVCodec->profiles 
> and AVProfiles in general. Because in an encoder we'd have to list the 
> supported profiles as AVOptions in order to be able to specify the profile 
> names:
> 
> (See my previous patchset [1] for the macro)
> 
> static const AVOption options[] = {
>      FF_AVCTX_PROFILE_OPTION("dnxhd",     NULL, VIDEO, FF_PROFILE_DNXHD)
>      FF_AVCTX_PROFILE_OPTION("dnxhr_444", NULL, VIDEO, FF_PROFILE_DNXHR_444)
>      FF_AVCTX_PROFILE_OPTION("dnxhr_hqx", NULL, VIDEO, FF_PROFILE_DNXHR_HQX)
>      FF_AVCTX_PROFILE_OPTION("dnxhr_hq",  NULL, VIDEO, FF_PROFILE_DNXHR_HQ)
>      FF_AVCTX_PROFILE_OPTION("dnxhr_sq",  NULL, VIDEO, FF_PROFILE_DNXHR_SQ)
>      FF_AVCTX_PROFILE_OPTION("dnxhr_lb",  NULL, VIDEO, FF_PROFILE_DNXHR_LB)
>      { NULL }
> }
> 
> And we'd also have to list the supported profiles as AVProfiles to specify 
> the valid profiles.
> 
> const AVProfile profiles[] = {
>    { FF_PROFILE_DNXHD,      "DNXHD"},
>    { FF_PROFILE_DNXHR_LB,   "DNXHR LB"},
>    { FF_PROFILE_DNXHR_SQ,   "DNXHR SQ"},
>    { FF_PROFILE_DNXHR_HQ,   "DNXHR HQ" },
>    { FF_PROFILE_DNXHR_HQX,  "DNXHR HQX"},
>    { FF_PROFILE_DNXHR_444,  "DNXHR 444"},
>    { FF_PROFILE_UNKNOWN },
> };
> 
> So we'd have the list of supported profiles in an encoder two times which 
> seems ugly and can become inconsistent. Also it seems cleaner to me to to do the 
> profile validation via AVOptions, rather than hardcoding checking the 
> profiles list in avcodec_open2.

I suppose that makes sense, but then AVCodec.profiles should be
deprecated, otherwise users will be confused about what to use.

-- 
Anton Khirnov