[FFmpeg-soc] expand filter (alternative to pad syntax)

[Michael Niedermayer]

On Sun, May 11, 2008 at 07:00:10PM +0900, Ryo Hirafuji wrote:
> I additionary fix the source ^^;
> I'm sorry to post frequently.
> I optimized drawing pad and renamed context structure.
[...]
> +typedef struct{
> +    int x, y, w, h;
> +
> +    int osd; //checked, but not used in this version.

> +    double aspect;

same as for vmrsss, remove all the aspect code please


[...]
> +    expand->x_shift[0] = 

trailing whitespace is forbidden in svn

[...]
> +    if(args){
> +        int length = strlen(args);
> +        char* copy_arg = av_malloc(length+1);
> +        char* item;
> +
> +        memcpy(copy_arg,args,length);
> +        copy_arg[length] = '\0';
> +
> +        item = strtok(copy_arg, ARG_DELIM);/* 1 */
> +        if(item && strlen(item) > 0){
> +            expand->w = atoi(item);
> +        }
> +        item = strtok(0, ARG_DELIM);/* 2 */
> +        if(item && strlen(item) > 0){
> +            expand->h = atoi(item);
> +        }
> +        item = strtok(0, ARG_DELIM);/* 3 */
> +        if(item && strlen(item) > 0){
> +            expand->x = atoi(item);
> +        }
> +        item = strtok(0, ARG_DELIM);/* 4 */

too complex
i suggest you use a single sscanf()


[...]

> +    if (expand->w == -1){
> +        expand->w=width;
> +    } else if (expand->w < -1){
> +        expand->w=width - expand->w;
> +    } else if (expand->w < width){
> +        expand->w=width;
> +    }
> +
> +    if (expand->h == -1){
> +        expand->h=height;
> +    } else if (expand->h < -1){
> +        expand->h=height - expand->h;
> +    } else if (expand->h < height){
> +        expand->h=height;
> +    }

duplicate


[...]

> +    if(expand->x < 0 || (expand->x+width) > expand->w){
> +        expand->x = (expand->w - width)>>1;
> +    }
> +    if(expand->y < 0 || (expand->y+height) > expand->h){
> +        expand->y = (expand->h - height)>>1;
> +    }

duplicate


[...]
> +        case PIX_FMT_YUVJ440P:
> +            expand->is_yuv = 1;
> +            expand->bpp = 1;
> +            expand->x_shift[0] = 0;
> +            expand->y_shift[0] = 0;
> +            expand->x_shift[1] = expand->x_shift[2] = hsub;
> +            expand->y_shift[1] = expand->y_shift[2] = vsub;
> +            break;
> +        case PIX_FMT_YUVA420P:
> +            expand->is_yuv = 1;
> +            expand->bpp = 1;
> +            expand->x_shift[0] = 0;
> +            expand->y_shift[0] = 0;
> +            expand->x_shift[1] = expand->x_shift[2] = hsub;
> +            expand->y_shift[1] = expand->y_shift[2] = vsub;
> +            expand->x_shift[3] = 0;
> +            expand->y_shift[3] = 0;
> +            break;

duplicate


> +        case PIX_FMT_NV12:
> +        case PIX_FMT_NV21:
> +            expand->is_yuv = 1;
> +            expand->bpp = 1;
> +            expand->x_shift[0] = 0;
> +            expand->y_shift[0] = 0;
> +            expand->x_shift[1] = hsub;
> +            expand->y_shift[1] = vsub;
> +            break;
> +        case PIX_FMT_RGB24:
> +        case PIX_FMT_BGR24:
> +            expand->bpp = 3;
> +            expand->x_shift[0] = 0;
> +            expand->y_shift[0] = 0;
> +            break;
> +        case PIX_FMT_RGB32:
> +        case PIX_FMT_BGR32:
> +        case PIX_FMT_RGB32_1:
> +        case PIX_FMT_BGR32_1:

> +            expand->bpp = 4;
> +            expand->x_shift[0] = 0;
> +            expand->y_shift[0] = 0;
> +            break;
> +        case PIX_FMT_YUYV422:
> +        case PIX_FMT_UYVY422:
> +            expand->is_yuv = 1;
> +        case PIX_FMT_GRAY16BE:
> +        case PIX_FMT_GRAY16LE:
> +        case PIX_FMT_BGR555:
> +        case PIX_FMT_BGR565:
> +        case PIX_FMT_RGB555:
> +        case PIX_FMT_RGB565:
> +            expand->bpp = 2;
> +            expand->x_shift[0] = 0;
> +            expand->y_shift[0] = 0;
> +            break;
> +        //case PIX_FMT_UYYVYY411: // not supported.
> +        case PIX_FMT_RGB8:
> +        case PIX_FMT_BGR8:
> +        case PIX_FMT_RGB4_BYTE:
> +        case PIX_FMT_BGR4_BYTE:
> +        case PIX_FMT_GRAY8:
> +            expand->bpp = 1;
> +            expand->x_shift[0] = 0;
> +            expand->y_shift[0] = 0;
> +            break;

Iam pretty sure theres a generic way to find the byte per pixel which
does not require a long switch-case. pix_fmt_info after all does contain
the information.

[...]
> +static void draw_slice(AVFilterLink *link, int y, int h)
> +{
> +    ExpandContext *expand = link->dst->priv;
> +    AVFilterPicRef *outpic = link->dst->outputs[0]->outpic;
> +    AVFilterPicRef *inpic = link->cur_pic;
> +    int i;
> +    int is_first = (y <= 0);
> +    int is_end = (y+h >= inpic->h);
> +
> +    for(i=0;i<4;i++) {
> +        if(outpic->data[i]) {
> +            int j;
> +            char* out_buff = outpic->data[i];
> +            const char* in_buff  = inpic->data[i];
> +            int copy_length = (inpic->w >> expand->x_shift[i]) * expand->bpp;
> +            int y_add = 1<<expand->y_shift[i];
> +            int padcolor;
> +            if(!expand->is_yuv || i == 3){ // not YUV, or alpha channel of YUVA
> +                padcolor = 0;
> +            }else{
> +                padcolor = (i == 0) ? 16 : 128;
> +            }
> +
> +            if(is_first){
> +                int size = (expand->y >> expand->y_shift[i]) * outpic->linesize[i];
> +                memset(out_buff,padcolor,size);
> +                out_buff += size;
> +            }else{
> +                int y_skip = expand->y >> expand->y_shift[i];
> +                out_buff += outpic->linesize[i] * y_skip;
> +                in_buff += inpic->linesize[i] * y_skip;
> +            }
> +
> +            for(j=0;j<h;j+=y_add){
> +                int size,total_size = 0;
> +                size = (expand->x >> expand->x_shift[i]) * expand->bpp;
> +                memset(out_buff,padcolor,size);
> +                out_buff += size;
> +                total_size += size;
> +
> +                memcpy(out_buff,in_buff,copy_length);

as with vmrsss, unconditional copying is not ok

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20080511/af86813f/attachment.pgp>