[FFmpeg-devel] [PATCH] ALS decoder

[Thilo Borgmann]

Reimar D?ffinger schrieb:
> On Tue, Aug 25, 2009 at 03:37:11PM +0200, Thilo Borgmann wrote:
>>>> +        ctx->last_frame_length = samples % ctx->sconf.frame_length;
>>>> +        if (!ctx->last_frame_length) {
>>>> +            ctx->last_frame_length = sconf->frame_length;
>>>> +        }
>>> thats the same as
>>> (L-1) % F + 1
>>> i think (unchecked though)
>> Missing lvalue and independent of samples?
> 
> l == samples and F == ctx->sconf.frame_length
if L == samples it makes sense. Tested and approved.


> Though I have to wonder what the difference between
> ctx->sconf.frame_length and sconf->frame_length is...
Ups that's left from a previous version... done.



> 
>>>> +            // damaged block, write zero for the rest of the frame
>>>> +            while (b < ctx->num_blocks) {
>>>> +                memset(raw_sample, 0, div_blocks[b]);
>>>> +                raw_sample += div_blocks[b];
>>>> +                b++;
>>>> +            }
>>>> +            return -1;
>>> [...]
>>>> +            // damaged block, write zero for the rest of the frame
>>>> +            while (b < ctx->num_blocks) {
>>>> +                memset(raw_samples_L, 0, div_blocks[b]);
>>>> +                memset(raw_samples_R, 0, div_blocks[b]);
>>>> +                raw_samples_L += div_blocks[b];
>>>> +                raw_samples_R += div_blocks[b];
>>>> +                b++;
>>>> +            }
>>> [...]
>>>> +                // damaged block, write zero for the rest of the frame
>>>> +                while (b < ctx->num_blocks) {
>>>> +                    memset(raw_samples_L, 0, div_blocks[b]);
>>>> +                    raw_samples_L += div_blocks[b];
>>>> +                    b++;
>>>> +                }
>>> cant these be factored/combined?
>> They could be factored into decode_frame() but several parameters would
>> have to be passed somehow to know wich samples of which channel could be
>> decoded correctly already.
>> Alternative 1) loose all samples of the current RA unit.
>> Alternative 2) infunction {while() memset;} but this would add a second
>> loop for the code block in the middle.
>> Which way to go?
> 
> Hm, neither?
> get_remain_count():
>   while (b < ctx->num_blocks) sum += div_blocks[b];
>   return sum;
> 
> count = get_remain_count();
> memset(raw_samples_L, 0, count);
> raw_samples_L += count;
> memset(raw_samples_R, 0, count);
> raw_samples_R += count;
Very close to alternative 2.
Ok, I think I do it like this then.

> 
> Though personally I'd suggest expanding the memset+increment
> into a bytestream_fill_byte() that does both.
If it would be beneficial compared to the other solution, I would
rewrite it, but I think it will add some kind of bytestream context or
similar overhead for a one-time usage? (Haven't used bytestream yet).


All added to revision 9.

Thanks!

-Thilo