Bug 212274 – MediaRecorder stopRecorder() returns empty Blob after first use

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

I started working on fixing this bug https://bugs.webkit.org/show_bug.cgi?id=202233 but found weird behaviour with stopRecording(). 
It returns an empty blob when you press stop after you press start, then stop and the start again.

To replicate:

 1. Open Safari
 2. Open the Develop Menu
 3. Hover over the Experimental Features list item
 4. Click on MediaRecorder to Enable it
 5. Navigate to https://codepen.io/majov5/pen/KKdEpKZ
 6. Click on the blue Start Test button at the bottom
 7. Allow access to Microphone & Camera
 8. Click on the Record button at the bottom
 9. Wait a few second and click Stop.
 
In the console of Codepen will we something like

[object Blob] {
   size: 18201,
   slice: function slice() {...},
   type: "video/mp4"
}

 10. Click on the Record button again.
 11. Wait a few second and click Stop.

Now in the console we have empty Blob

[object Blob] {
   size: 0,
   slice: function slice() {...},
   type: ""
}

Is this supposed to happen?

I have test it in Chrome and Firefox, and I always get a Blob with content.
 
I have test it in Safari 13.1, Safari Technology Preview (Release 106 (Safari 13.2, WebKit 15610.1.12.2)) and the MiniBrowser and is the same result as explained in the 11 steps.

[Radar WebKit Bug Importer]

<rdar://problem/63601298>

[youenn fablet]

Created attachment 402637 [details]
WIP

[youenn fablet]

Created attachment 402653 [details]
Patch

[Eric Carlson]

Comment on attachment 402653 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402653&action=review

> Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm:192
> +    fprintf(stderr, "AudioSampleBufferCompressor::attachPrimingTrimsIfNeeded1 %p\n", this);

Oops!

> Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm:194
> +        fprintf(stderr, "AudioSampleBufferCompressor::attachPrimingTrimsIfNeeded2 %p\n", this);

Ditto.

> Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm:206
> +    fprintf(stderr, "AudioSampleBufferCompressor::attachPrimingTrimsIfNeeded3 %p\n", this);

Ditto.

> Source/WebCore/platform/mediarecorder/cocoa/AudioSampleBufferCompressor.mm:208
> +        fprintf(stderr, "AudioSampleBufferCompressor::attachPrimingTrimsIfNeeded4 %p\n", this);

Ditto.

[youenn fablet]

Created attachment 402728 [details]
Patch

[EWS]

Committed r263511: <https://trac.webkit.org/changeset/263511>

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

[Chris Dumez]

Follow-up build fix:
<https://trac.webkit.org/changeset/263514>

[Jason Lawrence]

Reverted r263511, r263514, and r263565 for reason:

r263511 caused MediaRecorder test crashes on internal testers.

Committed r263588: <https://trac.webkit.org/changeset/263588>

[youenn fablet]

Fix is in MediaRecorderPrivateMock::fetchData to unhold m_bufferLock before calling the completion handler since calling the completion handler might delete 'this'

[youenn fablet]

Created attachment 402911 [details]
Patch

[EWS]

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

[youenn fablet]

Created attachment 402988 [details]
Patch for relanding

[EWS]

Committed r263633: <https://trac.webkit.org/changeset/263633>

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

[Ryan Haddad]

Reverted r263633, r263651, and r263671 for reason:

Still seeing MediaRecorder test crashes after re-landing r263633

Committed r263854: <https://trac.webkit.org/changeset/263854>

[Ryan Haddad]

(In reply to Ryan Haddad from comment #14)
> Reverted r263633, r263651, and r263671 for reason:
> 
> Still seeing MediaRecorder test crashes after re-landing r263633
Details in radar.

[youenn fablet]

Created attachment 403447 [details]
Patch for relanding

[EWS]

Committed r263891: <https://trac.webkit.org/changeset/263891>

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