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

[Ramiro Ribeiro Polla]

Hello,

I've been incredibly sick for the past week, but now I'm back.


Michael Niedermayer wrote:
> 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
>
>
>   
Fixed
>>          }
>>          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 ...
>
>   

I wanted to first change some to av_xx and then bytestream, but now it 
seems like it was an unnecessary step.

I'll commit attached patch later unless someone notices anything still 
wrong.

Ramiro Polla
-------------- next part --------------
A non-text attachment was scrubbed...
Name: byte3.diff
Type: text/x-patch
Size: 29317 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070601/7f6b626b/attachment.bin>