On Tue, May 19, 2020 at 2:33 AM Marton Balint <c...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
