[FFmpeg-devel] [PATCH] ALS decoder

[Thilo Borgmann]

Michael Niedermayer schrieb:
> On Wed, Oct 21, 2009 at 12:50:16PM +0200, Thilo Borgmann wrote:
>> Michael Niedermayer schrieb:
>>> On Thu, Sep 24, 2009 at 05:03:14PM +0200, Thilo Borgmann wrote:
>>>> Revision 23 attached.
>>> [...]
>>>> +/** Reformat block sizes from log2 format to direct form. Also assure that the
>>>> + *  block sizes of the last frame correspond to the actual number of samples.
>>>> + */
>>>> +static void reconstruct_block_sizes(ALSDecContext *ctx, uint32_t *div_blocks)
>>>> +{
>>>> +    unsigned int b;
>>>> +
>>>> +    // The last frame may have an overdetermined block structure given in
>>>> +    // the bitstream. In that case the defined block structure would need
>>>> +    // more samples than available to be consistent.
>>>> +    // The block structure is actually used but the block sizes are adapted
>>>> +    // to fit the actual number of available samples.
>>>> +    // Example: 5 samples, 2nd level block sizes: 2 2 2 2.
>>>> +    // This results in the actual block sizes:    2 2 1 0.
>>>> +    // This is not specified in 14496-3 but actually done by the reference
>>>> +    // codec RM22 revision 2.
>>>> +    // This appears to happen in case of an odd number of samples in the last
>>>> +    // frame which is actually not allowed by the block length switching part
>>>> +    // of 14496-3.
>>>> +    // The ALS conformance files feature an odd number of samples in the last
>>>> +    // frame.
>>>> +
>>>> +    for (b = 0; b < ctx->num_blocks; b++)
>>>> +        div_blocks[b] = ctx->sconf.frame_length >> div_blocks[b];
>>>> +
>>>> +    if (ctx->cur_frame_length == ctx->last_frame_length) {
>>> if(ctx->cur_frame_length != ctx->sconf.frame_length) {
>>> and last_frame_length becomes removeable 
>>>
>> Yes.
>>
>>> [...]
>>>> +/** Decodes an ALS frame.
>>>> + */
>>>> +static int decode_frame(AVCodecContext *avctx,
>>>> +                        void *data, int *data_size,
>>>> +                        AVPacket *avpkt)
>>>> +{
>>>> +    ALSDecContext *ctx       = avctx->priv_data;
>>>> +    ALSSpecificConfig *sconf = &ctx->sconf;
>>>> +    const uint8_t *buffer    = avpkt->data;
>>>> +    int buffer_size          = avpkt->size;
>>>> +    int invalid_frame, size;
>>>> +    unsigned int c, sample, ra_frame, bytes_read, shift;
>>>> +
>>>> +    init_get_bits(&ctx->gb, buffer, buffer_size * 8);
>>>> +
>>>> +    // In the case that the distance between random access frames is set to zero
>>>> +    // (sconf->ra_distance == 0) no frame is treated as a random access frame.
>>>> +    // For the first frame, if prediction is used, all samples used from the
>>>> +    // previous frame are assumed to be zero.
>>>> +    ra_frame = sconf->ra_distance && !(ctx->frame_id % sconf->ra_distance);
>>>> +
>>>
>>>> +    // the last frame to decode might have a different length
>>>> +    if (ctx->num_frames && ctx->num_frames - 1 == ctx->frame_id)
>>>> +        ctx->cur_frame_length = ctx->last_frame_length;
>>> ctx->cur_frame_length= FFMIN(samples - ctx->frame_id*(uint64_t)sconf->frame_length, sconf->frame_length);
>>> and ctx->num_frames becomes unneeded and can be removed
>> Isn't this way slower than before?
> 
> how large are frames? i thought they are large enough for 1 multiply & MIN
> not to matter ...

Sorry I did not recognize before, samples is also unknown here. Thus
samples would have replace num_frames in the context. And this would
bring in some more math here to check for samples == 0xFFFFFFFF like it
is done now in read_specific_config besides having to determine the
minimum afterwards and set cur_frame_length.

To be clear, this would result in something like:

if (ctx->samples != 0xFFFFFFFF)               // ctx->samples is new!
    ctx->cur_frame_length = FFMIN(...);
else
    ctx->cur_frame_length = ctx->frame_length;


Acceptable just to get rid of ctx->last_frame_length?
Yes, frames are large enough to allow some math although what I've
learned from FFmpeg so far, is just *not* to do such things...

-Thilo