RE: [Fwd: RE: Player buffers at 100% for h263+amr streams]

HEAD change looks good to me.

=======================================
Eric Hyche ([email protected])
Principal Engineer
RealNetworks, Inc.


>-----Original Message-----
>From: Sujeet Kharkar [mailto:[email protected]]
>Sent: Tuesday, April 14, 2009 6:57 PM
>To: [email protected]; [email protected]; [email protected]
>Subject: RE: [Fwd: RE: [Helix-client-dev] Player buffers at 100% for h263+amr streams]
>
>
>Here is diff for HEAD.
>Added check for multirate case and handled rollover case as suggested by
>Eric.
>
>Following case is not handled by this fix.
>If Audio key frame is received earlier in producer and has timestamp < then
>video keyframe and if both of them reach server at same time, there is a
>chance that server would keep in RSD buffer video key frame followed by
>audio keyframe of < time and this issue will repro for all realplayer with
>RTP client connecting during that RSD Duration.
>
>Chances of this occurring are minimal. Ideally would like to confirm that
>this is what happening in 2% failure case, which QA is seeing.
>
>
>
>Index: rbsfilter.cpp
>===================================================================
>RCS file:
>/cvsroot/client/encodesvc/plugins/output/rbsbroadcast/rbsfilter.cpp,v
>retrieving revision 1.5
>diff -u -r1.5 rbsfilter.cpp
>--- rbsfilter.cpp	12 Mar 2009 23:39:49 -0000	1.5
>+++ rbsfilter.cpp	14 Apr 2009 16:29:04 -0000
>@@ -140,6 +140,11 @@
> , m_pszStreamName( NULL )
> , m_eBroadcastType(	RBS_INVALID )
> , m_pRBSActor(NULL)
>+, m_ulLastKeyFrameTime(0)
>+, m_bVideoKeyFrameReceived(FALSE)
>+, m_bAdjustAudioPacketFlags(FALSE)
>+, m_ulVideoStreamId(MAX_UINT32)
>+, m_ulAudioStreamId(MAX_UINT32)
> {
> }
>
>@@ -351,6 +356,70 @@
> 	    {
> 		HXTLOG_APPROVED(LC_DEV_ERROR,BCAST,"ASM header	passed to
>rbs filter does not contain a	file header");
> 	    }
>+
>+	    if(SUCCEEDED(res))
>+	    {
>+		m_bAdjustAudioPacketFlags = FALSE;
>+		UINT32 bIsRealDataType = 0;
>+	        res = spIFileHeader->GetPropertyULONG32("IsRealDataType",
>bIsRealDataType);
>+		if (SUCCEEDED(res))
>+		{
>+		    if (!bIsRealDataType && m_ulNumStreams > 1)
>+		    {
>+			m_bAdjustAudioPacketFlags = TRUE;
>+		    }
>+		}
>+
>+		if (SUCCEEDED(res) && m_bAdjustAudioPacketFlags)
>+		{
>+		    SPIHXValues spIStreamHeader;
>+		    //Find video stream.
>+		    SPIHXBuffer spMimeType;
>+		    HXBOOL bVideoStreamIdFound = FALSE;
>+		    HXBOOL bAudioStreamIdFound = FALSE;
>+		    for (UINT32 i=0; SUCCEEDED(res) && i<m_ulNumStreams;
>i++)
>+		    {
>+			res =
>m_spConnectingAsmHeaderSource->GetStreamHeader(i, spIStreamHeader.Adopt());
>+			if(SUCCEEDED(res))
>+			{
>+			    res =
>spIStreamHeader->GetPropertyCString("MimeType", *spMimeType.Adopt());
>+			}
>+
>+			// Determine mime type
>+			if (SUCCEEDED(res) && strncasecmp((const
>char*)spMimeType->GetBuffer(), "video", 5) == 0)
>+			{
>+			    if (bVideoStreamIdFound)
>+			    {
>+				//There is more then one video streami.e.
>this a multi-rate 3GPP stream.
>+				// In this case, we do not need to adjust
>Audio sample flags as HUS server which supports multi-rate has fixed this.
>+				m_bAdjustAudioPacketFlags = FALSE;
>+			    }
>+			    m_ulVideoStreamId = i;
>+			    bVideoStreamIdFound = TRUE;
>+			}
>+			if (SUCCEEDED(res) && strncasecmp((const
>char*)spMimeType->GetBuffer(), "audio", 5) == 0)
>+			{
>+			    if (bAudioStreamIdFound)
>+			    {
>+				//There is more then one audio stream i.e.
>this a multi-rate 3GPP stream
>+				// In this case, we do not need to adjust
>Audio sample flags as HUS server which supports multi-rate has fixed this.
>+				m_bAdjustAudioPacketFlags = FALSE;
>+			    }
>+
>+			    m_ulAudioStreamId = i;
>+			    bAudioStreamIdFound = TRUE;
>+			}
>+
>+			spIStreamHeader = NULL;
>+			spMimeType = NULL;
>+		    }
>+
>+		    if (bVideoStreamIdFound == FALSE)
>+		    {
>+			m_bAdjustAudioPacketFlags = FALSE;
>+		    }
>+		}
>+	    }
> 	}
>     }
>     else
>@@ -899,6 +968,9 @@
> 	return res;
>     }
>
>+    m_ulLastKeyFrameTime = 0;
>+    m_bVideoKeyFrameReceived = FALSE;
>+
>     if(!m_bIsInputPropertiesSet || !m_pszStreamName || ( m_eBroadcastType
>== RBS_INVALID) ||
> 	(m_pszStreamName && !(*m_pszStreamName)))
>     {
>@@ -1227,6 +1299,37 @@
> 		res = pIInSample->GetSampleField(
>HXT_FIELD_LOGICAL_STREAM_ID, &ulStreamId );
> 	    }
>
>+	    if (m_bAdjustAudioPacketFlags)
>+	    {
>+		//If realplayer 11 receives a RTP packet with RTP time less
>then RTP Info time
>+		//as reported by Helix Server then it stays at 100% loading
>and never plays stream.
>+
>+		//Fix/Work around is to remove HX_ASM_SWITCH_ON Flag for
>audio packets which have timestamps less then previous
>+		//video keyframe packet.
>+		//This will for most cases ensure that firt keyframe audio
>packet in Server's RSD Queue will have timestamp > Fisrt Video Keyframe
>timestamp.
>+                if (ulStreamId == m_ulVideoStreamId &&
>+		    (ulAsmFlags & HX_ASM_SWITCH_ON) && !(ulAsmFlags &
>HX_ASM_SIDE_EFFECT))
>+		{
>+		    //First video packet of a keyframe.
>+		    m_ulLastKeyFrameTime = ulTime;
>+		    m_bVideoKeyFrameReceived = TRUE;
>+		}
>+		else if (ulStreamId == m_ulAudioStreamId &&
>m_bVideoKeyFrameReceived)
>+		{
>+		    // audio packet
>+		    INT32 iDiffInTime = ulTime - m_ulLastKeyFrameTime;
>+		    if (iDiffInTime <= 0)
>+		    {
>+			//Remove HX_ASM_SWITCH_ON flag.
>+			ulAsmFlags = ulAsmFlags & ~HX_ASM_SWITCH_ON;
>+		    }
>+		    else
>+		    {
>+			m_bVideoKeyFrameReceived = FALSE;
>+		    }
>+		}
>+	    }
>+
> 	    if(SUCCEEDED(res))
> 	    {
> 		res = spIPacket->Set(spIData, ulTime, (UINT16)ulStreamId,
>(UINT8)ulAsmFlags, (UINT16)ulAsmRuleNum);
>Index: rbsfilter.h
>===================================================================
>RCS file:
>/cvsroot/client/encodesvc/plugins/output/rbsbroadcast/rbsfilter.h,v
>retrieving revision 1.3
>diff -u -r1.3 rbsfilter.h
>--- rbsfilter.h	20 Aug 2008 21:08:30 -0000	1.3
>+++ rbsfilter.h	14 Apr 2009 16:29:04 -0000
>@@ -192,6 +192,12 @@
>     BOOL* m_pbStreamDone;
>     CHAR* m_pszStreamName;
>     CEventResend m_eventresend;
>+
>+    UINT32 m_ulLastKeyFrameTime;
>+    HXBOOL m_bVideoKeyFrameReceived;
>+    HXBOOL m_bAdjustAudioPacketFlags;
>+    UINT32 m_ulVideoStreamId;
>+    UINT32 m_ulAudioStreamId;
> };
>
>
>
>Sujeet,
>
>For PRODUCER_12_5_RN branch, diff looks fine. However, for HEAD going
>forward, I have a couple of comments/suggestions:
>
>1) This change assumes one audio stream and one video stream. Remember,
>for multi-rate 3GP, there will be multiple audio stream
>headers and multiple video stream headers. So instead of finding one audio
>stream and finding one video stream, we need to identify
>the proper *pairs* of audio and video streams.
>
>You know, the more I think about it, this fix may not even be applicable
>for the multi-rate case, since we're not really guaranteed
>of the exact pairing of video and audio streams that will be sent to the
>client. For multi-rate we may have to come up with a more
>elaborate fix or simply require Helix servers that contain the server fix
>for this.
>
>However, I think we should still probably check this fix into HEAD. We may
>have to re-visit it later to make it more robust.
>
>2) +               BOOL bVideoStreamIdFound = FALSE;
>
>   HXBOOL instead of BOOL
>
>3) +               if (ulTime <= m_ulLastKeyFrameTime)
>
>Instead of doing a straight <= comparison, you should subtract ulTime -
>m_ulLastKeyFrameTime
>and compare against 0. The reason is that the straight <= comparison will
>fail under
>32-bit rollover while the ulTime - m_ulLastKeyFrameTime will still work.
>
>Rest looks good.
>
>Eric
>
>=======================================
>Eric Hyche ([email protected])
>Principal Engineer
>RealNetworks, Inc.
>
>>-----Original Message-----
>>From: [email protected]
>[mailto:[email protected]]
>>On Behalf Of Sujeet Kharkar
>>Sent: Friday, April 10, 2009 5:49 PM
>>To: [email protected]; [email protected]
>>Subject: [Helix-client-dev] Player buffers at 100% for h263+amr streams
>>
>>Synopsis
>>
>>============
>>
>>Real Player hangs at 100% loading when streaming audio + video.
>>
>>
>>
>>Suggested Reviewer: Eric Hyche
>>
>>
>>
>>Branches
>>
>>=========
>>
>>Head, PRODUCER_12_5(already checked in due to urgency of issue.)
>>
>>
>>
>>Description
>>
>>==============
>>
>>Cause:
>>
>>If realplayer 11 receives a RTP packet with RTP time less then RTP Info
>time reported by HUS in play
>>response
>>
>>then it stays at 100% loading and never plays stream.
>>
>>
>>
>>Fix
>>
>>=======
>>
>>Fix/Work around is to remove HX_ASM_SWITCH_ON Flag for audio packets
>which have timestamps less then
>>previous
>>
>>video keyframe packet.
>>
>>
>>
>>Mulirate case was addressed in code as multirate 3GP live is only there
>for 12.1, which should not
>>require this fix.
>>
>>There might be requirement to turn this off if server version is 12.1 or
>later.
>>
>>
>>
>>
>>
>>Impact:
>>
>>=======
>>
>>This fix will only affect case of 3GP streaming to HUS. Will not impact
>rm to file and/or server and
>>3gp to file cases.
>>
>>
>>
>>Files affected
>>
>>==================
>>
>>client/encodesvc/plugins/output/rbsbroadcast/rbsfilter.h,
>>
>>client/encodesvc/plugins/output/rbsbroadcast/rbsfilter.cpp
>>
>>
>>
>>Testing
>>
>>=================
>>
>>Tested locally and on QA machine with job to be used by SMART.
>>
>>
>>
>>QA Hints
>>
>>=============
>>
>>Please test with mobile and desktop RealPlayer 11.
>>
>>Please test FCS case with HUS 12.0.1 and SSPL with HUS 12.1
>>
>>
>>
>>
>>
>>Index: rbsfilter.cpp
>>
>>===================================================================
>>
>>RCS file:
>/cvsroot/client/encodesvc/plugins/output/rbsbroadcast/rbsfilter.cpp,v
>>
>>retrieving revision 1.5
>>
>>retrieving revision 1.5.4.1
>>
>>diff -u -r1.5 -r1.5.4.1
>>
>>--- rbsfilter.cpp 12 Mar 2009 23:39:49 -0000    1.5
>>
>>+++ rbsfilter.cpp 10 Apr 2009 05:15:17 -0000    1.5.4.1
>>
>>@@ -140,6 +140,11 @@
>>
>> , m_pszStreamName( NULL )
>>
>> , m_eBroadcastType(    RBS_INVALID )
>>
>> , m_pRBSActor(NULL)
>>
>>+, m_ulLastKeyFrameTime(0)
>>
>>+, m_bVideoKeyFrameReceived(FALSE)
>>
>>+, m_bAdjustAudioPacketFlags(FALSE)
>>
>>+, m_ulVideoStreamId(MAX_UINT32)
>>
>>+, m_ulAudioStreamId(MAX_UINT32)
>>
>> {
>>
>> }
>>
>>
>>
>>@@ -351,6 +356,55 @@
>>
>>          {
>>
>>            HXTLOG_APPROVED(LC_DEV_ERROR,BCAST,"ASM header  passed to rbs
>filter does not contain a
>>file header");
>>
>>          }
>>
>>+
>>
>>+         if(SUCCEEDED(res))
>>
>>+         {
>>
>>+           m_bAdjustAudioPacketFlags = FALSE;
>>
>>+           UINT32 bIsRealDataType = 0;
>>
>>+             res = spIFileHeader->GetPropertyULONG32("IsRealDataType",
>bIsRealDataType);
>>
>>+           if (SUCCEEDED(res))
>>
>>+           {
>>
>>+               if (!bIsRealDataType && m_ulNumStreams > 1)
>>
>>+               {
>>
>>+                 m_bAdjustAudioPacketFlags = TRUE;
>>
>>+               }
>>
>>+           }
>>
>>+
>>
>>+           if (SUCCEEDED(res) && m_bAdjustAudioPacketFlags)
>>
>>+           {
>>
>>+               SPIHXValues spIStreamHeader;
>>
>>+               //Find video stream.
>>
>>+               SPIHXBuffer spMimeType;
>>
>>+               BOOL bVideoStreamIdFound = FALSE;
>>
>>+               for (UINT32 i=0; SUCCEEDED(res) && i<m_ulNumStreams; i++)
>>
>>+               {
>>
>>+                 res = m_spConnectingAsmHeaderSource->GetStreamHeader(i,
>spIStreamHeader.Adopt());
>>
>>+                 if(SUCCEEDED(res))
>>
>>+                 {
>>
>>+                     res =
>spIStreamHeader->GetPropertyCString("MimeType", *spMimeType.Adopt());
>>
>>+                 }
>>
>>+
>>
>>+                 // Determine mime type
>>
>>+                 if (SUCCEEDED(res) && strncasecmp((const
>char*)spMimeType->GetBuffer(), "video", 5)
>>== 0)
>>
>>+                 {
>>
>>+                     m_ulVideoStreamId = i;
>>
>>+                     bVideoStreamIdFound = TRUE;
>>
>>+                 }
>>
>>+                 if (SUCCEEDED(res) && strncasecmp((const
>char*)spMimeType->GetBuffer(), "audio", 5)
>>== 0)
>>
>>+                 {
>>
>>+                     m_ulAudioStreamId = i;
>>
>>+                 }
>>
>>+
>>
>>+                 spIStreamHeader = NULL;
>>
>>+                 spMimeType = NULL;
>>
>>+               }
>>
>>+
>>
>>+               if (bVideoStreamIdFound == FALSE)
>>
>>+               {
>>
>>+                 m_bAdjustAudioPacketFlags = FALSE;
>>
>>+               }
>>
>>+           }
>>
>>+         }
>>
>>      }
>>
>>     }
>>
>>     else
>>
>>@@ -899,6 +953,9 @@
>>
>>      return res;
>>
>>     }
>>
>>
>>
>>+    m_ulLastKeyFrameTime = 0;
>>
>>+    m_bVideoKeyFrameReceived = FALSE;
>>
>>+
>>
>>     if(!m_bIsInputPropertiesSet || !m_pszStreamName || (
>m_eBroadcastType == RBS_INVALID) ||
>>
>>      (m_pszStreamName && !(*m_pszStreamName)))
>>
>>     {
>>
>>@@ -1227,6 +1284,36 @@
>>
>>            res = pIInSample->GetSampleField(
>HXT_FIELD_LOGICAL_STREAM_ID, &ulStreamId );
>>
>>          }
>>
>>
>>
>>+         if (m_bAdjustAudioPacketFlags)
>>
>>+         {
>>
>>+           //If realplayer 11 receives a RTP packet with RTP time less
>then RTP Info time
>>
>>+           //as reported by Helix Server then it stays at 100% loading
>and never plays stream.
>>
>>+
>>
>>+           //Fix/Work around is to remove HX_ASM_SWITCH_ON Flag for
>audio packets which have
>>timestamps less then previous
>>
>>+           //video keyframe packet.
>>
>>+           //This will for most cases ensure that firt keyframe audio
>packet in Server's RSD Queue
>>will have timestamp > Fisrt Video Keyframe timestamp.
>>
>>+                if (ulStreamId == m_ulVideoStreamId &&
>>
>>+               (ulAsmFlags & HX_ASM_SWITCH_ON) && !(ulAsmFlags &
>HX_ASM_SIDE_EFFECT))
>>
>>+           {
>>
>>+               //First video packet of a keyframe.
>>
>>+               m_ulLastKeyFrameTime = ulTime;
>>
>>+               m_bVideoKeyFrameReceived = TRUE;
>>
>>+           }
>>
>>+           else if (ulStreamId == m_ulAudioStreamId &&
>m_bVideoKeyFrameReceived)
>>
>>+           {
>>
>>+               // audio packet
>>
>>+               if (ulTime <= m_ulLastKeyFrameTime)
>>
>>+               {
>>
>>+                 //Remove HX_ASM_SWITCH_ON flag.
>>
>>+                 ulAsmFlags = ulAsmFlags & ~HX_ASM_SWITCH_ON;
>>
>>+               }
>>
>>+               else
>>
>>+               {
>>
>>+                 m_bVideoKeyFrameReceived = FALSE;
>>
>>+               }
>>
>>+           }
>>
>>+         }
>>
>>+
>>
>>          if(SUCCEEDED(res))
>>
>>          {
>>
>>            res = spIPacket->Set(spIData, ulTime, (UINT16)ulStreamId,
>(UINT8)ulAsmFlags,
>>(UINT16)ulAsmRuleNum);
>>
>>Index: rbsfilter.h
>>
>>===================================================================
>>
>>RCS file:
>/cvsroot/client/encodesvc/plugins/output/rbsbroadcast/rbsfilter.h,v
>>
>>retrieving revision 1.3
>>
>>retrieving revision 1.3.4.1
>>
>>diff -u -r1.3 -r1.3.4.1
>>
>>--- rbsfilter.h   20 Aug 2008 21:08:30 -0000    1.3
>>
>>+++ rbsfilter.h   10 Apr 2009 05:15:17 -0000    1.3.4.1
>>
>>@@ -192,6 +192,12 @@
>>
>>     BOOL* m_pbStreamDone;
>>
>>     CHAR* m_pszStreamName;
>>
>>     CEventResend m_eventresend;
>>
>>+
>>
>>+    UINT32 m_ulLastKeyFrameTime;
>>
>>+    HXBOOL m_bVideoKeyFrameReceived;
>>
>>+    HXBOOL m_bAdjustAudioPacketFlags;
>>
>>+    UINT32 m_ulVideoStreamId;
>>
>>+    UINT32 m_ulAudioStreamId;
>>
>> };
>>
>>
>>
>>
>>
>>
>>
>>
>
>