[FFmpeg-devel] [PATCH] make libavcodec use bytestream functions

[Michael Niedermayer]

Hi

On Wed, May 23, 2007 at 12:51:46PM -0300, Ramiro Ribeiro Polla wrote:
> Michael Niedermayer wrote:
> >Hi
> >
> >On Wed, May 09, 2007 at 08:37:22PM -0300, Ramiro Ribeiro Polla wrote:
> >  
> >>Michael Niedermayer wrote:
> >>    
> >>>Hi
> >>>
> >>>On Fri, Mar 16, 2007 at 01:45:52AM -0300, Ramiro Polla wrote:
> >>>  
> >>>      
> >>>>Hello,
> >>>>
> >>>>Attached patches (one per file) make libavcodec use bytestream 
> >>>>functions and AV_[RW]xx macros.
> >>>>
> >>>>It might be a good idea to cat the final reviewed patches and commit as 
> >>>>one patch, or else there'll be a big ammount of commits...
> >>>>
> >>>>Regression tests succeeded.
> >>>>    
> >>>>        
> >>>patches look ok
> >>>
> >>>  
> >>>      
> >>What's the best way to apply these patches?
> >>1 one big patch
> >>2 one for each file
> >>3 one for each kind of modification for each file
> >>4 one for each kind of modification for the whole libavcodec
> >>    
> >
> >i prefer 4.
> >
> >  
> 
> Applied removal of duplicate bytestream functions.
> 
> Anyone care to triple check attached patch before I commit it?
> 
> You once said to not hide *dst++ in {get,put}_byte() functions. Should I 
> go through and also remove those?
> (or even remove bytestream_{get,put}_byte)

well, i would use *foo++ when its possible but i wont reject a patch
if it uses bytestream_*_byte() for consistency ...

[...]

> @@ -274,7 +274,7 @@
>          ctx->pic.palette_has_changed = 1;
>          // palette starts from index 1 and has 127 entries
>          for (i = 1; i <= ctx->palsize; i++) {
> -            ctx->pal[i] = (buf[0] << 16) | (buf[1] << 8) | buf[2];
> +            ctx->pal[i] = AV_RB24(buf);
>              buf += 3;

shouldnt this be bytestream_get ... ?


[...]
> @@ -833,9 +830,7 @@
>              if (alpha && alpha != 0xff)
>                  has_alpha = 1;
>              *alpha_ptr++ = alpha;
> -            ptr[0] = v >> 16;
> -            ptr[1] = v >> 8;
> -            ptr[2] = v;
> +            AV_WB24(v);
>              ptr += 3;

this doesnt look correct


>          }
>          png_write_chunk(&s->bytestream, MKTAG('P', 'L', 'T', 'E'), s->buf, 256 * 3);
> Index: libavcodec/oggvorbis.c
> ===================================================================
> --- libavcodec/oggvorbis.c	(revision 9108)
> +++ libavcodec/oggvorbis.c	(working copy)
> @@ -234,8 +234,8 @@
>  
>      if(p[0] == 0 && p[1] == 30) {
>          for(i = 0; i < 3; i++){
> -            hsizes[i] = *p++ << 8;
> -            hsizes[i] += *p++;
> +            hsizes[i] = AV_RB16(p);
> +            p += 2;

bytestream_get ?
and there are more of these ...


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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20070523/e9ac22a6/attachment.pgp>