[Ffmpeg-devel] [PATCH] ported SGI decoder to the new API

[Xiaohui Sun]

Michael Niedermayer wrote:
> Hi
>
> On Thu, Mar 29, 2007 at 06:54:19PM +0800, Xiaohui Sun wrote:
>   
[...]

>> +    avctx->codec_id = CODEC_ID_SGI;
>>     
>
> uneeded codec_id has to be at that value already
>
> [...]
>   
>> Index: libavcodec/sgienc.c
>> ===================================================================
>> --- libavcodec/sgienc.c	(revision 0)
>> +++ libavcodec/sgienc.c	(revision 0)
>> @@ -0,0 +1,258 @@
>>     
> [...]
>   
>> +static int encode_init(AVCodecContext *avctx){
>> +    SgiContext *s = avctx->priv_data;
>> +
>> +    avcodec_get_frame_defaults((AVFrame*)&s->picture);
>> +    avctx->coded_frame = (AVFrame*)&s->picture;
>> +
>> +    return 0;
>> +}
>>     
>
> this function is still duplicated
>
>   

I think we don't need this function and put these code in 
encode_frame(), right?

[...]

>   
>> **
>> + * RLE compress one line of image, with maximum size of out_size
>> + * @param w Image width
>> + * @param bpp Bytes per pixel
>> + * @param step Image pixel step
>> + * @param ptr Input buffer
>> + * @param outbuf Output buffer
>> + * @param out_size Maximum output size
>> + * @return size of output in bytes, or -1 if larger than out_size
>> + */
>> +int encode_rle(int w, int bpp, int step, char* ptr, char* outbuf, int out_size)
>> +{
>> +    int x, y, count;
>> +    char* out = outbuf;
>> +    int span = step * bpp;
>> +
>> +    for (x = 0; x < w; x += count * step) {
>> +        /* see if we can encode the next set of pixels with RLE */
>> +        if ((count = count_pixels(ptr, w-x, step, bpp, 1)) > 1) {
>> +            if (out + bpp + 1 > outbuf + out_size) return -1;
>> +            *out++ = count;
>> +            if (bpp == 1)
>> +                *out = *ptr;
>> +            else
>> +                memcpy(out, ptr, bpp);
>> +            out += bpp;
>> +        } else {
>> +            /* fall back on uncompressed */
>> +            count = count_pixels(ptr, w-x, step, bpp, 0);
>> +            *out++ = 0x80 | count;
>> +
>> +            if (out + bpp * count + 1 > outbuf + out_size) return -1;
>> +            if (step == 1)
>> +                memcpy(out, ptr, bpp * count);
>> +            else {
>> +                for (y = 0; y < count; y++)
>> +                    memcpy(out + y, ptr + span * y, bpp);
>> +            }
>> +            out += bpp * count;
>> +        }
>> +        ptr += count * span;
>> +    }
>> +    *out++ = 0;
>> +    return (out - outbuf);
>> +}
>>     
>
> function is more complex then needed also it needs to have a ff_ prefix
>
>
>   

May I need to judge the bpp at the beginning of the function ?