Re: CR: fix for Bug 220782: using the ?start=xx URL parameter causes playback to never start

Milko Boic <[email protected]>
Newsgroups gmane.comp.multimedia.helix.devel
Message-ID <[email protected]>
This fix is not quite correct.  There are two problems:

1.) The applied offset is not correct.  It should be 
ulMaxRendererDispatchTime += m_ulStartTime - m_ulDelay;

2.) Second issue is more involved and stems from a flaw in RecordControl 
and RecordSource APIs.
m_ulStartTime - m_ulDelay offset is already communicated to the record 
control as packet associated time-stamp offset:
m_pRecordControl->OnPacket(pPacket, m_ulStartTime - m_ulDelay);
However, record control does not provide for the return of the passed in 
offset:
STDMETHODIMP
HXRecordControl::PacketReady(HX_RESULT status, IHXPacket* pPacket)
HX_RESULT
HXRecordControl::GetPacket(UINT16 usStreamNumber, IHXPacket*& pPacket)

Conceptually, the time-stamp offset is custom for each packet to allow for 
stream splicing by adjusting the time-stamp in datatype agnostic 
manner.  Treating it globally limits the usefulness of packet offset and 
thus engine capabilities.  It is true enough that this concept is broken in 
other places - HXRecordControl for example.  However, we should try to 
limit the flaw so it is easier to fix later when needed.  Ultimately, the 
time-stamp offset needs to be passed and preserved through all the 
components packet travels.


What to do now:
Limit the flaw to RecordControl and have 
RecordControl::GetLatestTimestampWritten() and GetLatestTimestampRead() 
return offset time-stamps so they can be directly to the timeline:

HX_RESULT HXRecordControl::SendPacket(IHXPacket* pPacket, INT32 lTimeOffset)
{
...
     if (m_pRecordSource)
     {
-        UINT32 ulTime = pPacket->GetTime();
+       UINT32 ulTime = pPacket->GetTime() - lTimeOffset;


HX_RESULT
HXRecordControl::OnPacket(IHXPacket* pPacket, INT32 nTimeOffset)
{
...

+   m_lTimeOffset = nTimeOffset;
#if defined(HELIX_FEATURE_RECORDCONTROL_MERGESORT)
     if (m_bDisableRSMergeSort)
     {
         nResult = WritePacket(pPacket, nTimeOffset);
     }
     else if (m_pMergeSorter)
     {
         // Save the time offset
-        m_lTimeOffset = nTimeOffset;


HX_RESULT
HXRecordControl::GetPacket(UINT16 usStreamNumber, IHXPacket*& pPacket)
{
...
     if (pPacket)
     {
-       UINT32 ulTime = pPacket->GetTime();
+      UINT32 ulTime = pPacket->GetTime() - m_lTimeOffset;  // This is not 
quite right but best we can do given that offset is not being passed 
through the record source.  It works as long as the packet offsets are not 
changing within the source.  Currently, there are no such cases.
         if (m_bFirstTimestampRead)

In addition, above fix corrects network source record control reading as 
well network source super-buffer overrun protection:
     // If we are playing from record control and we are
     // forward accelerated, then it's possible for accelerated
     // playback to overtake the write point. Here we detect
     // if we are about to overtake the write point. If we are,
     // then go back to normal velocity.
     if (m_bPlayFromRecordControl && m_pRecordControl &&
         m_pPlayer->GetVelocity() > HX_PLAYBACK_VELOCITY_NORMAL &&
         !m_pRecordControl->IsFinishedWriting() &&
         !m_bDisableRecordSourceOverrunProtection)
     {
         UINT32 ulPlayTime  = m_pPlayer->GetCurrentPlayTime();
         UINT32 ulWriteTime = m_pRecordControl->GetLatestTimestampWritten();
         if (ulWriteTime > m_ulRecordSourceOverrunProtectionTime &&
             ulPlayTime  > (ulWriteTime - 
m_ulRecordSourceOverrunProtectionTime))
         {
             HXLOGL2(HXLOG_TRIK, "Playback time (%lu) getting close to record "
                     "control write time (%lu), resetting velocity",
                     ulPlayTime, ulWriteTime);
             // We are getting close to overrun, so reset velocity
             m_pPlayer->SetVelocity(100, FALSE, FALSE);
         }
     }


Milko

At 07:36 PM 6/25/2008, Qiang Luo wrote:
>Synopsis:
>Fix for Bug 220782: using the ?start=xx URL parameter causes playback to 
>never start
>
>Overview:
>For http file-source, the renders get packets from the supper 
>buffer.  Before reading racket from the recorded file and inserting it 
>into event queue, the core computes ulMaxRendererDispatchTime and checks 
>it against the recorder's last read time-stamp.  When we have ?start=xx in 
>the http url parameter, the check would fail.  The player will be in a 
>loop where HXFileSource::GetEvent() will return HXR_WOULD_BLOCK.  The fix 
>is to account for the non-zero start time when we compute the 
>ulMaxRendererDispatchTime value.
>
>File modified:
>client/core/hxflsrc.cpp
>
>The diff:
>
>Index: hxflsrc.cpp
>===================================================================
>RCS file: /cvsroot/client/core/hxflsrc.cpp,v
>retrieving revision 1.126.2.2
>diff -u -w -1 -0 -r1.126.2.2 hxflsrc.cpp
>--- hxflsrc.cpp 23 Oct 2007 20:57:33 -0000      1.126.2.2
>+++ hxflsrc.cpp 26 Jun 2008 02:28:47 -0000
>@@ -1763,20 +1763,23 @@
>      }
>
>  #if defined(HELIX_FEATURE_RECORDCONTROL)
>      if (m_bPlayFromRecordControl && m_pRecordControl)
>      {
>         UINT32 ulMaxRendererDispatchTime = m_pPlayer->ComputeFillEndTime(
>                     m_pPlayer->GetInternalCurrentPlayTime(),
>                     m_pPlayer->GetGranularity(),
>                     m_ulMaxPreRoll + MAX_INTERSTREAM_TIMESTAMP_JITTER);
>
>+       // account for the start time.
>+       ulMaxRendererDispatchTime += m_ulStartTime;
>+
>         theErr = HXR_WOULD_BLOCK;
>
>         // We supply packets from the record control only when needed
>         // since sparse streams may result in excessive buffering of 
> packets due to
>         // interleaving with non-sparse packets.
>         if ((!m_pRecordControl->IsRead()) ||
>             (((LONG32) (ulMaxRendererDispatchTime - 
> m_pRecordControl->GetLatestTimestampRead())) >= 0))
>         {
>             IHXPacket* pPacket = NULL;
>
>Thanks,
>
>Qiang

_______________________________________________
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.