| Summary: | MediaRecorderPrivateWriter::appendEndOfVideoSampleDurationIfNeeded should only call requestMediaDataWhenReadyOnQueue once | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
| Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer, youennf | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Local Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 85851 | ||||||||
| Attachments: |
|
||||||||
|
Description
youenn fablet
2020-07-30 09:10:40 PDT
Created attachment 405697 [details]
Patch
Created attachment 405816 [details]
Patch for landing
Thanks for the r+ Eric. I changed it a bit to capture the queue of pending audio and video samples in the block given to requestMediaDataWhenReadyOnQueue. This should ensure we do not grab other incoming samples. Committed r265192: <https://trac.webkit.org/changeset/265192> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405816 [details]. Comment on attachment 405816 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=405816&action=review > Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:263 > [m_audioAssetWriterInput.get() appendSampleBuffer:buffer.get()]; Shouldn't we check [m_audioAssetWriterInput isReadyForMoreMediaData] before appending? > Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:294 > + m_lastVideoPresentationTime = CMSampleBufferGetPresentationTimeStamp(buffer); > + m_lastVideoDecodingTime = CMSampleBufferGetDecodeTimeStamp(buffer); > + m_hasEncodedVideoSamples = true; > + > + [m_videoAssetWriterInput.get() appendSampleBuffer:buffer]; Shouldn't this check [m_videoAssetWriterInput isReadyForMoreMediaData] before appending in case, for example, appendCompressedVideoSampleBufferIfPossible filled it with pending samples? Thanks, will fix that as a follow-up. (In reply to Eric Carlson from comment #5) > Comment on attachment 405816 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=405816&action=review > > > Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:263 > > [m_audioAssetWriterInput.get() appendSampleBuffer:buffer.get()]; > > Shouldn't we check [m_audioAssetWriterInput isReadyForMoreMediaData] before > appending? > > > Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:294 > > + m_lastVideoPresentationTime = CMSampleBufferGetPresentationTimeStamp(buffer); > > + m_lastVideoDecodingTime = CMSampleBufferGetDecodeTimeStamp(buffer); > > + m_hasEncodedVideoSamples = true; > > + > > + [m_videoAssetWriterInput.get() appendSampleBuffer:buffer]; > > Shouldn't this check [m_videoAssetWriterInput isReadyForMoreMediaData] > before appending in case, for example, > appendCompressedVideoSampleBufferIfPossible filled it with pending samples? We also need to make sure flushCompressedSampleBuffers is not called by stopRecording while doing work for fetchData. |