Bug 214972 – MediaRecorderPrivateWriter::appendEndOfVideoSampleDurationIfNeeded should only call requestMediaDataWhenReadyOnQueue once

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

MediaRecorderPrivateWriter::appendEndOfVideoSampleDurationIfNeeded should only call requestMediaDataWhenReadyOnQueue once

[youenn fablet]

Created attachment 405697 [details]
Patch

[youenn fablet]

Created attachment 405816 [details]
Patch for landing

[youenn fablet]

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.

[EWS]

Committed r265192: <https://trac.webkit.org/changeset/265192>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405816 [details].

[Eric Carlson]

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?

[youenn fablet]

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?

[youenn fablet]

https://bugs.webkit.org/show_bug.cgi?id=215081

[youenn fablet]

We also need to make sure flushCompressedSampleBuffers is not called by stopRecording while doing work for fetchData.

[Radar WebKit Bug Importer]

<rdar://problem/66489134>