On Wed, 20 May 2020, myp...@gmail.com wrote:
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
Which change? According to my plan, validating AVCodecContext->profile
will not not touch AVCodec->profiles. A potential next change can
deprecate it, but it is a deprecation, not an instant removal.
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".
