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

[Paul B Mahol]

On 3/27/18, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> 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.

Yes, will add.

>
>> +    }
>> +
>> +    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?

Sure, but not right now.