[FFmpeg-devel] [PATCH] [1/??] [2/3] Basic infrastructure
Vitor Sessak
vitor1001
Mon Feb 11 20:07:10 CET 2008
Hi
Michael Niedermayer wrote:
> On Sun, Feb 10, 2008 at 09:56:35PM +0100, Vitor Sessak wrote:
>> Hi
>>
>> Michael Niedermayer wrote:
>>> On Sun, Feb 10, 2008 at 05:59:43PM +0100, Vitor Sessak wrote:
>>>> Hi
>>>>
>>>> Michael Niedermayer wrote:
>>>>> On Sun, Feb 10, 2008 at 11:03:48AM +0100, Vitor Sessak wrote:
>>>>>> Hi and thanks for the review
[...]
>> Agreed with everything. New patch attached.
> [...]
>> void avfilter_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
>> AVFilterPad **pads, AVFilterLink ***links,
>> AVFilterPad *newpad)
>> {
>> unsigned i;
>>
>> idx = FFMIN(idx, *count);
>>
>> *pads = av_realloc(*pads, sizeof(AVFilterPad) * (*count + 1));
>> *links = av_realloc(*links, sizeof(AVFilterLink*) * (*count + 1));
>> memmove(*pads +idx+1, *pads +idx, sizeof(AVFilterPad) * (*count-idx));
>> memmove(*links+idx+1, *links+idx, sizeof(AVFilterLink*) * (*count-idx));
>> memcpy(*pads+idx, newpad, sizeof(AVFilterPad));
>> (*links)[idx] = NULL;
>>
>> (*count) ++;
>> for(i = idx+1; i < *count; i ++)
>> if(*links[i])
>> (*(unsigned *)((uint8_t *)(*links[i]) + padidx_off)) ++;
>
> the () around links seem superflous
indeed. done.
>
>
>> }
>>
>> int avfilter_link(AVFilterContext *src, unsigned srcpad,
>> AVFilterContext *dst, unsigned dstpad)
>> {
>> AVFilterLink *link;
>>
>> if(src->output_count <= srcpad || dst->input_count <= dstpad ||
>> src->outputs[srcpad] || dst->inputs[dstpad])
>> return -1;
>>
>
>> src->outputs[srcpad] =
>> dst->inputs[dstpad] = link = av_mallocz(sizeof(AVFilterLink));
>
> this can be aligned nicer:
> src->outputs[srcpad] =
> dst-> inputs[dstpad] = link = av_mallocz(sizeof(AVFilterLink));
>
>
> [...]
>> if((config_link = link_dpad(link).config_props))
>> if(config_link(link))
>> return -1;
>
> indention ...
ok
>
>
> [...]
>> int avfilter_request_frame(AVFilterLink *link)
>> {
>> if(link_spad(link).request_frame)
>> return link_spad(link).request_frame(link);
>> else if(link->src->inputs[0])
>> return avfilter_request_frame(link->src->inputs[0]);
>> else return -1;
>> }
>>
>> int avfilter_poll_frame(AVFilterLink *link)
>> {
>> int i, min=INT_MAX;
>>
>> if(link_spad(link).poll_frame)
>> return link_spad(link).poll_frame(link);
>> else
>> for (i=0; i<link->src->input_count; i++) {
>> if(!link->src->inputs[i])
>> return -1;
>> min = FFMIN(min, avfilter_poll_frame(link->src->inputs[i]));
>> }
>
> the elses arent needed due to the returns
ok
>
>
> [...]
>> src[0] = link->srcpic-> data[0] + y * link->srcpic-> linesize[0];
>> dst[0] = link->cur_pic->data[0] + y * link->cur_pic->linesize[0];
>> for(i = 1; i < 4; i ++) {
>> if(link->srcpic->data[i]) {
>> src[i] = link->srcpic-> data[i] + (y >> vsub) * link->srcpic-> linesize[i];
>> dst[i] = link->cur_pic->data[i] + (y >> vsub) * link->cur_pic->linesize[i];
>> } else
>> src[i] = dst[i] = NULL;
>> }
>
> the src/dst[0] can be put in the loop and then this and te next loop can be
> merged
Ok.
>
>> 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? Do
you think I should make it inline?
>
>
>> 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?
Also, do you think it is worth also rounding up the height to the next
multiple of 16 as I've done? Thinking again, it don't look very useful...
-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: get_plane_bwidth.diff
Type: text/x-patch
Size: 2031 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080211/318e83bd/attachment.bin>
More information about the ffmpeg-devel
mailing list