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

[Michael Niedermayer]

Hi

On Tue, Mar 27, 2007 at 11:11:13PM +0800, Xiaohui Sun wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi,
> 
> Michael Niedermayer wrote:
> > Hi
> > 
> > On Tue, Mar 27, 2007 at 03:34:39PM +0800, Xiaohui Sun wrote:
> >> Hi,
> >>      I have updated the path, Thank you again for your suggestions about
> >> my code.
> >>
> >> the new patch
> >> --  ported SGI encoder
> >> --  seperate encoder/decoder into two files
> >> --  fixed some problems of the previous code
> > 
> > [...]
> > 
> >> +        for (y = s->height - 1; y >= 0; y--) {
> >> +            dest_row = ptr + (y * s->linesize);
> >> +            for (x = s->width; x > 0; x--) {
> >> +                dest_row[chan_offset] = bytestream_get_byte(&buf);
> >> +                dest_row += s->depth;
> >> +            }
> >> +        }
> > 
> > sequential writes with random reads are faster then sequential reads with
> > random writes
> > 
> 
> I should alloc an separate buffer, read the data in and use memcpy() to
> do a sequential writes?

no you should just change the code which currently performs 1 pass over the
source and several passes over the destination to a loop which performs
one pass over the destination and several over the source


> 
> [...]
> > [...]
> >> +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;
> >> +}
> > 
> > code duplication
> > 
> > 
> 
> the structure of the avctx->priv_data is not the same in encode and decode

it doesnt have to be the same its enough if picture is at the same position
of course encoder and decoder could also use the same struct


> 
> 
> 
> > [...]
> >> +static int encode_frame(AVCodecContext *avctx, unsigned char *buf, 
> >> +			int buf_size, void *data){
> >> +    SgiContext *s = avctx->priv_data;
> >> +    AVFrame * const p = &s->picture;
> >> +    long *offsettab, *lengthtab;
> >> +    int i, y, z;
> >> +    int tablesize, chan_offset;
> >> +    uint8_t *srcrow;
> >> +    unsigned int width, height, depth, dimension;
> >> +    unsigned int n_bytes;
> >> +    unsigned char *buf0 = buf;
> >> +
> > 
> > there are no checks for buf_size being large enough
> > 
> > 
> 
> we can not get the encoded image size before we actually encode it

that isnt true, you can very well calculate how much space it would need
also we dont need the exact size, some upper bound will do

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070327/c4b7b047/attachment.pgp>