Summary: | [GStreamer][MSE] Playback rate update support | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | kpect | ||||
Component: | Media | Assignee: | Philippe Normand <pnormand> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Minor | CC: | aboya, calvaris, cgarcia, eocanha, eric.carlson, erusan, ews-watchlist, glenn, gustavo, jer.noble, menard, philipj, pnormand, sergio, vjaquez, webkit-bug-importer, webkit | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Local Build | ||||||
Hardware: | PC | ||||||
OS: | Other | ||||||
Attachments: |
|
Description
kpect
2020-03-02 11:00:42 PST
This is overridden as unimplemented in the MSE player, I currently see no reason for such override. Created attachment 393475 [details]
Patch
I'm open to give an r+ but I would prefer Alicia has a look at this as well. Comment on attachment 393475 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393475&action=review I hope some day I can finally get playbin3 and the new WebKitMediaSrc working and landed and I can get rid of most of this madness with seeks... but that is an ongoing rabbit hole (currently working on fixing WebKitWebSrc races to be able to debug playbin3 player races to be able to use playbin3 for MSE). With that in mind, if this passes tests, LGTM. > Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:248 > + // FIXME: Make a copy here because in some cases below it is modified. This > + // sounds like a bad idea and should be investigated further. > + MediaTime seekTime = position; Could you elaborate? View in context: https://bugs.webkit.org/attachment.cgi?id=393475&action=review I hope some day I can finally get playbin3 and the new WebKitMediaSrc working and landed and I can get rid of most of this madness with seeks... but that is an ongoing rabbit hole (currently working on fixing WebKitWebSrc races to be able to debug playbin3 player races to be able to use playbin3 for MSE). With that in mind, if this passes tests, LGTM. > Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:248 > + // FIXME: Make a copy here because in some cases below it is modified. This > + // sounds like a bad idea and should be investigated further. > + MediaTime seekTime = position; Could you elaborate? Comment on attachment 393475 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393475&action=review >> Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp:248 >> + MediaTime seekTime = position; > > Could you elaborate? the seek time is updated there: https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp#L315 Comment on attachment 393475 [details]
Patch
The only thing I'd say is that it would be better if you split in two, one for the debug and the other one to actually fix the problem but I won't object if you want to land it like this.
Committed r258542: <https://trac.webkit.org/changeset/258542> *** Bug 209386 has been marked as a duplicate of this bug. *** *** Bug 190844 has been marked as a duplicate of this bug. *** |