[datatype-dev] CN:Changes to fix Bug 9312: Video is not getting synchronized with audio particularly when movie is paused and replayed - when tried with the movies from Samsung for this test.
Rishi Mathew rmathew at real.comAlso to HEAD please. THanks, Rishi. At 12:26 AM 6/19/2009, Varun Kathuria wrote: >Thanks Eric, > >This has been checked into atlas310, atlas347 & atlas349. > >Thanks >Varun Kathuria > >----- Original Message ----- From: "Eric Hyche" <ehyche at real.com> >To: "'Varun Kathuria'" <vkathuria at real.com>; "'Gregory Wright'" ><gwright at real.com> >Cc: <datatype-dev at helixcommunity.org> >Sent: Friday, June 19, 2009 3:18 AM >Subject: RE: [datatype-dev] CR:Changes to fix Bug 9312: Video is not >getting synchronized with audio particularly when movie is paused >and replayed - when tried with the movies from Samsung for this test. > > >>I believe your file format fix is the correct one. >> >>Eric >> >>======================================= >>Eric Hyche (ehyche at real.com) >>Principal Engineer >>RealNetworks, Inc. >> >> >>>-----Original Message----- >>>From: datatype-dev-bounces at helixcommunity.org >>>[mailto:datatype-dev-bounces at helixcommunity.org] On >>>Behalf Of Varun Kathuria >>>Sent: Tuesday, June 16, 2009 7:28 AM >>>To: Gregory Wright >>>Cc: datatype-dev at helixcommunity.org >>>Subject: Re: [datatype-dev] CR:Changes to fix Bug 9312: Video is >>>not getting synchronized with audio >>>particularly when movie is paused and replayed - when tried with >>>the movies from Samsung for this >>>test. >>> >>>Hi Greg, >>> >>>Here are the details, >>>The avi file has mp3 audio which has has 16 bits per sample, 2 channels & >>>48000 sample rate. >>>Frametime is calculated as 24 ms. >>> >>>Now Audio device & mp3 renderer calculates timestamp based on 24. It means >>>they both add 24 ms after every >>>packet & maintain sync. >>> >>>But in this avi file, packet difference is sometimes 21,24,23,29 etc . >>>So this creates the difference between Original Packettime & calculated >>>packettime. >>> >>>Lets take the case, we pause the file at 44959 ms(playback time) Here >>>lastpacket time is 49080 ms & llActualTimestamp calculated from >>>audio device is 50400 ms. >>> >>>Now when we resume the file, then CRnMp3Ren::OnBegin(UINT32 timeAfterBegin) >>>is called which inturns calls m_pPacketParser->Begin(timeAfterBegin) >>>functions which do nothing with timeAfterBegin but only setting m_dNextPts = >>>0. Because m_dNextPts is set at 0 so it is calculating again from packettime >>>& Now Packet timestamp is 49103 ms & so m_dNextPts is calculated as 49103 ms >>>which is less than 50400 ms. It creates difference between packet >>>timestamps of mp3 renderer & audio device. >>> >>>The difference between Actual packet timestamp & calculated packet timestamp >>>is because mp3 packetizer is calculating packet timestamp >>>based on constant frame size. But when avi file format added these packets, >>>it is calculated by total bytes received so far. >>> >>>In AVI fileformat, it is calculated as >>> ulTime = (UINT32) (((double)m_ulTotalBytesPacketised / >>>pWaveInfo->ulAvgBytesPerSec) * 1000); >>> >>>When we play only mp3 VBR file, it does not make any difference because mp3 >>>fileformat also calculates packet timestamp based on constant framesize. >>> >>>The MP3 file has a standard format, which is a frame that consists of 1152 >>>samples. So frame time is also constant. >>> >>>There can be 2 solutions to fix this issue: >>> >>>1) We can calculate packet time in avi fileformat for mp3 file taking >>>constant frame time like below: >>> >>>Index: avistrm.cpp >>>=================================================================== >>>RCS file: /cvsroot/datatype/avi/fileformat/avistrm.cpp,v >>>retrieving revision 1.10.2.10 >>>diff -u -r1.10.2.10 avistrm.cpp >>>--- avistrm.cpp 24 Dec 2008 23:19:40 -0000 1.10.2.10 >>>+++ avistrm.cpp 16 Jun 2009 10:09:06 -0000 >>>@@ -1398,7 +1399,15 @@ >>> if (IsAudio ()) >>> { >>> WaveInfo* pWaveInfo = (WaveInfo*) m_pFormat; >>>- ulTime = (UINT32) (((double)m_ulTotalBytesPacketised / >>>pWaveInfo->ulAvgBytesPerSec) * 1000); >>>+ if(pWaveInfo->usFormatTag==WAVE_FORMAT_MPEGLAYER3) >>>+ { >>>+ m_ulTime += >>>(UINT32)(((double)1152/pWaveInfo->ulSamplesPerSec)*1000); >>>+ ulTime = m_ulTime; >>>+ } >>>+ else >>>+ { >>>+ ulTime = (UINT32) (((double)m_ulTotalBytesPacketised / >>>pWaveInfo->ulAvgBytesPerSec) * 1000); >>>+ } >>> } >>> >>>2) We can detect VBR file in mp3 packetizer & do not set m_dNextPts as 0 in >>>VBR like below: >>> >>>Index: pub/pktparse.h >>>=================================================================== >>>RCS file: /cvsroot/datatype/mp3/payload/pub/pktparse.h,v >>>retrieving revision 1.6.16.1 >>>diff -u -r1.6.16.1 pktparse.h >>>--- pub/pktparse.h 3 Apr 2008 21:17:02 -0000 1.6.16.1 >>>+++ pub/pktparse.h 16 Jun 2009 10:13:31 -0000 >>>@@ -59,7 +59,7 @@ >>> inline virtual double GetTimePerPkt (void) { return m_dFrameTime; } >>> inline virtual double GetLastPCMTime (void) { return >>>m_dLastPCMTime; } >>> >>>- inline virtual void Begin (UINT32 time) { m_dNextPts = >>>0.0; } >>>+ inline virtual void Begin (UINT32 time) { if (!m_bIsVBR) >>>m_dNextPts = 0.0; } >>> >>>______________________________________________________________________________________________________ >>>________________ >>>Here are the answers to your Questions: >>> >>>>>o Where in the engine is this called, and under what conditions? Is the >>>>>value always >>>>> zero in all cases? >>>[Varun] >>>1) When we start playing the file or seek the file then CRnMp3Ren::OnBegin() >>>is called from HXPlayer::CheckBeginList(void) >>> >>>2) CRnMp3Ren::OnBegin() is called from SourceInfo::Begin() when we pause >>>the file & resume it again. >>>>>o For CBR, is timeAfterBegin correct and useful? I assume that is the >>>>>resume time >>>>> really. >>>[Varun] >>>No, timeAfterBegin is not used in CBR as well as VBR. >>> >>>>>o Can you provide a, possibly new, member variable of MP3 that can keep >>>>>track of >>>>> internal state for VBR playback and adjust as needed that way? >>>[Varun] I dont understand the question. Can you please explain.. >>> >>>>>o Can you just detect VBR and ignore it in the packetizer? It seems that >>>>>it is >>>>> the packetizer that has problems with a begin time and the difference >>>>>between >>>>> VBR and CBR. I would think it would make sense to fix it there. >>>[Varun] We can fix it in packtizer such that if it is VBR file then we do >>>not make m_dNextPts as 0. Does it sound right? >>> >>> >>>Please suggest what can we do? >>> >>> >>>Thanks >>>Varun Kathuria >>>----- Original Message ----- >>>From: "Gregory Wright" <gwright at real.com> >>>To: "Varun Kathuria" <vkathuria at real.com> >>>Cc: <datatype-dev at helixcommunity.org> >>>Sent: Thursday, June 11, 2009 8:26 PM >>>Subject: Re: [datatype-dev] CR:Changes to fix Bug 9312: Video is not getting >>>synchronized with audio particularly when movie is paused and replayed - >>>when tried with the movies from Samsung for this test. >>> >>> >>>I don't believe this is the correct fix, at least not at the right >>>level. I will list my concerns below, but perhaps I just don't >>>understand >>>your failure case well enough. Can you go into a bit more detail on >>>what is going wrong? It appears that we are calculating a time or offset >>>from a starting time based on a constant MP3 frame size and that this >>>leads to incorrect times/positions for VBR MP3? >>> >>> Index: mp3rend.cpp >>>=================================================================== >>>RCS file: /cvsroot/datatype/mp3/renderer/mp3rend.cpp,v >>>retrieving revision 1.34.8.1 >>>diff -u -r1.34.8.1 mp3rend.cpp >>>--- mp3rend.cpp 3 Apr 2008 21:19:36 -0000 1.34.8.1 >>>+++ mp3rend.cpp 10 Jun 2009 09:40:29 -0000 >>>@@ -519,7 +519,7 @@ >>> #endif >>> #endif >>> >>>- if(m_pPacketParser) >>>+ if(m_pPacketParser && !timeAfterBegin) >>> { >>> m_pPacketParser->Begin(timeAfterBegin); >>> } >>> >>> >>>A few questions. >>> >>>o Where in the engine is this called, and under what conditions? Is >>>the value always >>> zero in all cases? >>>o For CBR, is timeAfterBegin correct and useful? I assume that is the >>>resume time >>> really. >>>o Can you provide a, possibly new, member variable of MP3 that can >>>keep track of >>> internal state for VBR playback and adjust as needed that way? >>>o Can you just detect VBR and ignore it in the packetizer? It seems >>>that it is >>> the packetizer that has problems with a begin time and the >>>difference between >>> VBR and CBR. I would think it would make sense to fix it there. >>> >>> >>>--greg. >>> >>> >>> >>>On Jun 10, 2009, at 6:23 AM, Varun Kathuria wrote: >>> >>>> >>>>RealPlayer For Netbook >>>> >>>>Synopsis: >>>>Changes to fix Bug 9312: Video is not getting synchronized with audio >>>>particularly when movie is paused and replayed - when tried with the >>>>movies from Samsung for this test. >>>> >>>>Overview: >>>>When we play the particular avi file in which audio container is mp3 then >>>>there is audio - video sync issue when we pause the file & play again. >>>>The problem is because in the mp3 render we are calculating >>>>audioData.ulAudioTime assuming the fix frame time i.e 24 ms. But as it is >>>>VBR mp3 so we are getting variable sized packets. When we pause the file >>>>& play again then we set m_dNextPts(next packet timestamp) as zero. At >>>>the time of rendering m_dNextPts is calculated again based on current >>>>packet timestamp. Then there occurs difference between actual packet >>>>timestamp & calculated packet timestamp in audio device and we get ASSERT >>>>fail for "Packets written out of order". The solution is not to set >>>>m_dNextPts = 0 when pausing the file & playing again. >>>> >>>>Files Added: >>>>None >>>> >>>>Files Modified: >>>>/cvsroot/datatype/mp3/renderer/mp3rend.cpp >>>> >>>>Image Size and Heap Use impact (Client -Only): >>>>None. >>>> >>>>Platforms and Profiles Affected: >>>>None >>>> >>>>Distribution Libraries Affected: >>>>None >>>> >>>>Distribution library impact and planned action: >>>>None >>>> >>>>Branch >>>>atlas310, atlas347 & atlas349 >>>> >>>>Platforms and Profiles Build Verified: >>>>BIF branch -> hxclient_3_1_0_atlas_restricted >>>>Target(s) -> player_all >>>>Profile -> helix-client-moblin >>>>SYSTEM_ID -> linux-2.2-libc6-gcc32-i586 >>>> >>>>Files Attached: >>>>diff_9312.txt >>>> >>>>Thanks & Regards, >>>>Varun Kathuria >>>><diff_9312.txt>_______________________________________________ >>>>Datatype-dev mailing list >>>>Datatype-dev at helixcommunity.org >>>>http://lists.helixcommunity.org/mailman/listinfo/datatype-dev >>> >>> >>>_______________________________________________ >>>Datatype-dev mailing list >>>Datatype-dev at helixcommunity.org >>>http://lists.helixcommunity.org/mailman/listinfo/datatype-dev > > >_______________________________________________ >Datatype-dev mailing list >Datatype-dev at helixcommunity.org >http://lists.helixcommunity.org/mailman/listinfo/datatype-dev Rishi Mathew Director of Technology Licensing Operations RealNetworks, Inc. rmathew at real.com http://www.helixcommunity.org http://www.realnetworks.com/products/support/devsupport.html -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.helixcommunity.org/pipermail/datatype-dev/attachments/20090619/e5ed612d/attachment-0001.html