[MPlayer-dev-eng] patch for bmovl

[Nicolas George]

Le quartidi 24 nivôse, an CCXX, Emmanuel Anne a écrit :
> Ok, here is finally a patch for the problem I posted 1 week ago.

Thanks for the patch, and sorry for the delay.

> Finally it's not related to fps, but rather to the size of the video : if
> the video buffer is aligned its size changes while the bmovl buffer
> doesn't, which produces a nice crash. Fixed by passing the right flag to
> vf_get_image
> 
> Then if you play a video with a broken size (like 540x396), the application
> communicating via bmovl will send a buffer for a 540 pixels wide buffer and
> it will be totally distorted because the actual width here is 544 for
> alignement reasons. So this patch now makes the difference between the
> width of the source bitmap and the one of the destination bitmap instead of
> assuming they are always identical.
> 
> So far all the videos I could test pass without problem (tested on 2
> computers !).

> Index: libmpcodecs/vf_bmovl.c
> ===================================================================
> --- libmpcodecs/vf_bmovl.c	(r?vision 34553)
> +++ libmpcodecs/vf_bmovl.c	(copie de travail)
> @@ -221,11 +221,13 @@
>  	unsigned char red=0, green=0, blue=0;
>  	int  alpha;
>  	mp_image_t* dmpi;
> +	static int halfw;

Static local variables are rarely a good idea (except const ones). This
would break if two instances of the filter are inserted.

>  
> +	/* get dmpi aligned as mpi */
>      dmpi = vf_get_image(vf->next, mpi->imgfmt, MP_IMGTYPE_TEMP,
> -						MP_IMGFLAG_ACCEPT_STRIDE | MP_IMGFLAG_PREFER_ALIGNED_STRIDE,
> -						mpi->w, mpi->h);
> -
> +						MP_IMGFLAG_ACCEPT_ALIGNED_STRIDE  | MP_IMGFLAG_PREFER_ALIGNED_STRIDE ,
> +						mpi->w, mpi->height);

Changing w into width too seems more consistent.

> +  
>      memcpy_pic( dmpi->planes[0], mpi->planes[0], mpi->width, mpi->height, dmpi->stride[0], mpi->stride[0] );
>      memcpy_pic( dmpi->planes[1], mpi->planes[1], mpi->chroma_width, mpi->chroma_height, dmpi->stride[1], mpi->stride[1] );
>      memcpy_pic( dmpi->planes[2], mpi->planes[2], mpi->chroma_width, mpi->chroma_height, dmpi->stride[2], mpi->stride[2] );
> @@ -346,6 +348,7 @@
>  				}
>  				return vf_next_put_image(vf, dmpi, MP_NOPTS_VALUE);
>  			}
> +			halfw = vf->priv->w/2;
>  
>  			for( buf_y=0 ; (buf_y < imgh) && (buf_y < (vf->priv->h-imgy)) ; buf_y++ ) {
>  			    for( buf_x=0 ; (buf_x < (imgw*pxsz)) && (buf_x < ((vf->priv->w+imgx)*pxsz)) ; buf_x += pxsz ) {
> @@ -389,7 +392,7 @@
>  						vf->priv->bitmap.oa[pos] = alpha;
>  						vf->priv->bitmap.a[pos]  = INRANGE((alpha+imgalpha),0,255);
>  						if((buf_y%2) && ((buf_x/pxsz)%2)) {
> -							pos = ( ((buf_y+imgy)/2) * dmpi->stride[1] ) + (((buf_x/pxsz)+imgx)/2);
> +							pos = ( ((buf_y+imgy)/2) * halfw ) + (((buf_x/pxsz)+imgx)/2);
>  							vf->priv->bitmap.u[pos] = rgb2u(red,green,blue);
>  							vf->priv->bitmap.v[pos] = rgb2v(red,green,blue);
>  						}
> @@ -421,34 +424,37 @@
>  	} else { // Blit the bitmap to the videoscreen, pixel for pixel
>  	    for( ypos=vf->priv->y1 ; ypos < vf->priv->y2 ; ypos++ ) {
>  	        for ( xpos=vf->priv->x1 ; xpos < vf->priv->x2 ; xpos++ ) {
> +				int pos0 = (ypos * vf->priv->w) + xpos;
>  				pos = (ypos * dmpi->stride[0]) + xpos;
>  
> -				alpha = vf->priv->bitmap.a[pos];
> +				alpha = vf->priv->bitmap.a[pos0];
>  
>  				if (alpha == 0) continue; // Completly transparent pixel
>  
>  				if (alpha == 255) {	// Opaque pixel
> -					dmpi->planes[0][pos] = vf->priv->bitmap.y[pos];
> +					dmpi->planes[0][pos] = vf->priv->bitmap.y[pos0];
>  					if ((ypos%2) && (xpos%2)) {
>  						pos = ( (ypos/2) * dmpi->stride[1] ) + (xpos/2);
> -						dmpi->planes[1][pos] = vf->priv->bitmap.u[pos];
> -						dmpi->planes[2][pos] = vf->priv->bitmap.v[pos];
> +						pos0 = ( (ypos/2) * halfw ) + (xpos/2);
> +						dmpi->planes[1][pos] = vf->priv->bitmap.u[pos0];
> +						dmpi->planes[2][pos] = vf->priv->bitmap.v[pos0];
>  					}
>  				} else { // Alphablended pixel
>  					dmpi->planes[0][pos] =
>  						((255 - alpha) * (int)dmpi->planes[0][pos] +
> -						alpha * (int)vf->priv->bitmap.y[pos]) >> 8;
> +						alpha * (int)vf->priv->bitmap.y[pos0]) >> 8;
>  
>  					if ((ypos%2) && (xpos%2)) {
>  						pos = ( (ypos/2) * dmpi->stride[1] ) + (xpos/2);
> +						pos0 = ( (ypos/2) * halfw ) + (xpos/2);
>  
>  						dmpi->planes[1][pos] =
>  							((255 - alpha) * (int)dmpi->planes[1][pos] +
> -							alpha * (int)vf->priv->bitmap.u[pos]) >> 8;
> +							alpha * (int)vf->priv->bitmap.u[pos0]) >> 8;
>  
>  						dmpi->planes[2][pos] =
>  							((255 - alpha) * (int)dmpi->planes[2][pos] +
> -							alpha * (int)vf->priv->bitmap.v[pos]) >> 8;
> +							alpha * (int)vf->priv->bitmap.v[pos0]) >> 8;
>  					}
>  			    }
>  			} // for xpos
> Index: sub/sub.c
> ===================================================================
> --- sub/sub.c	(r?vision 34553)
> +++ sub/sub.c	(copie de travail)
> @@ -157,8 +157,8 @@
>  	obj->allocated = len;
>  	free(obj->bitmap_buffer);
>  	free(obj->alpha_buffer);
> -	obj->bitmap_buffer = memalign(16, len);
> -	obj->alpha_buffer  = memalign(16, len);
> +	obj->bitmap_buffer = (len ? memalign(16, len) : NULL);
> +	obj->alpha_buffer  = (len ? memalign(16, len) : NULL);
>      }
>      memset(obj->bitmap_buffer, sub_bg_color, len);
>      memset(obj->alpha_buffer, sub_bg_alpha, len);

This part seems unrelated.

I have the attached commit in my work tree that I have tested, I will commit
it soon unless someone has remarks.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20120307/e9d7f4aa/attachment.asc>