[FFmpeg-devel] [PATCH] brender_pix: a new image decoder

[Aleksi Nurmi]

2012/11/15 Paul B Mahol <onemda at gmail.com>:
> Such text about background story should be in separate comment block
> bellow copyright,

Fixed.

> This is not libav mailing list/project.

Sorry. (Copied it from a file that was merged from libav.)

> unused

Good catch.

> No much point in separate struct.

The point is that the palette has its own header struct.

> Please follow style of other files and put { in separate line.
> ditto

Done.

> redundant "PIX"

Removed.

> please return maningful error code (the one returned by get_buffer())

Fixed.

>> +        for (int i = 0; i < 256; ++i) {
>> +            bytestream2_skipu(&gb, 1);
>> +            *pal_out++ = (0xFFU << 24) | bytestream2_get_be24u(&gb);
>
> IMHO overwritting instead of this would be better.

I don't mind either way, but I think I'm going to leave it like this.

>> +    // read the image data to the buffer
>> +    {
>
> Ugly.
>> +        unsigned int bytes_per_scanline = bytes_pp * hdr.width;
>> +        unsigned int bytes_left = bytestream2_get_bytes_left(&gb);

I agree, but now the reader needs to carry less context in his/her
mind. Left them as-is for now, but I don't really care.

> put { above and after ) .

Done.

I'll post the updated patch soon.