[FFmpeg-devel] [PATCH] avcodec/ac3: add support for dependent stream

[Hendrik Leppkes]

On Tue, Mar 27, 2018 at 1:57 PM, Paul B Mahol <onemda at gmail.com> wrote:
>      /* keep last block for error concealment in next frame */
>      for (ch = 0; ch < s->out_channels; ch++)
> -        memcpy(s->output[ch], output[ch], AC3_BLOCK_SIZE*sizeof(SHORTFLOAT));
> +        memcpy(s->output[ch + offset], output[ch], AC3_BLOCK_SIZE*sizeof(SHORTFLOAT));
> +
> +    /* check if there is dependent frame */
> +    if (buf_size > s->frame_size) {
> +        AC3HeaderInfo hdr;
> +        int err;
> +
> +        if ((ret = init_get_bits8(&s->gbc, buf + s->frame_size, buf_size - s->frame_size)) < 0)
> +            return ret;
> +
> +        err = ff_ac3_parse_header(&s->gbc, &hdr);
> +        if (err)
> +            return err;
> +
> +        if (hdr.frame_type == EAC3_FRAME_TYPE_DEPENDENT) {
> +            buf += s->frame_size;
> +            buf_size -= s->frame_size;
> +            s->prev_output_mode = s->output_mode;
> +            goto dependent_frame;
> +        }

Maybe some general safety checks might be reasonable here? Like, same
sample rate, number of audio blocks being equal to the core? The 'hdr'
variable has all of that anyway.

> +    }
> +
> +    frame->decode_error_flags = err ? FF_DECODE_ERROR_INVALID_BITSTREAM : 0;
> +
> +    for (ch = 0; ch < 16; ch++)
> +        extended_channel_map[ch] = ch;
> +
> +    if (s->frame_type == EAC3_FRAME_TYPE_DEPENDENT) {
> +        uint64_t ich_layout = avpriv_ac3_channel_layout_tab[s->prev_output_mode & ~AC3_OUTPUT_LFEON];
> +        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 << (15 - ch))) {
> +                channel_layout |= custom_channel_map_locations[ch][1];
> +            }
> +        }
> +
> +        avctx->channel_layout = channel_layout;
> +        avctx->channels = av_get_channel_layout_nb_channels(channel_layout);
> +
> +        for (ch = 0; ch < 16; ch++) {
> +            if (s->channel_map & (1 << (15 - ch))) {
> +                if (custom_channel_map_locations[ch][0]) {
> +                    int index = av_get_channel_layout_channel_index(channel_layout,
> +                                                                    custom_channel_map_locations[ch][1]);
> +                    if (index < 0)
> +                        return AVERROR_INVALIDDATA;
> +                    extended_channel_map[index] = offset + channel_map[extend++];
> +                } else {
> +                    int i;
> +
> +                    for (i = 0; i < 64; i++) {
> +                        if ((1LL << i) & custom_channel_map_locations[ch][1]) {
> +                            int index = av_get_channel_layout_channel_index(channel_layout,
> +                                                                            1LL << i);
> +                            if (index < 0)
> +                                return AVERROR_INVALIDDATA;
> +                            extended_channel_map[index] = offset + channel_map[extend++];
> +                        }
> +                    }
> +                }
> +            }
> +        }
> +    }
> +

The channel mapping code looks a bit confusing, but I don't have any
idea how to make it any better right now either, so..

--
Maybe a fate test might be useful?

- Hendrik