[FFmpeg-devel] [PATCH] Canopus Lossless decoder

[Derek Buitenhuis]

On 27/07/2012 9:51 PM, Michael Niedermayer wrote:
> this has a high chance to break compatibility of the codec ids between
> forks

Uh, I placed it where I thought it wouldn't break it (right under
Kostya's last works).

Where do you suggest?

> wrong project

Fixed in the v2 I sent earlier.

>> +            for (k = prefix << sym_shift; k < prefix + 1 << sym_shift; k++) {
>> +                code_table[2 * k]     = symbol;
>> +                code_table[2 * k + 1] = i + 1;
>> +            }
> 
> looks like it can overwrite the buffer

I'll have to stare at this a bit.. brain dead tonight.

I know I should add a check to see if there are bits left, as well.

>> +        if (num_codes)
>> +            num_lens = num_lens_orig;
> 
> does nothing

Left over cruft from hacking. Will fix.


>> +    /* Simultaniously read and restore the line */
>> +    for (i = 0; i < ctx->avctx->width; i++) {
>> +        pred += code_table[2 * show_bits(gb, 14)];
>> +        skip  = code_table[2 * show_bits(gb, 14) + 1];
>> +
>> +        skip_bits(gb, skip);
> 
> this looks like a reimplementation of get_vlc*

I am not that familiar with the functions. I'll read them and see if I can
replace this with get_vlc*.

> why is coding_type uint32_t ? the %d would be wrong too in that case

I'll change it to and 8-bit int or just and int, and modify the format
accordingly.

Thanks for the review. I'm going to wait for any more reviews, and send
an update patch tomorrow.

Cheers,
- Derek

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 898 bytes
Desc: OpenPGP digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120727/b5fb4d0b/attachment.asc>