[FFmpeg-devel] [PATCH] ALS decoder

[Thilo Borgmann]

Thilo Borgmann schrieb:
> 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

Ping. Need input.

-Thilo