Summary: | Ensure synchronized rendering of incoming audio tracks | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||
Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | cdumez, eric.carlson, ews-watchlist, glenn, hta, japhet, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
youenn fablet
2021-10-27 05:43:48 PDT
Created attachment 442682 [details]
Patch
Created attachment 442683 [details]
Patch
Created attachment 442694 [details]
Patch
Created attachment 442703 [details]
Patch
Comment on attachment 442703 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442703&action=review r=me once the bots are happy > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCAudioModule.cpp:146 > + if (m_isRenderingIncomingAudio) > + m_incomingAudioMediaStreamTrackRendererUnit->newAudioChunkPushed(); > + m_currentAudioSampleCount += LibWebRTCAudioFormat::chunkSampleCount; Is it correct to update m_currentAudioSampleCount after calling `newAudioChunkPushed`? In either case, it might be clearer to pass sample count to `newAudioChunkPushed()` rather than having it query the module. (In reply to Eric Carlson from comment #5) > Comment on attachment 442703 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=442703&action=review > > r=me once the bots are happy Issue was with resetting the audio model when resetting the factory. > > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCAudioModule.cpp:146 > > + if (m_isRenderingIncomingAudio) > > + m_incomingAudioMediaStreamTrackRendererUnit->newAudioChunkPushed(); > > + m_currentAudioSampleCount += LibWebRTCAudioFormat::chunkSampleCount; > > Is it correct to update m_currentAudioSampleCount after calling > `newAudioChunkPushed`? In either case, it might be clearer to pass sample > count to `newAudioChunkPushed()` rather than having it query the module. Right, passing the sample count to newAudioChunkPushed is a good idea. As of the order, I think it makes sense this way, we start calling newAudioChunkPushed with count = 0. Created attachment 442810 [details]
Patch for landing
Committed r285027 (243671@main): <https://commits.webkit.org/243671@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 442810 [details]. (In reply to youenn fablet from comment #6) > (In reply to Eric Carlson from comment #5) > > Comment on attachment 442703 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=442703&action=review > > > > r=me once the bots are happy > > Issue was with resetting the audio model when resetting the factory. > > > > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCAudioModule.cpp:146 > > > + if (m_isRenderingIncomingAudio) > > > + m_incomingAudioMediaStreamTrackRendererUnit->newAudioChunkPushed(); > > > + m_currentAudioSampleCount += LibWebRTCAudioFormat::chunkSampleCount; > > > > Is it correct to update m_currentAudioSampleCount after calling > > `newAudioChunkPushed`? In either case, it might be clearer to pass sample > > count to `newAudioChunkPushed()` rather than having it query the module. > > Right, passing the sample count to newAudioChunkPushed is a good idea. > As of the order, I think it makes sense this way, we start calling > newAudioChunkPushed with count = 0. I forgot to update newAudioChunkPushed, will do this as a follow-up |