Bug 202233 – MediaRecorder.start() Method is Ignoring the "timeslice" Parameter

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

[Zachariah Moreno]

Created attachment 379577 [details]
test fails in safari 13.0

Thank You for implementing the GetUserMedia & MediaRecorder interfaces!

We developed a test to verify GetUserMedia & MediaRecorder @ https://codepen.io/XachMoreno/pen/bGbeNWX.

The "timeslice" parameter within the MediaRecorder.start() method is being ignored & recording continues beyond the number in milliseconds.

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/XachMoreno/pen/bGbeNWX
 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 5 seconds

If the test FAILS, an alert dialog will open communicating the lack of support for the timeslice parameter.

If the test PASSES, the 5 second timer will reset to 1 & continue recording without an alert.

Spec Reference - https://w3c.github.io/mediacapture-record/#dom-mediarecorder-start

Docs Reference - https://developer.mozilla.org/en-US/docs/Web/API/MediaRecorder/start

[Radar WebKit Bug Importer]

<rdar://problem/55720555>

[Eric Carlson]

We are aware that the "timeslice" parameter doesn't work yet, but we plan to implement it.

Thanks for the detailed bug report!

[octavn]

Eric, when's the MediaRecorder API planned to be pulled out of experimental and into production?

[Zachariah Moreno]

(In reply to Eric Carlson from comment #2)
> We are aware that the "timeslice" parameter doesn't work yet, but we plan to
> implement it.
> 
> Thanks for the detailed bug report!

Thank You Eric! Do timelines exist for...
 1. timeslice support?
 2. graduation from Experimental to Stable Feature?

I have started working on the bug

[youenn fablet]

requestData should work as expected soon in WebKit ToT after https://bugs.webkit.org/show_bug.cgi?id=206929.

To implement the time slice parameter, the easiest might be to create a repeating timer, see Timer.h.

It would be interesting to know whether calling requestData is changing how time slice schedules fetches in other browsers implementing MediaRecorder.

We could either go with Timer::startRepeating, or have a one shot timer that is started in MediaRecorder::start and rescheduled in MediaRecorder::requestData lambda.

[youenn fablet]

Created attachment 402801 [details]
Patch

[Eric Carlson]

Comment on attachment 402801 [details]
Patch

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

> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:129
> +ExceptionOr<void> MediaRecorder::startRecording(Optional<int> timeSlice)

`timeSlice` is `optional unsigned long` in the spec. Does it matter that this uses a signed int?

[youenn fablet]

(In reply to Eric Carlson from comment #8)
> Comment on attachment 402801 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402801&action=review
> 
> > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:129
> > +ExceptionOr<void> MediaRecorder::startRecording(Optional<int> timeSlice)
> 
> `timeSlice` is `optional unsigned long` in the spec. Does it matter that
> this uses a signed int?

Ah, let's change the WebIDL and timeSlice signature as a follow-up.
It does not make sense to use a signed value here.
WPT media recorder tests are not very comprehensive...

[Darin Adler]

Comment on attachment 402801 [details]
Patch

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

>>> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:129
>>> +ExceptionOr<void> MediaRecorder::startRecording(Optional<int> timeSlice)
>> 
>> `timeSlice` is `optional unsigned long` in the spec. Does it matter that this uses a signed int?
> 
> Ah, let's change the WebIDL and timeSlice signature as a follow-up.
> It does not make sense to use a signed value here.
> WPT media recorder tests are not very comprehensive...

Keep in mind that "unsigned long" in IDL corresponds to "unsigned" in C++ and "long" corresponds to "int". Although if we use the "wrong" type in C++ the symptom is mild.

[EWS]

Committed r263565: <https://trac.webkit.org/changeset/263565>

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

[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]

Created attachment 403041 [details]
Relanding

[youenn fablet]

*** Bug 190291 has been marked as a duplicate of this bug. ***

[EWS]

Committed r263651: <https://trac.webkit.org/changeset/263651>

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

[Darin Adler]

Comment on attachment 403041 [details]
Relanding

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

> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:188
> +    if (m_timeSliceTimer.isActive())
> +        m_timeSliceTimer.stop();

Is this more efficient than unconditionally calling stop?

[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>

[youenn fablet]

(In reply to Darin Adler from comment #16)
> Comment on attachment 403041 [details]
> Relanding
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403041&action=review
> 
> > Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:188
> > +    if (m_timeSliceTimer.isActive())
> > +        m_timeSliceTimer.stop();
> 
> Is this more efficient than unconditionally calling stop?

I think so, stop is unconditionally setting m_repeatInterval and calling setNextFireTime.

[EWS]

Committed r263892: <https://trac.webkit.org/changeset/263892>

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

[octavn]

The timeslice feature in Safari 14.0.2 on macOS is broken. It produces an.mp4 file that's corrupt and won't play in VLC or Safari. ffmpeg and ffprobe will not properly process the file.

[youenn fablet]

(In reply to Octavian Naicu from comment #20)
> The timeslice feature in Safari 14.0.2 on macOS is broken. It produces
> an.mp4 file that's corrupt and won't play in VLC or Safari. ffmpeg and
> ffprobe will not properly process the file.

Does it reproduce on the latest Safari Tech Preview?
Can you provide repro steps or a jsfiddle?

https://jsfiddle.net/hdwc2xzL/ seems to work for me:
- Load page
- grant access
- wait 10 seconds
- video should start playing
- download link should allow to download an mp4 file that plays in VLC locally

[octavn]

The jsfiddle you linked to seems to be working on 14.0.2. I'll double check my code on Monday.


(In reply to youenn fablet from comment #21)
> (In reply to Octavian Naicu from comment #20)
> > The timeslice feature in Safari 14.0.2 on macOS is broken. It produces
> > an.mp4 file that's corrupt and won't play in VLC or Safari. ffmpeg and
> > ffprobe will not properly process the file.
> 
> Does it reproduce on the latest Safari Tech Preview?
> Can you provide repro steps or a jsfiddle?
> 
> https://jsfiddle.net/hdwc2xzL/ seems to work for me:
> - Load page
> - grant access
> - wait 10 seconds
> - video should start playing
> - download link should allow to download an mp4 file that plays in VLC
> locally

Youenn Fablet wrote|: (in webkit-dev)
"On the most recent BigSur and iOS versions, MediaRecorder should be enabled by default and timeslice should be supported."

I just installed iOS 14.4 and MediaRecorder is still listed in 'experimental' features.

It's turned ON, but I had it turned on before I upgraded, so its not clear if a fresh end user would be able to use MediaRecorder.  There does not appear to be a 'reset defaults' so I can't really determine the actual status.

Instinct says that if its still an "experimental" feature maybe its not there by default ?

Could anyone point me to a changelog which might clarify this question ?

[Eric Carlson]

(In reply to mark from comment #23)
> Youenn Fablet wrote|: (in webkit-dev)
> "On the most recent BigSur and iOS versions, MediaRecorder should be enabled
> by default and timeslice should be supported."
> 
> I just installed iOS 14.4 and MediaRecorder is still listed in
> 'experimental' features.
> 
> It's turned ON, but I had it turned on before I upgraded, so its not clear
> if a fresh end user would be able to use MediaRecorder.  There does not
> appear to be a 'reset defaults' so I can't really determine the actual
> status.
> 
> Instinct says that if its still an "experimental" feature maybe its not
> there by default ?
> 
> Could anyone point me to a changelog which might clarify this question ?

https://bugs.webkit.org/show_bug.cgi?id=216664 and https://bugs.webkit.org/show_bug.cgi?id=216663

[octavn]

The timeslice feature in Safari 14.0.2 on macOS seems to work as expected. I can't reproduce the issue anymore. It seems I do not know how to force refresh in Safari.

(In reply to Octavian Naicu from comment #22)
> The jsfiddle you linked to seems to be working on 14.0.2. I'll double check
> my code on Monday.
> 
> 
> (In reply to youenn fablet from comment #21)
> > (In reply to Octavian Naicu from comment #20)
> > > The timeslice feature in Safari 14.0.2 on macOS is broken. It produces
> > > an.mp4 file that's corrupt and won't play in VLC or Safari. ffmpeg and
> > > ffprobe will not properly process the file.
> > 
> > Does it reproduce on the latest Safari Tech Preview?
> > Can you provide repro steps or a jsfiddle?
> > 
> > https://jsfiddle.net/hdwc2xzL/ seems to work for me:
> > - Load page
> > - grant access
> > - wait 10 seconds
> > - video should start playing
> > - download link should allow to download an mp4 file that plays in VLC
> > locally

[Darin Adler]

(In reply to Octavian Naicu from comment #25)
> It seems I do not know how to force refresh in Safari.

One way is Command-Option-R, "Reload Page from Origin"