[MPlayer-cvslog] CVS: main/libmpcodecs vd_libmpeg2.c,1.37,1.38

[Jindrich Makovicka]

Ivan Kalvachev wrote:
> On Mon, 22 Nov 2004 16:32:10 +0100 (CET)
> syncmail at mplayerhq.hu (Jindrich Makovicka CVS) wrote:
> 
> 
>>CVS change done by Jindrich Makovicka CVS
>>
>>Update of /cvsroot/mplayer/main/libmpcodecs
>>In directory mail:/var2/tmp/cvs-serv20667
>>
>>Modified Files:
>>	vd_libmpeg2.c 
>>Log Message:
>>fix segfault with (height|width)%6!=0
>>
>>Index: vd_libmpeg2.c
>>===================================================================
>>RCS file: /cvsroot/mplayer/main/libmpcodecs/vd_libmpeg2.c,v
>>retrieving revision 1.37
>>retrieving revision 1.38
>>diff -u -r1.37 -r1.38
>>--- vd_libmpeg2.c	22 Nov 2004 10:20:47 -0000	1.37
>>+++ vd_libmpeg2.c	22 Nov 2004 15:32:08 -0000	1.38
>>@@ -157,13 +157,13 @@
>> 	    if (info->sequence->width >> 1 == info->sequence->chroma_width &&
>> 		info->sequence->height >> 1 == info->sequence->chroma_height) {
>> 		if(!mpcodecs_config_vo(sh,
>>-				       info->sequence->width,
>>-				       info->sequence->height, IMGFMT_YV12)) return 0;
>>+				       info->sequence->picture_width,
>>+				       info->sequence->picture_height, IMGFMT_YV12)) return 0;
>> 	    } else if (info->sequence->width >> 1 == info->sequence->chroma_width &&
>> 		info->sequence->height == info->sequence->chroma_height) {
>> 		if(!mpcodecs_config_vo(sh,
>>-				       info->sequence->width,
>>-				       info->sequence->height, IMGFMT_422P)) return 0;
>>+				       info->sequence->picture_width,
>>+				       info->sequence->picture_height, IMGFMT_422P)) return 0;
>> 	    } else return 0;
>> 	    break;
>> 	case STATE_PICTURE:
>>@@ -186,8 +186,8 @@
>> 	    mpi_new=mpcodecs_get_image(sh,MP_IMGTYPE_IPB,
>> 				       (type==PIC_FLAG_CODING_TYPE_B) ?
>> 					use_callback : (MP_IMGFLAG_PRESERVE|MP_IMGFLAG_READABLE),
>>-				       (info->sequence->picture_width+15)&(~15),
>>-				       (info->sequence->picture_height+15)&(~15) );
>>+				       info->sequence->picture_width,
>>+				       info->sequence->picture_height );
> 
> 
> The above change does not make sense for me. Are you aware that the video system may actually
> return buffer that is not aligned to 16? This will segfault, as mpeg always write full macro blocks.
> I see that you are aware that sequence->height is already aligned to 16. While picture_height is
> raw mpeg stream value (visible dimension)

I must admit I am confused here, because the original with aligned 
dimensions crashes exactly in the case when the dimensions are not 
multiples of 16. For example the following 1920x1080 video playes using 
-vo x11 crashes (xv doesn't work for me due to the video size).

http://mplayerhq.hu/~diego/libmpeg2bug.ts

The crash occurs at the end of playback, byt valgrind reports write 
access error the YUV->RGB conversion right after line 1080. This occurs 
with the original code with dims aligned to 16. The current code works 
flawlessly.

> BTW something I forgot to mention in my previous mail. libmpeg2 already takes care of chroma_height,
> why are you messing with it?

This is the 422 vs 420 detection taken from original mpeg2dec code.

Regards,

-- 
Jindrich Makovicka