This change has been checked in to 221Cays, 210Cays and HEAD.
Thanks
Saleem
-----Original Message-----
From: ext Eric Hyche [mailto:[email protected]]
Sent: Tuesday, June 10, 2008 12:35 PM
To: Adookkattil Saleem (Nokia-D-MSW/Dallas);
[email protected]; [email protected];
[email protected]
Cc: [email protected]; [email protected]; [email protected]
Subject: RE: [Helix-client-dev] CR: Fix to drop initial RTP packets when
packettime stamp < RTP offset time (Fix only for live streaming)
Ok - this looks good to me.
=============================================
Eric Hyche ([email protected])
Technical Lead
RealNetworks, Inc.
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]]
> Sent: Friday, June 06, 2008 6:52 PM
> To: [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]
> Subject: RE: [Helix-client-dev] CR: Fix to drop initial RTP packets
> when packettime stamp < RTP offset time (Fix only for live streaming)
>
> Hi,
>
> I am checking two places whether the packet timestamp < RTPOffset time
> and dropping the packet if the timestamp < RTP offset time.
> Sometimes we
> receive RTP packets before receiving the RTPOffsettime time. In this
> case the first check is going to fail and the packet get added to
> StartInfoWaitQueue. The second check going to tak care of the
> packets in
> the queue.
>
> Added condition to check whether received RTP offset time in the play
> response. Ie, if (m_bIsLive && m_bRTPTimeSet). This should
> take care of
> the specified failure case. All other case we don't drop the packets.
>
> Thanks
> Saleem
>
> Index: rtptran.cpp
> ===================================================================
> RCS file: /cvsroot/protocol/transport/rtp/rtptran.cpp,v
> retrieving revision 1.101.2.4.2.1
> diff -u -w -r1.101.2.4.2.1 rtptran.cpp
> --- rtptran.cpp 8 May 2008 13:41:10 -0000 1.101.2.4.2.1
> +++ rtptran.cpp 6 Jun 2008 22:29:38 -0000
> @@ -1560,6 +1560,13 @@
> // stamps
> if (m_bWaitForStartInfo)
> {
> + if (m_bIsLive && m_bRTPTimeSet)
> + {
> + if (CompareRTPTimestamp(pkt.timestamp,
> m_lTimeOffsetRTP) <
> 0)
> + {
> + return HXR_OK;
> + }
> + }
> if (m_StartInfoWaitQueue.GetCount() == 0)
> {
> // First packet received
> @@ -1661,19 +1668,29 @@
>
> if (pStoredBuffer)
> {
> - if (m_bFirstSeqNumLocked)
> - {
> - // If sequence number is locked, we drop any
> packet that precedes the locked in sequence number
> RTPPacket pkt;
> if (pkt.unpack(pBuffer->GetBuffer(),
> pBuffer->GetSize()) == 0)
> {
> + if (m_bFirstSeqNumLocked)
> + {
> + // If sequence number is locked, we drop
> any packet that precedes the locked in sequence number
> LONG32 lSeqNumDelta = ((LONG32)
> (((UINT16)
> pkt.seq_no) - m_uFirstSeqNum));
> if (lSeqNumDelta < 0)
> {
> HX_RELEASE(pStoredBuffer);
> }
> }
> +
> + if (pStoredBuffer && m_bIsLive &&
> m_bRTPTimeSet)
> + {
> + if (CompareRTPTimestamp(pkt.timestamp,
> m_lTimeOffsetRTP) < 0)
> + {
> + HX_RELEASE(pStoredBuffer);
> + }
> +
> + }
> }
> +
> if (pStoredBuffer)
> {
> _handlePacket(pStoredBuffer, FALSE);
> @@ -4654,3 +4671,22 @@
>
> return rc;
> }
> +
> +INT16 RTPBaseTransport::CompareRTPTimestamp(ULONG32 x, ULONG32 y)
> +{
> + ULONG32 min = y;
> +
> + if ( (x <= y && y - x < ULONG32( 1 << 31 ) ) ||
> + (x > y && x - y > ULONG32( 1 << 31 ) ) )
> + {
> + min = x;
> + }
> +
> + INT16 nResult = -1;
> + if ( x != min )
> + nResult = 1;
> + else if ( x == y )
> + nResult = 0;
> +
> + return nResult;
> +}
>
>
>
> -----Original Message-----
> From: ext Eric Hyche [mailto:[email protected]]
> Sent: Thursday, June 05, 2008 9:33 AM
> To: Adookkattil Saleem (Nokia-D-MSW/Dallas);
> [email protected]; [email protected];
> [email protected]
> Cc: 'Gregory Wright'; 'Henry Ping'; Milko Boic
> Subject: RE: [Helix-client-dev] CR: Fix to drop initial RTP
> packets when
> packettime stamp < RTP offset time (Fix only for live streaming)
>
>
> Saleem,
>
> Here are my comments. Greg/Henry/Milko: I would appreciate it
> if another
> set of eyeballs could take a look at this change, since it's in code
> that I'm not the most familiar with.
>
> I'm not sure why this change is needed:
>
> @@ -1661,19 +1668,29 @@
>
> if (pStoredBuffer)
> {
> - if (m_bFirstSeqNumLocked)
> - {
> - // If sequence number is locked, we drop any
> packet that precedes the locked in sequence number
>
> RTPPacket pkt;
> if (pkt.unpack(pBuffer->GetBuffer(),
> pBuffer->GetSize()) == 0)
> {
> + if (m_bFirstSeqNumLocked)
> + {
> + // If sequence number is
> locked, we drop
> + any packet that precedes the locked in sequence number
>
> LONG32 lSeqNumDelta = ((LONG32)
> (((UINT16)
> pkt.seq_no) - m_uFirstSeqNum));
> if (lSeqNumDelta < 0)
> {
> HX_RELEASE(pStoredBuffer);
> }
> }
> +
> + if (pStoredBuffer && m_bIsLive )
> + {
> + if (CompareRTPTimestamp(pkt.timestamp,
> m_lTimeOffsetRTP) < 0)
> + {
> + HX_RELEASE(pStoredBuffer);
> + }
> +
> + }
> }
> +
> if (pStoredBuffer)
> {
> _handlePacket(pStoredBuffer, FALSE);
>
> Since it is checking packets in the m_StartInfoWaitQueue queue.
> However, there is only one place that packets can be added to the
> m_StartInfoWaitQueue queue, and that's here:
>
> pBuffer->AddRef();
> m_StartInfoWaitQueue.AddTail(pBuffer);
> m_ulWaitQueueBytes += pBuffer->GetSize();
>
> But this code is below this change:
>
> @@ -1560,6 +1560,13 @@
> // stamps
> if (m_bWaitForStartInfo)
> {
> + if (m_bIsLive)
> + {
> + if (CompareRTPTimestamp(pkt.timestamp,
> m_lTimeOffsetRTP) <
> 0)
> + {
> + return HXR_OK;
> + }
> + }
> if (m_StartInfoWaitQueue.GetCount() == 0)
> {
> // First packet received
>
> which means that if the CompareRTPTimestamp check failed, then these
> packets could never get added to the StartInfoWaitQueue queue.
>
> Also, what happens if the PLAY response doesn't have a play range? I
> guess in that case lTimeOffsetRTP will be 0 in which case the
> CompareRTPTimestamp check will never fail. If that correct?
>
> Eric
>
> =============================================
> Eric Hyche ([email protected])
> Technical Lead
> RealNetworks, Inc.
>
> > -----Original Message-----
> > From: [email protected]
> > [mailto:[email protected]] On Behalf Of
> > [email protected]
> > Sent: Tuesday, June 03, 2008 3:49 PM
> > To: [email protected];
> > [email protected]; [email protected]
> > Subject: [Helix-client-dev] CR: Fix to drop initial RTP
> packets when
> > packettime stamp < RTP offset time (Fix only for live streaming)
> >
> > "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: [email protected]
> >
> > Reviewed by:
> >
> > Date: 02-June-2008
> >
> > Project: SymbianMmf_Rel
> >
> > TSW: NGRN-7DRASY
> >
> > Synopsis: Fix to drop initial RTP packets when packet time stamp
> <
> > RTP offset time (Fix only for live streaming)
> >
> > Overview: Sometimes helix client receives few initial RTP
> packets
> > with timestamp < RTP offset time. As these packets are
> already delayed
>
> > packets there is no need to store these packets to the
> buffer. Added
> > code to drop these packet after comparing the timestamp with RTP
> > offset time.
> >
> >
> > Files Modified:
> >
> > /cvsroot/protocol/transport/rtp/rtptran.cpp
> >
> > Image Size and Heap Use impact: None
> >
> > Module Release testing (STIF) : Pass
> >
> > Test case(s) Added : No
> >
> > Memory leak check performed : No new leaks introduced.
> >
> > Platforms and Profiles Build Verified:
> > Profile -> helix-client-s60-32-mmf-mdf-arm
> > BIF branch -> helix_restricted
> > SYSTEM_ID -> symbian-91-armv5
> > Target -> symbianMmf_rel
> >
> > Platforms and Profiles Functionality verified: armv5, winscw
> >
> >
> > Branch: 210CayS, 221CayS& head
> >
> > Index: rtptran.cpp
> > ===================================================================
> > RCS file: /cvsroot/protocol/transport/rtp/rtptran.cpp,v
> > retrieving revision 1.101.2.4.2.1
> > diff -u -w -r1.101.2.4.2.1 rtptran.cpp
> > --- rtptran.cpp 8 May 2008 13:41:10 -0000 1.101.2.4.2.1
> > +++ rtptran.cpp 3 Jun 2008 19:16:26 -0000
> > @@ -1560,6 +1560,13 @@
> > // stamps
> > if (m_bWaitForStartInfo)
> > {
> > + if (m_bIsLive)
> > + {
> > + if (CompareRTPTimestamp(pkt.timestamp,
> > m_lTimeOffsetRTP) < 0)
> > + {
> > + return HXR_OK;
> > + }
> > + }
> > if (m_StartInfoWaitQueue.GetCount() == 0)
> > {
> > // First packet received @@ -1661,19 +1668,29 @@
> >
> > if (pStoredBuffer)
> > {
> > - if (m_bFirstSeqNumLocked)
> > - {
> > - // If sequence number is locked, we
> > drop any packet that precedes the locked in sequence number
> >
> > RTPPacket pkt;
> > if (pkt.unpack(pBuffer->GetBuffer(),
> > pBuffer->GetSize()) == 0)
> > {
> > + if (m_bFirstSeqNumLocked)
> > + {
> > + // If sequence number is
> > locked, we drop any packet that precedes the locked in
> sequence number
> >
> > LONG32 lSeqNumDelta = ((LONG32)
> > (((UINT16) pkt.seq_no) - m_uFirstSeqNum));
> > if (lSeqNumDelta < 0)
> > {
> > HX_RELEASE(pStoredBuffer);
> > }
> > }
> > +
> > + if (pStoredBuffer && m_bIsLive )
> > + {
> > + if
> > (CompareRTPTimestamp(pkt.timestamp, m_lTimeOffsetRTP) < 0)
> > + {
> > + HX_RELEASE(pStoredBuffer);
> > + }
> > +
> > + }
> > }
> > +
> > if (pStoredBuffer)
> > {
> > _handlePacket(pStoredBuffer, FALSE); @@
> > -4654,3 +4671,22 @@
> >
> > return rc;
> > }
> > +
> > +INT16 RTPBaseTransport::CompareRTPTimestamp(ULONG32 x, ULONG32 y) {
> > + ULONG32 min = y;
> > +
> > + if ( (x <= y && y - x < ULONG32( 1 << 31 ) ) ||
> > + (x > y && x - y > ULONG32( 1 << 31 ) ) )
> > + {
> > + min = x;
> > + }
> > +
> > + INT16 nResult = -1;
> > + if ( x != min )
> > + nResult = 1;
> > + else if ( x == y )
> > + nResult = 0;
> > +
> > + return nResult;
> > +}
> >
> > Index: rtptran.h
> > ===================================================================
> > RCS file: /cvsroot/protocol/transport/rtp/pub/rtptran.h,v
> > retrieving revision 1.51.2.1.2.1
> > diff -u -w -r1.51.2.1.2.1 rtptran.h
> > --- rtptran.h 8 May 2008 13:41:11 -0000 1.51.2.1.2.1
> > +++ rtptran.h 3 Jun 2008 19:19:12 -0000
> > @@ -355,6 +355,7 @@
> >
> > private:
> > HX_RESULT _handlePacket(IHXBuffer* pBuffer, HXBOOL
> bIsRealTime);
> > + INT16 CompareRTPTimestamp(ULONG32 x, ULONG32 y);
> > };
> >
> > /*
> >
> >
>
_______________________________________________
Helix-client-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/helix-client-dev
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.