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

[Ramiro Ribeiro Polla]

Ramiro Ribeiro Polla wrote:
> 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.
>

Applied