Summary: | MediaRecorder.start() Method is Ignoring the "timeslice" Parameter | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zachariah Moreno <zach> | ||||||||
Component: | Media | Assignee: | youenn fablet <youennf> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Major | CC: | darin, eric.carlson, ews-watchlist, glenn, jer.noble, Lawrence.j, majo, mark, naicuoctavian, philipj, sergio, webkit-bug-importer, youennf, yuhan_wu | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari Technology Preview | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 85851 | ||||||||||
Attachments: |
|
Description
Zachariah Moreno
2019-09-25 13:33:56 PDT
We are aware that the "timeslice" parameter doesn't work yet, but we plan to implement it. Thanks for the detailed bug report! Eric, when's the MediaRecorder API planned to be pulled out of experimental and into production? (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 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. Created attachment 402801 [details]
Patch
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? (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... 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. Committed r263565: <https://trac.webkit.org/changeset/263565> All reviewed patches have been landed. Closing bug and clearing flags on attachment 402801 [details]. Reverted r263511, r263514, and r263565 for reason: r263511 caused MediaRecorder test crashes on internal testers. Committed r263588: <https://trac.webkit.org/changeset/263588> Created attachment 403041 [details]
Relanding
*** Bug 190291 has been marked as a duplicate of this bug. *** Committed r263651: <https://trac.webkit.org/changeset/263651> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403041 [details]. 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? Reverted r263633, r263651, and r263671 for reason: Still seeing MediaRecorder test crashes after re-landing r263633 Committed r263854: <https://trac.webkit.org/changeset/263854> (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. Committed r263892: <https://trac.webkit.org/changeset/263892> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403041 [details]. 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. (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 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 ? (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 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 (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" |