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

Vitor Sessak vitor1001
Fri Feb 15 19:05:30 CET 2008


Hi

Michael Niedermayer wrote:
> On Fri, Feb 15, 2008 at 05:39:31PM +0100, Vitor Sessak wrote:
>> Hi
>>
>> Michael Niedermayer wrote:
>>> On Thu, Feb 14, 2008 at 09:02:23PM +0100, Vitor Sessak wrote:
>>>> 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.
>>> Could you please give the new functions a ff_ prefix instead of avpicture_
>>> and keep them out of public headers like avcodec.h.
>>> The simplification should definitly be seperate of any API extension.
>> Ok. But add it to which header file? Should I create a imgconvert.h?
> 
> thats a possibility ...

Since you didn't say explicitly yes, here is the patch (it also makes 
the function I added in another patch in this thread not to be in the 
public API).

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: imgconvert_h.diff
Type: text/x-patch
Size: 2235 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080215/97afc6a9/attachment.bin>



More information about the ffmpeg-devel mailing list