RE: RESEND: CR: Release 6 Rate Adaptation bug fix

Junhong.Liunokia.com wrote:
> Hi, Greg:
> 
> Yes, this fix  covers both Helix and 3GPP RA since both
of them are
> using the free buffer size as the feedback. I will make
update as you
> suggested.

Ok, please also check this into 310Atlas.

--greg.

> 
> Thanks!
> 
> Junhong 
> 
>> -----Original Message-----
>> From: ext Greg Wright [mailto:gwrightreal.com]

>> Sent: Tuesday, November 13, 2007 2:51 PM
>> To: Liu Junhong (Nokia-TP-MSW/Dallas)
>> Cc: protocol-devhelixcommunity.org;
client-devhelixcommunity.org
>> Subject: Re: RESEND: CR: Release 6 Rate Adaptation
bug fix
>>
>> Does this patch fix both Helix and 3GPP RA?
>>
>> More below.....
>>
>>
>> Junhong.Liunokia.com wrote:
>>> "Nokia submits this code under the terms
of a commercial 
>> contribution 
>>> agreement with RealNetworks, and I am
authorized to contribute this 
>>> code under said agreement."
>>>
>>> Modified by: Junhong.Liunokia.com
>>>
>>> Reviewed by: 
>>>
>>> Date: 08-13-2007
>>>
>>> Error ID: TVAA-6WDGBE
>>>
>>> Project: Helix plugin for Symbian
>>>
>>> Synopsis:  Release 6 Rate Adaptation bug fix
>>>
>>> We are experiencing streaming problems with
helix 11 server 
>> when 3GPP 
>>> rata adaptation feature is turned on. Here is
the situation.
>>>
>>> There is a clip on helix 11 server, the video
bit rate is 60 
>> kpbs, and 
>>> the audio bit rate is 20 kpb. The clip duration
is 90 seconds and 
>>> preroll is 1 second.
>>>
>>> The data flow in helix client for streaming
case is: server
>>> ->TransportBuffer.m_pPacketDeque
->HXPlayer.m_EventList -> renderer
>>>
>>> The steps of 3GPP Rel6 RA are: 
>>>
>>> 1. Feedback_buffer_control reads
"TransportByteLimit" from the .cfg 
>>> file and set audio and video's TransportBuffer
byte limit, 
>>> proportional to their bit rates. Here, audio's
TransportBuffer byte  
>>> is 100k, and video is 200k.
>>>
>>>
>>> 2. Rate_adaptation_info calculates the 3GPP
RA's 
>> max_buffer_size (used 
>>> by SETUP message) as TransportByteLimit (300k)
+ proroll *  
>> bit rate / 
>>> 8.
>>> The results are around 300k for both video and
audio streams 
>> since the 
>>> preroll is very small.
>>>
>>>
>>> 3. Helix sends RTCP APP (PSS0) feedback
including the free 
>> buffer size
>>> (FBS) periodically. 
>>>
>>> The calculation of FBS is
>>>
>>> FBS = max_buffer_size  - sizeof (
TransportBuffer.m_pPacketDeque + 
>>> HXPlayer.m_EventList + renderer ).
>>>
>>>
>>> When the TransportBuffer reaches its byte
limit, it starts 
>> to drop the 
>>> packet before moving to HXPlayer.m_EventList.
For audio 
>> stream,  when 
>>> TansfortBuff is full
(TransportBuffer.m_pPacketDeque  = 100k),  the 
>>> calculation of its FBS is :
>>>
>>> FBS = 300k - ( 100k + HXPlayer.m_EventList +
renderer ) = 200k - 
>>> (HXPlayer.m_EventList + renderer) which is
still around 200k since 
>>> "HXPlayer.m_EventList + renderer" is
usually small.
>>>
>>> So helix client keeps notifying the server that
it has around 200k 
>>> available buffer, and the server will not
slowdown it's sending, 
>>> resulting in more packets dropped in
TransportBuffer. We have seen 
>>> that after playing for a while, the video/audio
quality was 
>> downgraded 
>>> significantly.
>>>
>>> The root cause of this problem is the byte
limit of 
>> TransportBuffer is 
>>> smaller than the max_buffer_size. In fact, the
problem also 
>> happens on 
>>> Helix adaptation which also uses the free
buffer size as it's RDT 
>>> feedback too.
>>>
>>>
>>> The solution is to let Rate_adaptation_info
calculates and sets 
>>> TransportByteLimit same as max_buffer_size and
disables 
>>> Feedback_buffer_control setting on
TransportByteLimit when 
>> the server 
>>> side rate adapation is on.
>>>
>>>
>>>
>>>
>>> Root Cause of the problem: Implementation.
>>>
>>> Files Modified:
>>>
>>> client/core/fbbufctl.h
>>> client/core/fbbufctl.cpp
>>> protocol/rtsp/rateadaptinfo.cpp
>>>
>>> Image Size and Heap Use impact: No.
>>>
>>> Module Release testing (STIF): All local cases
pass on Rosetta.
>>>
>>> Test case(s) Added:  No. 
>>>
>>> Memory leak check performed: Yes. No memory
leaks are introduced by 
>>> this CR.
>>>
>>> Platforms and Profiles Build Verified:
helix-client-s60-mmf-mdf-arm
>>>
>>> Platforms and Profiles Functionality Verified:
armv5, winscw
>>>
>>> Branch: 
>>>
>>> HEAD and Hxclient_2_1_0_cayennes
>>>
>>>
>>> Index: rateadaptinfo.cpp
>>>
============================================================
=======
>>> RCS file:
/cvsroot/protocol/rtsp/rateadaptinfo.cpp,v
>>> retrieving revision 1.12.8.5
>>> diff -w -u -b -r1.12.8.5 rateadaptinfo.cpp
>>> --- rateadaptinfo.cpp   31 Oct 2006 19:06:28
-0000      1.12.8.5
>>> +++ rateadaptinfo.cpp   13 Nov 2007 19:44:25
-0000
>>>  -61,6 +61,7 
>>>  #include "pckunpck.h"
>>>  #include "3gpadapthdr.h"
>>>  #include "helixadapthdr.h"
>>> +#include "hxbufctl.h" //
IHXTransportBufferLimit
>>>  
>>>  static const char z_3GPPAdaptationHdr[]  = 
>> "3GPP-Adaptation";  static 
>>> const char z_HelixAdaptationHdr[] =
"Helix-Adaptation";  -593,6 
>>> +594,20 
>>>                  {
>>>                      m_bRateAdaptationUsed =
TRUE;
>>>                      hr = HXR_OK;
>>> +
>>> +                    IHXTransportBufferLimit*
pBufLimit = NULL;
>>> +                    IHXStreamSource* pSource =
NULL;
>>> +                    if (HXR_OK == 
>> m_pContext->QueryInterface(IID_IHXStreamSource,
(void**)&pSource))
>>> +                    {
>>> +                        if (pSource &&
(HXR_OK == 
>>
pSource->QueryInterface(IID_IHXTransportBufferLimit, 
>> (void**)&pBufLimit)))
>>> +                        {
>>> +                          
>>
pBufLimit->SetByteLimit(pInfo->StreamNumber(), 
>> pInfo->Get3gpBufferSize());
>>> +                        }
>>> +
>>> +                       
HX_RELEASE(pBufLimit);
>>> +                    }
>>> +
>>> +                    HX_RELEASE(pSource);
>>>                  }
>>>              }
>>>          }
>>>
>>>
>>> Index: fbbufctl.cpp
>>>
============================================================
=======
>>> RCS file: /cvsroot/client/core/fbbufctl.cpp,v
>>> retrieving revision 1.13
>>> diff -w -u -b -r1.13 fbbufctl.cpp
>>> --- fbbufctl.cpp        25 Jul 2005 15:30:04
-0000      1.13
>>> +++ fbbufctl.cpp        13 Nov 2007 19:43:33
-0000
>>>  -53,6 +53,7 
>>>  #include "hlxclib/math.h"
>>>  #include "hxprefutil.h"
>>>  #include "hxtlogutil.h"
>>> +#include "ihxrateadaptctl.h"
>>>  
>>>  //#define HELIX_FEATURE_DBG_LOG
>>>  //#define BUFFER_CONTROL_TESTING
>>>  -174,7 +175,8 
>>>      m_ulControlBw(0),
>>>      m_bControlTotal(FALSE),
>>>      m_bPaused(TRUE),
>>> -    m_state(csNotInitialized)
>>> +    m_state(csNotInitialized),
>>> +    m_pRateAdaptCtl(NULL)
>>>  {
>>>      m_control.Init(0.7, 1.25664, 0.2, 5.0);  }
 -252,7
+254,11 
>>>                                                

>> (void**)&m_pStatus)) &&
>>>              (HXR_OK ==
pContext->QueryInterface(IID_IHXPreferences, 
>>>                                                

>> (void**)&m_pPrefs)) &&
>>> -            (HXR_OK == ReadPrefSettings()))
>>> +            (HXR_OK == ReadPrefSettings())
&&
>>> +            (HXR_OK == 
>>
pContext->QueryInterface(IID_IHXClientRateAdaptControl,
>>> +                                           
>> (void**)&m_pRateAdaptCtl))
>>> +
>>> +            )
>>>  
>>>          {
>>>  #ifdef BUFFER_CONTROL_TESTING
>>>  -460,6 +466,7 
>>>      HX_RELEASE(m_pThin);
>>>      HX_RELEASE(m_pPrefs);
>>>      HX_RELEASE(m_pStatus);
>>> +    HX_RELEASE(m_pRateAdaptCtl);
>>>  
>>>      ChangeState(csNotInitialized);
>>>  
>>>  -819,6 +826,10 
>>>                      ulByteLimit = 1;
>>>                  }
>>>  
>>> +                HXBOOL bEnabled = TRUE;
>>> +               
m_pRateAdaptCtl->IsEnabled(i, bEnabled);
>>> +
>>> +                if (bEnabled)
>>>                  pBufLimit->SetByteLimit(i,
ulByteLimit);
>> Please add '{' around all 'if' statements, even one
liners.
>>
>> the rest looks good.
>>
>> --greg.
>>
>>
>>
>>>              }
>>>          }
>>> Index: fbbufctl.h
>>>
============================================================
=======
>>> RCS file: /cvsroot/client/core/fbbufctl.h,v
retrieving revision 1.5 
>>> diff -w -u -b -r1.5 fbbufctl.h
>>> --- fbbufctl.h  14 Mar 2005 20:31:01 -0000     
1.5
>>> +++ fbbufctl.h  13 Nov 2007 19:43:33 -0000
>>>  -57,6 +57,7 
>>>  #include "hxerror.h"
>>>  #include "hxprefs.h" //
IHXPreferences  #include "hxpends.h" // 
>>> IHXPendingStatus
>>> +#include "ihxrateadaptctl.h"
>>>  
>>>  class HXFeedbackControl
>>>  {
>>>  -271,6 +272,7 
>>>  
>>>      HXBOOL m_bPaused;
>>>      ControlState m_state;
>>> +    IHXClientRateAdaptControl*
m_pRateAdaptCtl;
>>>  };
>>>  
>>>  #endif /* FBBUFCTL_H */
>>
> 
> _______________________________________________
> Client-dev mailing list
> Client-devhelixcommunity.org
> http://lists.helixcommunity.org/mailman/listinfo/clie
nt-dev

_______________________________________________
Protocol-dev mailing list
Protocol-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/pr
otocol-dev

RE: RESEND: CR: Release 6 Rate Adaptation bug fix

Thread: RE: RESEND: CR: Release 6 Rate Adaptation bug fix