[FFmpeg-devel] [PATCH] avcodec/ac3dec: fix downmix logic for eac3

James Almer jamrial at gmail.com
Sun Nov 24 23:52:14 EET 2024


On 11/24/2024 2:07 PM, James Almer wrote:
> Ensure downmixed is only set once during init, as it used to be.
> 
> Fixes a regression since acbb2777e28c.
> Fixes ticket #11321.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>   libavcodec/ac3dec.c | 121 ++++++++++++++++++++++----------------------
>   1 file changed, 61 insertions(+), 60 deletions(-)
> 
> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
> index 0a4d3375ee..11374bcb7d 100644
> --- a/libavcodec/ac3dec.c
> +++ b/libavcodec/ac3dec.c
> @@ -199,7 +199,6 @@ static void ac3_downmix(AVCodecContext *avctx)
>           av_channel_layout_uninit(&avctx->ch_layout);
>           avctx->ch_layout = (AVChannelLayout)AV_CHANNEL_LAYOUT_STEREO;
>       }
> -    s->downmixed = 1;
>   }
>   
>   /**
> @@ -241,6 +240,7 @@ static av_cold int ac3_decode_init(AVCodecContext *avctx)
>           avctx->sample_fmt = AV_SAMPLE_FMT_FLTP;
>   
>       ac3_downmix(avctx);
> +    s->downmixed = 1;
>   
>       for (i = 0; i < AC3_MAX_CHANNELS; i++) {
>           s->xcfptr[i] = s->transform_coeffs[i];
> @@ -1647,6 +1647,66 @@ dependent_frame:
>           if (ch < s->out_channels)
>               s->outptr[channel_map[ch]] = s->output_buffer[ch + offset];
>       }
> +
> +    for (ch = 0; ch < EAC3_MAX_CHANNELS; ch++)
> +        extended_channel_map[ch] = ch;
> +
> +    if (s->frame_type == EAC3_FRAME_TYPE_DEPENDENT) {
> +        uint64_t ich_layout = ff_ac3_channel_layout_tab[s->prev_output_mode & ~AC3_OUTPUT_LFEON];
> +        int channel_map_size = ff_ac3_channels_tab[s->output_mode & ~AC3_OUTPUT_LFEON] + s->lfe_on;
> +        uint64_t channel_layout;
> +        int extend = 0;
> +
> +        if (s->prev_output_mode & AC3_OUTPUT_LFEON)
> +            ich_layout |= AV_CH_LOW_FREQUENCY;
> +
> +        channel_layout = ich_layout;
> +        for (ch = 0; ch < 16; ch++) {
> +            if (s->channel_map & (1 << (EAC3_MAX_CHANNELS - ch - 1))) {
> +                channel_layout |= ff_eac3_custom_channel_map_locations[ch][1];
> +            }
> +        }
> +        if (av_popcount64(channel_layout) > EAC3_MAX_CHANNELS) {
> +            av_log(avctx, AV_LOG_ERROR, "Too many channels (%d) coded\n",
> +                   av_popcount64(channel_layout));
> +            return AVERROR_INVALIDDATA;
> +        }
> +
> +        av_channel_layout_uninit(&avctx->ch_layout);
> +        av_channel_layout_from_mask(&avctx->ch_layout, channel_layout);
> +
> +        for (ch = 0; ch < EAC3_MAX_CHANNELS; ch++) {
> +            if (s->channel_map & (1 << (EAC3_MAX_CHANNELS - ch - 1))) {
> +                if (ff_eac3_custom_channel_map_locations[ch][0]) {
> +                    int index = av_channel_layout_index_from_channel(&avctx->ch_layout,
> +                                                                     ff_ctzll(ff_eac3_custom_channel_map_locations[ch][1]));
> +                    if (index < 0)
> +                        return AVERROR_INVALIDDATA;
> +                    if (extend >= channel_map_size)
> +                        break;
> +
> +                    extended_channel_map[index] = offset + channel_map[extend++];
> +                } else {
> +                    int i;
> +
> +                    for (i = 0; i < 64; i++) {
> +                        if ((1ULL << i) & ff_eac3_custom_channel_map_locations[ch][1]) {
> +                            int index = av_channel_layout_index_from_channel(&avctx->ch_layout, i);
> +                            if (index < 0)
> +                                return AVERROR_INVALIDDATA;
> +                            if (extend >= channel_map_size)
> +                                break;
> +
> +                            extended_channel_map[index] = offset + channel_map[extend++];
> +                        }
> +                    }
> +                }
> +            }
> +        }
> +
> +        ac3_downmix(avctx);
> +    }
> +

Actually, i don't even need to move this chunk. Just moving s->downmixed 
is enough, so amended locally.

>       for (blk = 0; blk < s->num_blocks; blk++) {
>           if (!err && decode_audio_block(s, blk, offset)) {
>               av_log(avctx, AV_LOG_ERROR, "error decoding the audio block\n");
> @@ -1713,65 +1773,6 @@ skip:
>           return AVERROR_INVALIDDATA;
>       }
>   
> -    for (ch = 0; ch < EAC3_MAX_CHANNELS; ch++)
> -        extended_channel_map[ch] = ch;
> -
> -    if (s->frame_type == EAC3_FRAME_TYPE_DEPENDENT) {
> -        uint64_t ich_layout = ff_ac3_channel_layout_tab[s->prev_output_mode & ~AC3_OUTPUT_LFEON];
> -        int channel_map_size = ff_ac3_channels_tab[s->output_mode & ~AC3_OUTPUT_LFEON] + s->lfe_on;
> -        uint64_t channel_layout;
> -        int extend = 0;
> -
> -        if (s->prev_output_mode & AC3_OUTPUT_LFEON)
> -            ich_layout |= AV_CH_LOW_FREQUENCY;
> -
> -        channel_layout = ich_layout;
> -        for (ch = 0; ch < 16; ch++) {
> -            if (s->channel_map & (1 << (EAC3_MAX_CHANNELS - ch - 1))) {
> -                channel_layout |= ff_eac3_custom_channel_map_locations[ch][1];
> -            }
> -        }
> -        if (av_popcount64(channel_layout) > EAC3_MAX_CHANNELS) {
> -            av_log(avctx, AV_LOG_ERROR, "Too many channels (%d) coded\n",
> -                   av_popcount64(channel_layout));
> -            return AVERROR_INVALIDDATA;
> -        }
> -
> -        av_channel_layout_uninit(&avctx->ch_layout);
> -        av_channel_layout_from_mask(&avctx->ch_layout, channel_layout);
> -
> -        for (ch = 0; ch < EAC3_MAX_CHANNELS; ch++) {
> -            if (s->channel_map & (1 << (EAC3_MAX_CHANNELS - ch - 1))) {
> -                if (ff_eac3_custom_channel_map_locations[ch][0]) {
> -                    int index = av_channel_layout_index_from_channel(&avctx->ch_layout,
> -                                                                     ff_ctzll(ff_eac3_custom_channel_map_locations[ch][1]));
> -                    if (index < 0)
> -                        return AVERROR_INVALIDDATA;
> -                    if (extend >= channel_map_size)
> -                        break;
> -
> -                    extended_channel_map[index] = offset + channel_map[extend++];
> -                } else {
> -                    int i;
> -
> -                    for (i = 0; i < 64; i++) {
> -                        if ((1ULL << i) & ff_eac3_custom_channel_map_locations[ch][1]) {
> -                            int index = av_channel_layout_index_from_channel(&avctx->ch_layout, i);
> -                            if (index < 0)
> -                                return AVERROR_INVALIDDATA;
> -                            if (extend >= channel_map_size)
> -                                break;
> -
> -                            extended_channel_map[index] = offset + channel_map[extend++];
> -                        }
> -                    }
> -                }
> -            }
> -        }
> -
> -        ac3_downmix(avctx);
> -    }
> -
>       /* get output buffer */
>       frame->nb_samples = s->num_blocks * AC3_BLOCK_SIZE;
>       if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241124/2dd3eae0/attachment.sig>


More information about the ffmpeg-devel mailing list