[Ffmpeg-devel] [PATCH] ported SGI decoder to the new API
Michael Niedermayer
michaelni
Sun Apr 1 20:59:43 CEST 2007
Hi
On Mon, Apr 02, 2007 at 12:05:07AM +0800, Xiaohui Sun wrote:
> Michael Niedermayer wrote:
> >Hi
> >
> >On Sun, Apr 01, 2007 at 08:38:24PM +0800, Xiaohui Sun wrote:
> >[...]
> >
> >>Index: libavcodec/rle.c
> >>===================================================================
> >>--- libavcodec/rle.c (revision 0)
> >>+++ libavcodec/rle.c (revision 0)
> >>@@ -0,0 +1,253 @@
> >>
> >
> >your rle encoder has as many lines of code as the whole targa encoder
> >this can be done simpler
> >
>
> I think I need to optimize for 1bpp and for step encoding situation ,
> which is needed for SGI encoding.
optimization is certainly nice yes but you dont need that much complexity
for it
>
> [...]
> >
> >[...]
> >
> >>+ start_offset = bytestream_get_be32(&start_table);
> >>+ len = bytestream_get_be32(&length_table);
> >>+ if(in_buf + start_offset + len > end_buf) {
> >>+ return AVERROR_INVALIDDATA;
> >>+ }
> >>
> >
> >this check still is not catching all cases, also it seems you
> >missunderstood
> >me, i didnt mean that you check the values in an otherwise unused table but
> >rather that you properly check the values you do use for overflow
> >
> >
> >
>
> you means I should check if the start_offset is overflow, and check
> against INT32_MAX?
maybe though maybe i missunderstand you again but you definitly should
check the thing so that it wont segfault when dereferenced
>
> >>+ for (x = s->width; x > 0; x--) {
> >>+ ptr = in_buf;
> >>+ offset = 0;
> >>+ for(z = 0; z < s->depth; z ++) {
> >>+ ptr += offset;
> >>+ bytestream_put_byte(&dest_row, *ptr);
> >>+ }
> >>+ in_buf ++;
> >>+ }
> >>
> >
> >this is buggy
> >
>
> yes, it seems quite ugly, since I want to do a discrete read and a
> sequential write, so I move the pointer each time.
it is buggy (=not working)
>
> >
> >[...]
> >
> >>+ case PIX_FMT_RGB32:
> >>+ dimension = SGI_MULTI_CHAN;
> >>+ depth = SGI_RGBA;
> >>+ break;
> >>
> >
> >i think this is broken on either little or big endian
> >
> >[...]
> >
>
> should I use PIX_FMT_RGBA instead, which is endian independant.
you should use the correct one,that is the one which works
they are documented in libavutil/avutil.h
>
> >>+ return (buf - orig_buf);
> >>
> >
> >superfluous ()
> >
> I need to calculate the encoded size here.
yes you do but you do not need the ()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The educated differ from the uneducated as much as the living from the
dead. -- Aristotle
-------------- 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/20070401/39729e8c/attachment.pgp>
More information about the ffmpeg-devel
mailing list