[Helix-client-dev] RESEND: Bug 11047 H263+ clips cause SIGABORT due to memory corruption

I sent to myself by mistake. Resend to  [email protected]. Thanks.
________________________________________
From: Renjie Huang
Sent: Sunday, September 26, 2010 9:04 PM
To: Renjie Huang
Subject: RESEND: Bug 11047 H263+ clips cause SIGABORT due to memory corruption

I compare the source code of FFmpeg h263 encoder/decoder with Helix decoder. In the slice structure mode of H263+, the data structure format is the same. The only difference is that: FFmpeg simply ignores the first macroblock address  in the current slice. Since the testing clips can be both played with ffmpeg and gstreamer,  I think Helix should  be able to decode them correctly too. I submit a new patch to get the helix decoder to behave in the same as FFmpeg by ignoring the first macroblock address retrieved from the slice header. With that change, Helix is now able to play custom format H263+ videos.  Can someone help me review this CR? Thanks.

Changed files:
datatype-restricted/rm/video/codec/g2mp4combo/dec/d3mvdec.cpp
datatype-restricted/rm/video/codec/g2mp4combo/common/csbsx.cpp

Index: common/c3bsx.cpp
===================================================================
RCS file: /cvsroot/rarvcode-mpeg4combo/codec/g2mp4combo/common/c3bsx.cpp,v
retrieving revision 1.5.12.2
diff -u -w -r1.5.12.2 c3bsx.cpp
--- common/c3bsx.cpp    23 Oct 2009 10:37:47 -0000  1.5.12.2
+++ common/c3bsx.cpp    27 Sep 2010 03:47:59 -0000
@@ -3788,8 +3788,9 @@
        // Get macro-block address
        *iSliceMBA = GetBits(m_mbaSize);

-       if (bFirst && *iSliceMBA)
-           bFirst = FALSE;
+       //for some videos, the start MB address of the first slice can be non-zero
+       /*if (bFirst && *iSliceMBA)
+           bFirst = FALSE;*/

        if (bFirst)
        {
Index: dec/d3mvdec.cpp
===================================================================
RCS file: /cvsroot/rarvcode-mpeg4combo/codec/g2mp4combo/dec/d3mvdec.cpp,v
retrieving revision 1.3.18.2
diff -u -w -r1.3.18.2 d3mvdec.cpp
--- dec/d3mvdec.cpp 5 Aug 2010 16:42:54 -0000   1.3.18.2
+++ dec/d3mvdec.cpp 27 Sep 2010 03:47:59 -0000
@@ -3376,19 +3376,8 @@
                m_BitCounters[BC_SLICE_HEADERS] += (m_pBitStream->GetBsOffset() - uBitOffset);
 #endif

-                   if (bFirstSlice && !iSliceMBA)
-                   {
                        m_iGQuant = m_iPQuant;
-                       // m_iPQuant == 0 has been caught before.
-                   }
-                   else
-                   {
-                       if(!iSQUANT) iSQUANT =1;
-                       m_iPQuant = m_iGQuant = iSQUANT;
-                   }
-
                    bFirstSlice = FALSE;
-                   iMBA = iSliceMBA;

                    // Set iLeft to one if this is the first MB for the slice.
                    GOBEdges.iLeft = 1;

________________________________________
From: Renjie Huang
Sent: Friday, September 24, 2010 5:19 PM
To: [email protected]
Subject: CR: Bug 11047 H263+ clips cause SIGABORT due to memory corruption

Date: 2010-09-24
Project: RealPlayer for Android Smartphones

Synopsis: H263+ clips cause SIGABORT due to memory corruption

Overview: In my test, some H263+ clips cause SIGABORT during playback. The reason is that in slice structure mode, the micro block index iMBA read from the slice header exceed the valid range and corrupt the memory.  The clips for testing are created by the SUPER video converter, which uses FFmpeg.  The comparison among Helix h263 decoder, FFmpeg h263 decoder and the ITU H263 document show that FFmpeg does not strictly follow the picture header structure format of the ITU H263 for extended type.  In FFmpeg, the SQUANT field is right after the SSS field (ituh263dec.cpp: line 1072). It also ignores the MBA value.  In ITU 263,  the fields after SSS are "ELNUM RLNUM" (page 162); the SQUANT is after fields "MBA SEPB2" (page 96). In the CR, the decoder return PIA_S_UNSUPPORTED instead of continuing when iMBA exceeds valid range.

Reference:
  ITU H263 doc: http://www.itu.int/rec/dologin_pub.asp?lang=e&id=T-REC-H.263-200501-I!!PDF-E&type=items
  FFmpeg ituh263dec.cpp: http://git.ffmpeg.org/?p=ffmpeg;a=blob;f=libavcodec/ituh263dec.c;h=8b5d9391b2e196b6767848660ecbc93497d337b8;hb=HEAD

Changed files:
datatype-restricted/rm/video/codec/g2mp4combo/dec/d3mvdec.cpp

Image Size and Heap Use impact (Client -Only):
None

Platforms and Profiles Affected:
Platform: : head, 361, and 362
Profile: helix-client-android

Distribution Libraries Affected:
NA

Distribution library impact and planned action:
NA

Platforms and Profiles Build Verified:
Platform: hxclient_3_6_1_atlas
Profile:  helix-client-android

Platforms and Profiles Functionality verified:
Platform: hxclient_3_6_1_atlas
Profile: helix-client-android


Copyright assignment: I am a RealNetworks employee or contractor

_______________________________________________
Helix-client-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/helix-client-dev

? Makefile
? a
? adecencg2mp4.mak
? adecencg2mp4.upp
? adecg2mp4.mak
? adecg2mp4.upp
? android-dbg
? cdecg2mp4.mak
? cdecg2mp4.upp
? dbg
? dmp4lib.mak
? dmp4lib.upp
? h263-new.diff
? h263.diff
? mp4vlib.mak
? mp4vlib.upp
? rel
? ribosome_logs
? umakefil.upp
Index: common/c3bsx.cpp
===================================================================
RCS file: /cvsroot/rarvcode-mpeg4combo/codec/g2mp4combo/common/c3bsx.cpp,v
retrieving revision 1.5.12.2
diff -u -w -r1.5.12.2 c3bsx.cpp
--- common/c3bsx.cpp	23 Oct 2009 10:37:47 -0000	1.5.12.2
+++ common/c3bsx.cpp	27 Sep 2010 03:47:59 -0000
@@ -3788,8 +3788,9 @@
 		// Get macro-block address
 		*iSliceMBA = GetBits(m_mbaSize);
 
-		if (bFirst && *iSliceMBA)
-			bFirst = FALSE;
+		//for some videos, the start MB address of the first slice can be non-zero
+		/*if (bFirst && *iSliceMBA)
+			bFirst = FALSE;*/
 
 		if (bFirst)
 		{
Index: dec/d3mvdec.cpp
===================================================================
RCS file: /cvsroot/rarvcode-mpeg4combo/codec/g2mp4combo/dec/d3mvdec.cpp,v
retrieving revision 1.3.18.2
diff -u -w -r1.3.18.2 d3mvdec.cpp
--- dec/d3mvdec.cpp	5 Aug 2010 16:42:54 -0000	1.3.18.2
+++ dec/d3mvdec.cpp	27 Sep 2010 03:47:59 -0000
@@ -3376,19 +3376,8 @@
 				m_BitCounters[BC_SLICE_HEADERS] += (m_pBitStream->GetBsOffset() - uBitOffset);
 #endif
 				
-					if (bFirstSlice && !iSliceMBA)
-					{
 						m_iGQuant = m_iPQuant;
-						// m_iPQuant == 0 has been caught before.
-					}
-					else
-					{
-						if(!iSQUANT) iSQUANT =1;
-						m_iPQuant = m_iGQuant = iSQUANT;						
-					}
-
 					bFirstSlice	= FALSE;
-					iMBA = iSliceMBA;
 
 					// Set iLeft to one if this is the first MB for the slice.
 					GOBEdges.iLeft = 1;