[FFmpeg-devel] [RFC] encoder profile validation

[mypopy at gmail.com]

On Tue, May 19, 2020 at 2:33 AM Marton Balint <cus at passwd.hu> wrote:
>
>
>
> On Mon, 18 May 2020, Anton Khirnov wrote:
>
> > Quoting Marton Balint (2020-05-16 15:52:22)
> >> 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
> >
> > All of these sound like bugs that can and should be fixed.
> >
> >> - 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.
> >
> > Why should it be different? Isn't that a bug that should be fixed?
>
> The list can be different because possibly not all the profiles which are
> recognized by the decoder is supported by the encoder. Also the encoder
> can define pseudo profiles which it can support, for example the AAC
> encoders have support for FF_PROFILE_MPEG2_AAC_LOW which is LC profile
> with some features disabled.
>
> >
> >>
> >> So I'd rather not user AVCodec->profiles for this validation. I am
> >> thinking about two possible solutions:
> >
> > It seems preferable to me to fix the above issues and not have multiple
> > fields with subtly different meanings that are just bound to cause
> > confusion and bugs.
>
> I agree that having the list of suppored profiles in two places is wrong,
> so in the long run, I would simply deprecate AVCodec->profiles, and use
> only the AVOptions to list the supported profiles. Also see my previous
> patchset which moves profile names directly to encoders:
>
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1193
>
> So the supported profiles would be contained in one place, AVOptions.
> Even querying the list should be doable, if needed, and
> av_get_profile_name can also be modified to get the profile name from the
> AVOptions.
So this change will break the API if deprecate AVCodec->profiles? I
know some guys used the FFmpeg API for encoding, it's bad new for them