[FFmpeg-devel] [PATCH] ALS decoder

[Thilo Borgmann]

Michael Niedermayer schrieb:
> On Thu, Sep 03, 2009 at 09:44:25PM +0200, Thilo Borgmann wrote:
>> Revision 16 attached.
> [...]
> 
>> +#define DEBUG 0
> 
> that doesnt belong in here
> 

This is what Reimar and you mentioned yesterday, how else to
"
if(!DEBUG)
 return
"
?


> 
> [...]
>> +typedef struct {
>> +    int resolution;           ///< 000 = 8-bit; 001 = 16-bit; 010 = 24-bit; 011 = 32-bit
>> +    int floating;             ///< 1 = IEEE 32-bit floating-point, 0 = integer
>> +    int frame_length;         ///< frame length for each frame (last frame may differ)
> 
>> +    int ra_distance;          ///< distance between RA frames (in frames, 0...255)
> 
> if each frame is a RA frame that would be 1 i  assume, but then 0...255
> makes no sense as 0 makes no sense

Makes sense. If ra_distance == 0, there are no RA frames at all.




>> +    if (sconf->bgmc) {
>> +        // TODO: BGMC mode
>> +    } else {
>> +        s[0] = get_bits(gb, 4 + (sconf->resolution > 1));
>> +        for (k = 1; k < sub_blocks; k++)
>> +            s[k] = s[k - 1] + decode_rice(gb, 0);
>> +    }
>> +
>> +    if (get_bits1(gb))
>> +        *shift_lsbs = get_bits(gb, 4) + 1;
>> +
>> +
>> +    if (!sconf->rlslms) {
>> +        if (sconf->adapt_order) {
> 
>> +            int opt_order_length = FFMAX(av_ceil_log2((block_length >> 3) - 1), 1);
>> +            opt_order_length     = FFMIN(av_ceil_log2(sconf->max_order+1), opt_order_length);
> 
> this can be calculated as
> av_ceil_log2(av_clip(...))
> 
> which should be simpler and faster

Justin mentioned this during his GSoC review and we agreed that this is
not an av_clip() case. I could not find an equal solution, can you?



>> +    // reconstruct all samples from residuals
>> +    if (ra_block) {
> 
>> +        unsigned int progressive = FFMIN(block_length, opt_order);
> 
> useless, you can just use opt_order instead of progressive in te following
> code

If block_length < opt_order, max would have a wrong value and y would
not be correct any more.
I think the specs don't say anything about it but I think this weird
case might be true for small last frames + block switching + high
prediction order. The reference decoder does this, too.


> 
> 
>> +
>> +        for (smp = 0; smp < block_length; smp++) {
>> +            unsigned int dequant = smp < progressive;
>> +            unsigned int max     = dequant ? smp : progressive;
>> +
>> +            y = 1 << 19;
>> +
>> +            for (sb = 0; sb < max; sb++)
>> +                y += MUL64(lpc_cof[sb],raw_samples[smp - (sb + 1)]);
>> +
>> +            raw_samples[smp] -= y >> 20;
>> +            if (dequant)
>> +                parcor_to_lpc(smp, quant_cof, lpc_cof);
>> +        }
> 
> this could be split in 2 loop with the 2nd being identical to the else
> case below and thus factorizeable i think
> 

If it would be safe to use opt_order exclusively in the case above, then
yes. But I don't think so...

I've tried to generate a sample file where block_length < opt_order case
is true, but this seems to require a suitable piece of sound I don't
seem to have - because one cannot tell how many block switching levels
are to be used.

The question is if factorization is worth the case that there might be
files that can be decoded using the reference decoder but not using FFmpeg?


> [...]
>> +/** 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);
>> +    ra_frame = sconf->ra_distance && !(ctx->frame_id % sconf->ra_distance);
> 
> is it true that if ra_distance == 0 then no frame is a ra frame?
> seems odd
> 

As mentioned above, this is the specified behaviour.
And that is the reason for using av_mallocz() for the raw_buffer buffer,
to have fake 0's for prediction in the carryover samples from frame "-1".

Thanks!

-Thilo