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.
Regards,
Marton
_______________________________________________
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".
