[FFmpeg-devel] [PATCH] [1/??] [2/3] Basic infrastructure

Vitor Sessak vitor1001
Thu Feb 14 21:02:23 CET 2008


Hi

Michael Niedermayer wrote:
> On Tue, Feb 12, 2008 at 06:59:53PM +0100, Vitor Sessak wrote:
>> Hi
>>
>> Michael Niedermayer wrote:
>>> On Mon, Feb 11, 2008 at 08:07:10PM +0100, Vitor Sessak wrote:
>>> [...]
>>>>>>         for(i = 0; i < 4; i ++) {
>>>>>>             if(!src[i]) continue;
>>>>>>
>>>>>>             for(j = 0; j < h >> (i==0 ? 0 : vsub); j ++) {
>>>>>>                 memcpy(dst[i], src[i], link->cur_pic->linesize[i]);
>>>>> this should be width*pixel size or something like that ideally
>>>> Well, that would mean duplicating a lot of code already in 
>>>> av_picture_copy(), unless I create a
>>>> int av_get_plane_bytewidth(int pix_fmt, int width, int plane)
>>>> to factorate this code (see attached patch). Are you ok with that? 
>>> Ok, but give pix_fmt its proper type not int.
>>>
>>>
>>>> Do you think I should make it inline?
>>> no
>>>
>>>
>>>>>>                 src[i] += link->srcpic ->linesize[i];
>>>>>>                 dst[i] += link->cur_pic->linesize[i];
>>>>>>             }
>>>>>>         }
>>>>> [...]
>>>>>>     if(!link_dpad(link).draw_slice)
>>>>>>         return;
>>>>>>
>>>>>>     link_dpad(link).draw_slice(link, y, h);
>>>>>> }
>>>>> if(link_dpad(link).draw_slice)
>>>>>     link_dpad(link).draw_slice(link, y, h);
>>>>> [...]
>>>>>>     avpicture_alloc((AVPicture *)pic, pic->format,
>>>>>>                     (ref->w + 15) & (~15), // make linesize a multiple of 
>>>>>> 16
>>>>> wont work for chroma
>>>> It's a bit complicated... I need either to duplicate code in 
>>>> avpicture_alloc, either rounding up to the next multiple of 16<<hsub. Any 
>>>> ideas?
>>> see avcodec_default_get_buffer() if you figured out a way to clean it up
>>> it also does something similar ...
>> It's pretty complex. I have two other ideas:
>>
>> 1- Make avpicture_fill always create aligned linesizes
> 
> Will need a fix somewhere in the raw video code i think, as that uses
> avpicture_fill and expects linesize=width.
> 
> 
>> or
>>
>> 2- Split avpicture_fill in two functions. One that sets the linesize and 
>> other that do all the rest, using linesize[i] and height to figure the 
>> sizes. So, to align linesizes
>>
>> avpicture_fill_linesizes(&pic, ...);
>> for (i) align(pic.linesize[i]);
>> buf = av_malloc(...);
>> avpicture_fill_buf(&pic, buf, ...);
>>
>>
>> and a third option would be trying to create a cleaner version of 
>> avcodec_default_get_buffer()...
>>
>> What do you think?
> 
> I dunno, but theres no reason not to cleanup avcodec_default_get_buffer()
> if you think tha would help.

Ok, so fill_split.diff do what I describe as idea No. 2 and 
get_buffer.diff tries to make avcodec_default_get_buffer() a little less 
obfuscated using code added in fill_split.diff. It can be simplified 
further, but it was already painful to simplify it a little without 
breaking anything.

Regression tests pass.

-Vitor

-------------- next part --------------
A non-text attachment was scrubbed...
Name: fill_split.diff
Type: text/x-patch
Size: 9184 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080214/1f1e8630/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: get_buffer.diff
Type: text/x-patch
Size: 2457 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080214/1f1e8630/attachment-0001.bin>



More information about the ffmpeg-devel mailing list