[FFmpeg-devel] [PATCH 01/17] avcodec/ac3enc: Don't presume ch_layout to be AV_CHANNEL_ORDER_NATIVE
James Almer
jamrial at gmail.com
Mon Apr 8 01:21:47 EEST 2024
On 4/7/2024 6:53 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 4/7/2024 5:39 PM, Andreas Rheinhardt wrote:
>>> It is perfectly legal for users to use a custom layout
>>> that is equivalent to a supported native one.
>>
>> Is that really the case? FFCodec.p.ch_layouts[] has a list of native
>> ones, and the generic encode.c code will reject anything not in it.
>>
>
> This is not true. It allows everything that is equivalent (in the
> av_channel_layout_compare() sense) to one of the channel layouts in
> AVCodec.ch_layouts. This means that if ch_layout.u.map[i].id is strictly
> ascending for 0 <= i <nb_channels and < 64, then it is equivalent to the
> native channel layout with the same nb_channels whose mask is the
> bitwise or of all (1ULL << ch_layout.u.map[i].id).
Right, misread av_channel_layout_compare() as rejecting layouts that
were not the same order.
>
>> I guess the encode.c check could be improved with
>> av_channel_layout_retype().
>>
>
> ch_layout is not supposed to be set by lavc for encoders.
I didn't mean to set the layout for encoders, i meant it as a way for
encode.c to compare input layouts with encoder-supported native layouts,
because of my misunderstanding of how av_channel_layout_compare() behaved.
>
>>> In this case the union in AVChannelLayout is not an uint64_t mask,
>>> but a pointer to a custom map.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>> ---
>>> libavcodec/ac3enc.c | 6 +-----
>>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
>>> index 7a6bcf7900..dc197c1517 100644
>>> --- a/libavcodec/ac3enc.c
>>> +++ b/libavcodec/ac3enc.c
>>> @@ -2192,18 +2192,14 @@ av_cold int ff_ac3_encode_close(AVCodecContext
>>> *avctx)
>>> static av_cold int set_channel_info(AVCodecContext *avctx)
>>> {
>>> AC3EncodeContext *s = avctx->priv_data;
>>> + uint64_t mask = av_channel_layout_subset(&avctx->ch_layout,
>>> ~(uint64_t)0);
>>
>> Might as well use UINT64_MAX.
>>
>
> This is a bitfield, so I intentionally used a bit-operation.
Then ~UINT64_C(0).
>
>>> int channels = avctx->ch_layout.nb_channels;
>>> - uint64_t mask = avctx->ch_layout.u.mask;
>>> if (channels < 1 || channels > AC3_MAX_CHANNELS)
>>> return AVERROR(EINVAL);
>>> if (mask > 0x7FF)
>>> return AVERROR(EINVAL);
>>> - if (!mask)
>>> - av_channel_layout_default(&avctx->ch_layout, channels);
>>> - mask = avctx->ch_layout.u.mask;
>>> -
>>> s->lfe_on = !!(mask & AV_CH_LOW_FREQUENCY);
>>> s->channels = channels;
>>> s->fbw_channels = channels - s->lfe_on;
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list