[FFmpeg-devel] [PATCH] [1/??] [2/3] Basic infrastructure
Vitor Sessak
vitor1001
Fri Feb 15 17:39:31 CET 2008
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?
>
> [...]
>> + if(picture.linesize[2])
>> + picture.linesize[1] = picture.linesize[2] =
>> + FFMAX(picture.linesize[1],
>> + picture.linesize[2]);
>
> when is this needed?
Never. I misunderstood the original code?
-Vitor
More information about the ffmpeg-devel
mailing list