[FFmpeg-devel] [PATCH] [WIP] Updated MLP encoder - Qualification Task (TrueHD)

[Michael Niedermayer]

On Sun, Apr 10, 2016 at 04:57:33AM +0530, Jai Luthra wrote:
> Hi,
> 
> I've updated the patch (attached):
> * Fixed the build warnings
> * Applied atomnuker's changes to update the mlp_encode_frame function
> * Added AudioFrameQueue as suggested by atomnuker
> 
> The encoder now works without any errors to silence and can encode
> stereo files with a valid bitstream.
> 
> I guess the qualification task is now complete (except validating
> bitstream without using ffmpeg)
> 

> I've noticed that it doesn't generate valid bitstream for mono files.
> ffplay's output is full of:
> [mlp @ 0x7fd2b4001980] No restart header present in substream 0.
> [mlp @ 0x7fd2b4001980] No samples to output.
> [mlp @ 0x7fd2b4001980] Max channel must be equal max matrix channel.
> 
> I'll find a fix for this asap, and might need some help from the mentor if I
> get stuck.

if you havnt found the issue yet, the slow but foolproof way to
find such bugs is to print all kinds or things like values written
to the bitstream and also print whats read in the decoder and similarly
all the matching things that you printed in the encoder
and compare with diff or equivalent to find the spot where things
start to differ

one can do that by more or less brute force or in some sort of
bisect over the code approuch

[...]

> diff --git a/libavcodec/mlp.c b/libavcodec/mlp.c
> index 87f7c77..1a56d9f 100644
> --- a/libavcodec/mlp.c
> +++ b/libavcodec/mlp.c
> @@ -41,6 +41,20 @@ const uint8_t ff_mlp_huffman_tables[3][18][2] = {
>      }
>  };
>

> +ChannelInformation ff_mlp_ch_info[21] = {

const



> +    { 0x01, 0x01, 0x00, 0x1f }, { 0x03, 0x02, 0x00, 0x1b },
> +    { 0x07, 0x02, 0x01, 0x1f }, { 0x0F, 0x02, 0x02, 0x19 },
> +    { 0x07, 0x02, 0x01, 0x03 }, { 0x0F, 0x02, 0x02, 0x1f },
> +    { 0x1F, 0x02, 0x03, 0x01 }, { 0x07, 0x02, 0x01, 0x1a },
> +    { 0x0F, 0x02, 0x02, 0x1f }, { 0x1F, 0x02, 0x03, 0x18 },
> +    { 0x0F, 0x02, 0x02, 0x02 }, { 0x1F, 0x02, 0x03, 0x1f },
> +    { 0x3F, 0x02, 0x04, 0x00 }, { 0x0F, 0x03, 0x01, 0x1f },
> +    { 0x1F, 0x03, 0x02, 0x18 }, { 0x0F, 0x03, 0x01, 0x02 },
> +    { 0x1F, 0x03, 0x02, 0x1f }, { 0x3F, 0x03, 0x03, 0x00 },
> +    { 0x1F, 0x04, 0x01, 0x01 }, { 0x1F, 0x04, 0x01, 0x18 },
> +    { 0x3F, 0x04, 0x02, 0x00 },
> +};
[...]

> +/** Min and max values that can be encoded with each codebook. The values for
> + *  the third codebook take into account the fact that the sign shift for this
> + *  codebook is outside the coded value, so it has one more bit of precision.
> + *  It should actually be -7 -> 7, shifted down by 0.5.
> + */
> +static int codebook_extremes[3][2] = {

const


> +    {-9, 8}, {-8, 7}, {-15, 14},
> +};
[...]
> +input_and_return:
> +
> +    if (data) {
> +        ctx->frame_size[ctx->frame_index] = avctx->frame_size;
> +        ctx->next_major_frame_size += avctx->frame_size;
> +        ctx->next_major_number_of_frames++;
> +        input_data(ctx, data);
> +    } else if (!ctx->last_frame) {
> +        ctx->last_frame = ctx->inout_buffer;
> +    }
> +
> +    restart_frame = (ctx->frame_index + 1) % ctx->min_restart_interval;
> +
> +    if (!restart_frame) {
> +    int seq_index;
> +
> +    for (seq_index = 0;
> +         seq_index < ctx->restart_intervals && (seq_index * ctx->min_restart_interval) <= ctx->avctx->frame_number;
> +         seq_index++) {
> +        unsigned int number_of_samples = 0;
> +        unsigned int index;
> +
> +        ctx->sample_buffer = ctx->major_scratch_buffer;
> +        ctx->inout_buffer = ctx->major_inout_buffer;
> +        ctx->seq_index = seq_index;
> +
> +        ctx->starting_frame_index = (ctx->avctx->frame_number - (ctx->avctx->frame_number % ctx->min_restart_interval)
> +                                  - (seq_index * ctx->min_restart_interval)) % ctx->max_restart_interval;
> +        ctx->number_of_frames = ctx->next_major_number_of_frames;
> +        ctx->number_of_subblocks = ctx->next_major_number_of_frames + 1;
> +
> +        ctx->seq_channel_params = (ChannelParams *) ctx->channel_params +
> +                                  (ctx->frame_index / ctx->min_restart_interval)*(ctx->sequence_size)*(ctx->avctx->channels) +
> +                                  (ctx->seq_offset[seq_index])*(ctx->avctx->channels);
> +
> +        ctx->seq_decoding_params = (DecodingParams *) ctx->decoding_params +
> +                                   (ctx->frame_index / ctx->min_restart_interval)*(ctx->sequence_size)*(ctx->num_substreams) +
> +                                   (ctx->seq_offset[seq_index])*(ctx->num_substreams);
> +
> +        for (index = 0; index < ctx->number_of_frames; index++) {
> +            number_of_samples += ctx->frame_size[(ctx->starting_frame_index + index) % ctx->max_restart_interval];
> +        }
> +        ctx->number_of_samples = number_of_samples;
> +
> +        for (index = 0; index < ctx->seq_size[seq_index]; index++) {
> +            clear_channel_params(ctx, ctx->seq_channel_params + index*(ctx->avctx->channels));
> +            default_decoding_params(ctx, ctx->seq_decoding_params + index*(ctx->num_substreams));
> +        }
> +
> +        input_to_sample_buffer(ctx);
> +
> +        analyze_sample_buffer(ctx);
> +    }
> +

> +    if (ctx->frame_index == (ctx->max_restart_interval - 1)) {
> +        ctx->major_frame_size = ctx->next_major_frame_size;
> +        ctx->next_major_frame_size = 0;
> +        ctx->major_number_of_frames = ctx->next_major_number_of_frames;
> +        ctx->next_major_number_of_frames = 0;
> +
> +        if (!ctx->major_frame_size)
> +            goto no_data_left;
> +    }
> +    }

something has gone wrong with the indention here

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160412/ac275342/attachment.sig>