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

[Michael Niedermayer]

On Sun, May 11, 2008 at 11:02:03AM +0100, vmrsss wrote:
> PS. I have corrected a minor bug in the code (one y that should have been 
> 0). Thanks.
>
>

> /*
>  * Video pad filter
>  * copyright (c) 2008 vmrsss
>  *
>  * This file is part of FFmpeg.
>  *
>  * FFmpeg is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Lesser General Public
>  * License as published by the Free Software Foundation; either
>  * version 2.1 of the License, or (at your option) any later version.
>  *
>  * FFmpeg is distributed in the hope that it will be useful,
>  * but WITHOUT ANY WARRANTY; without even the implied warranty of
>  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>  * Lesser General Public License for more details.
>  *
>  * You should have received a copy of the GNU Lesser General Public
>  * License along with FFmpeg; if not, write to the Free Software
>  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>  */
> 
> #include <string.h>
> #include <stdio.h>
> 
> #include "avfilter.h"
> 
> typedef struct
> {

>   int  w, h, x, y;        // required size

comment is not doxygen compatible


>   double a;               // required aspect

remove all aspect related code, you dont know what you are doing.
read about pixel vs display aspect ratio


[...]
>     if( y == 0 ){ // draw top black bar
>       memset(out->data[0], padcolor[0], pad->y * out->linesize[0]);
>       memset(out->data[1], padcolor[1], (pad->y >>pad->hsub)*out->linesize[1]);
>       memset(out->data[2], padcolor[1], (pad->y >>pad->hsub)*out->linesize[2]);
>     }

indention is 4 spaces in ffmpeg


> 

>     /* copy slice (y, y+h) from the original frame */
>     for(plane = 0; plane < 3; plane++) {
> 
>       if( plane == 0 ){
> 	vsub = hsub = 0;
>       }else{
> 	vsub = pad->vsub; hsub = pad->hsub;
>       }
> 
>       j1 = ((pad->y + y) >>vsub) * out->linesize[plane]; 
>       j2 = (y >>vsub) * in->linesize[plane]; 
> 
>       for(i = 0; i < (h >>vsub); i++){
> 
> 	memset(out->data[plane] + j1, padcolor[plane], pad->x >>hsub);
> 	memcpy(out->data[plane] + j1 + (pad->x >>hsub), 
> 	       in->data[plane]  + j2, in->w >>hsub);
> 	memset(out->data[plane] + j1 + ((pad->x + in->w) >>hsub), 
> 	       padcolor[plane], k >>hsub);
> 
> 	j1 += out->linesize[plane];
> 	j2 += in->linesize[plane];
>       }
>     }

unconditional copying is unacceptable
tabs are unacceptable as well


> 
>     if( y+h >= in->h ){ // draw bottom bar
>       j1 = in->h + pad->y; 
>       j2 = out->h - j1;

>       memset(out->data[0] + j1 * out->linesize[0], 
> 	   padcolor[0], j2 * out->linesize[0]);
>       memset(out->data[1] + (j1 >>pad->vsub)*out->linesize[1], 
> 	     padcolor[1], (j2 >>pad->hsub) * out->linesize[1]);
>       memset(out->data[2] + (j1 >>pad->vsub)*out->linesize[2], 
> 	     padcolor[1], (j2 >>pad->hsub) * out->linesize[2]);

duplicate (rectangle drawing)


>     }
> 
>     j1 = ( y == 0 )? 0: y + pad->y; 
>     j2 = ( h == in->h )? out->h : h + pad->y;
>     

trailing whitespace is forbidden in svn as well

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/0c753618/attachment.pgp>