135802 2014-08-11 09:37:00 -0700 [GStreamer] playback rate is rounded to integer 2014-08-12 09:32:25 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 fvallee webkit-unassigned commit-queue pnormand oldest_to_newest 1028200 0 fvallee 2014-08-11 09:37:00 -0700 tested with webkitgtk (Changeset 172150) w/ linux(64Bit) regression seems to come from http://trac.webkit.org/changeset/163871 The MediaPlayerPrivateGstreamer implementation is rounding (to integer) the playback rate, therefore slow motion is not working. e.g. set playback rate to 0.5 will actually change to rate to 0. The issue can be tested using very basic page such as : http://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_av_prop_playbackrate With firefox / chromium, if we start the video then click on "Set video to play in slow motion", it will actually play slow motion (rate = 0.5) With webkit-gtk, video is paused (even if reported playback rate is 0.5). The issue comes from clampTo (template) function void MediaPlayerPrivateGStreamer::setRate(float rate) { // Higher rate causes crash. rate = clampTo(rate, -20, 20); -20, 20 are integers, therefore I guess returned value is also an integer - based on MathExtras.h anyway clampTo(0.5, -20, 20) returns 0 The issue can be fixed using float-friendly clampTo version: rate = clampTo(rate, -20.0, 20.0); (clampTo(0.5, -20.0, 20.0) returns 0.5 as expected) 1028222 1 pnormand 2014-08-11 10:46:28 -0700 Thanks for investigating this issue! Would you mind providing a patch? The whole procedure is explained there: http://www.webkit.org/coding/contributing.html Handy documentation if it's the first WebKit patch you're contributing. 1028462 2 236431 fvallee 2014-08-12 01:39:05 -0700 Created attachment 236431 Patch 1028463 3 236431 pnormand 2014-08-12 01:42:15 -0700 Comment on attachment 236431 Patch Thanks! 1028467 4 fvallee 2014-08-12 01:51:31 -0700 I think I can add a regression test, the wrong playback rate cannot be detected in javascript but I could write a simple test check to if we reach EOS while in slow motion (e.g.playRate = 0.5) If the actual gstreamer rate is 0 we will never get the ended event. What do you think ? 1028470 5 pnormand 2014-08-12 02:24:38 -0700 (In reply to comment #4) > I think I can add a regression test, the wrong playback rate cannot be detected in javascript but I could write a simple test check to if we reach EOS while in slow motion (e.g.playRate = 0.5) > If the actual gstreamer rate is 0 we will never get the ended event. > What do you think ? Hum yeah that could work indeed. Before setting the playback rate I advise to perform a seek near the end of the file so the test runs faster :) 1028495 6 236440 fvallee 2014-08-12 04:34:52 -0700 Created attachment 236440 Patch 1028496 7 fvallee 2014-08-12 04:37:29 -0700 FYI LayoutTests/media/video-ended-event-slow-motion-playback.html is based on LayoutTests/media/video-ended-event-negative-playback.html, but position is set near the end of the media and playback rate is set to 0.5 (and loop attribute is removed) 1028509 8 pnormand 2014-08-12 08:28:43 -0700 If you can't land this patch yourself we can use the commit-queue bot. Let me know :) 1028511 9 236440 commit-queue 2014-08-12 08:44:15 -0700 Comment on attachment 236440 Patch Rejecting attachment 236440 from commit-queue. fvallee@connected-labs.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights. 1028512 10 fvallee 2014-08-12 08:49:53 -0700 seems I can't land it myself ;) patch has "commit-queue?" flag now. Thanks for the review anyway 1028523 11 236440 commit-queue 2014-08-12 09:32:22 -0700 Comment on attachment 236440 Patch Clearing flags on attachment: 236440 Committed r172472: <http://trac.webkit.org/changeset/172472> 1028524 12 commit-queue 2014-08-12 09:32:25 -0700 All reviewed patches have been landed. Closing bug. 236431 2014-08-12 01:39:05 -0700 2014-08-12 04:34:45 -0700 Patch bug-135802-20140812103854.patch text/plain 1619 fvallee ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBhODliOWYxNjg5NzAxMzBiY2RhYzRhMDBiY2YwYTdjZWYxYmEwNDUxLi42 MjhkNzE0NDllOTY3NzRiMDlmZjQyMmYxZjkyZTNjYjJlMjBhN2Y5IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTcgQEAKKzIwMTQtMDgtMTIgIEZhYmllbiBWYWxsw6llICA8ZnZhbGxlZUBjb25uZWN0 ZWQtbGFicy5jb20+CisKKyAgICAgICAgW0dTdHJlYW1lcl0gcGxheWJhY2sgcmF0ZSBpcyByb3Vu ZGVkIHRvIGludGVnZXIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTEzNTgwMgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIE5vIG5ldyB0ZXN0cyBzaW5jZSB0aGUgaXNzdWUgY2Fubm90IGJlIGRldGVjdGVkCisg ICAgICAgIGluIGphdmFzY3JpcHQgKGJ1dCBhIHBpeGVsIHRlc3QgY291bGQgYmUgdXNlZCB0bwor ICAgICAgICBlbnN1cmUgdmlkZW8gcGxheWJhY2sgaXMgY29ycmVjdCkKKworICAgICAgICAqIHBs YXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuY3Bw OgorICAgICAgICAoV2ViQ29yZTo6TWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyOjpzZXRSYXRl KToKKwogMjAxNC0wOC0wNiAgUmFkdSBTdGF2aWxhICA8c3RhdmlsYUBhZG9iZS5jb20+CiAKICAg ICAgICAgUkVHUkVTU0lPTiAocjE2MzM4Mik6IE92ZXJmbG93IGhpZGRlbiBmb3IgaW5uZXIgZWxl bWVudHMgYnJlYWtzIGJsdXJyaW5nCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNwcCBiL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZh dGVHU3RyZWFtZXIuY3BwCmluZGV4IDQyYjFhNzQ0M2UwM2M3ZWQ2MGI0MDBhNzJlNWZlNzVjMmVl ZDI3YmYuLmM2ODczNzA0ZWQ2ZjIzZjg5MWZiMjM4N2U0ZTBmZjgwOGUyMTkyNGYgMTAwNjQ0Ci0t LSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXll clByaXZhdGVHU3RyZWFtZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBo aWNzL2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuY3BwCkBAIC04MDIsNyAr ODAyLDcgQEAgdm9pZCBNZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXI6Om5ld1RleHRTYW1wbGUo KQogdm9pZCBNZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXI6OnNldFJhdGUoZmxvYXQgcmF0ZSkK IHsKICAgICAvLyBIaWdoZXIgcmF0ZSBjYXVzZXMgY3Jhc2guCi0gICAgcmF0ZSA9IGNsYW1wVG8o cmF0ZSwgLTIwLCAyMCk7CisgICAgcmF0ZSA9IGNsYW1wVG8ocmF0ZSwgLTIwLjAsIDIwLjApOwog CiAgICAgLy8gQXZvaWQgdXNlbGVzcyBwbGF5YmFjayByYXRlIHVwZGF0ZS4KICAgICBpZiAobV9w bGF5YmFja1JhdGUgPT0gcmF0ZSkgewo= 236440 2014-08-12 04:34:52 -0700 2014-08-12 09:32:22 -0700 Patch bug-135802-20140812133440.patch text/plain 3926 fvallee ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBhODliOWYxNjg5NzAxMzBiY2RhYzRhMDBiY2YwYTdjZWYxYmEwNDUxLi4w Yjk3ZjdjYTYwOThjMmQzOTg4ODU5M2E0YjVmZTVkZDQ0MjRlNjBhIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTUgQEAKKzIwMTQtMDgtMTIgIEZhYmllbiBWYWxsw6llICA8ZnZhbGxlZUBjb25uZWN0 ZWQtbGFicy5jb20+CisKKyAgICAgICAgW0dTdHJlYW1lcl0gcGxheWJhY2sgcmF0ZSBpcyByb3Vu ZGVkIHRvIGludGVnZXIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTEzNTgwMgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFRlc3Q6IG1lZGlhL3ZpZGVvLWVuZGVkLWV2ZW50LXNsb3ctbW90aW9uLXBsYXliYWNr Lmh0bWwKKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXll clByaXZhdGVHU3RyZWFtZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6TWVkaWFQbGF5ZXJQcml2 YXRlR1N0cmVhbWVyOjpzZXRSYXRlKToKKwogMjAxNC0wOC0wNiAgUmFkdSBTdGF2aWxhICA8c3Rh dmlsYUBhZG9iZS5jb20+CiAKICAgICAgICAgUkVHUkVTU0lPTiAocjE2MzM4Mik6IE92ZXJmbG93 IGhpZGRlbiBmb3IgaW5uZXIgZWxlbWVudHMgYnJlYWtzIGJsdXJyaW5nCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2 YXRlR1N0cmVhbWVyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJl YW1lci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuY3BwCmluZGV4IDQyYjFhNzQ0M2UwM2M3 ZWQ2MGI0MDBhNzJlNWZlNzVjMmVlZDI3YmYuLmM2ODczNzA0ZWQ2ZjIzZjg5MWZiMjM4N2U0ZTBm ZjgwOGUyMTkyNGYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNz L2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuY3BwCisrKyBiL1NvdXJjZS9X ZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZhdGVHU3Ry ZWFtZXIuY3BwCkBAIC04MDIsNyArODAyLDcgQEAgdm9pZCBNZWRpYVBsYXllclByaXZhdGVHU3Ry ZWFtZXI6Om5ld1RleHRTYW1wbGUoKQogdm9pZCBNZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXI6 OnNldFJhdGUoZmxvYXQgcmF0ZSkKIHsKICAgICAvLyBIaWdoZXIgcmF0ZSBjYXVzZXMgY3Jhc2gu Ci0gICAgcmF0ZSA9IGNsYW1wVG8ocmF0ZSwgLTIwLCAyMCk7CisgICAgcmF0ZSA9IGNsYW1wVG8o cmF0ZSwgLTIwLjAsIDIwLjApOwogCiAgICAgLy8gQXZvaWQgdXNlbGVzcyBwbGF5YmFjayByYXRl IHVwZGF0ZS4KICAgICBpZiAobV9wbGF5YmFja1JhdGUgPT0gcmF0ZSkgewpkaWZmIC0tZ2l0IGEv TGF5b3V0VGVzdHMvQ2hhbmdlTG9nIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCmluZGV4IDdkOTQx MDg2MWNiY2FmMjU3Yzg3NmU1ODJlMzA1MmQwNjUyZGNjYWIuLjMzYjk4N2QxYzI2MDg2MmRkNDhl YWNhYjhiMzIzNWI3Mzc2OTg3MWYgMTAwNjQ0Ci0tLSBhL0xheW91dFRlc3RzL0NoYW5nZUxvZwor KysgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAxNC0wOC0xMiAg RmFiaWVuIFZhbGzDqWUgIDxmdmFsbGVlQGNvbm5lY3RlZC1sYWJzLmNvbT4KKworICAgICAgICBb R1N0cmVhbWVyXSBwbGF5YmFjayByYXRlIGlzIHJvdW5kZWQgdG8gaW50ZWdlcgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTM1ODAyCisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgYWRkIHRlc3QgdG8gZW5zdXJl IHBsYXliYWNrIGlzIGFjdHVhbGx5IHdvcmtpbmcKKyAgICAgICAgaW4gc2xvdyBtb3Rpb24gKHBs YXliYWNrIHJhdGUgPSAwLjUpOiBQbGF5ZXIKKyAgICAgICAgc2hvdWxkIHJlYWNoIEVPUyBhbmQg ZGlzcGF0Y2ggZW5kZWQgZXZlbnQuCisKKyAgICAgICAgKiBtZWRpYS92aWRlby1lbmRlZC1ldmVu dC1zbG93LW1vdGlvbi1wbGF5YmFjay1leHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAgICAqIG1l ZGlhL3ZpZGVvLWVuZGVkLWV2ZW50LXNsb3ctbW90aW9uLXBsYXliYWNrLmh0bWw6IEFkZGVkLgor CiAyMDE0LTA4LTA2ICBNaWhuZWEgT3ZpZGVuaWUgIDxtaWhuZWFAYWRvYmUuY29tPgogCiAgICAg ICAgIFtDU1NSZWdpb25zXSBNb3ZlIGZ1bGwgc2NyZWVuIHRlc3RzIGludG8gZmFzdC9yZWdpb25z L2Z1bGxzY3JlZW4KZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL21lZGlhL3ZpZGVvLWVuZGVkLWV2 ZW50LXNsb3ctbW90aW9uLXBsYXliYWNrLWV4cGVjdGVkLnR4dCBiL0xheW91dFRlc3RzL21lZGlh L3ZpZGVvLWVuZGVkLWV2ZW50LXNsb3ctbW90aW9uLXBsYXliYWNrLWV4cGVjdGVkLnR4dApuZXcg ZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwLi45MWRmYTJjMjczMGUxNjhlOGRkOWI5ZWMzOGE3ZTdiNTlkOTdkMjE4Ci0tLSAvZGV2 L251bGwKKysrIGIvTGF5b3V0VGVzdHMvbWVkaWEvdmlkZW8tZW5kZWQtZXZlbnQtc2xvdy1tb3Rp b24tcGxheWJhY2stZXhwZWN0ZWQudHh0CkBAIC0wLDAgKzEsNSBAQAorCitFVkVOVChjYW5wbGF5 dGhyb3VnaCkKK0VWRU5UKGVuZGVkKQorRU5EIE9GIFRFU1QKKwpkaWZmIC0tZ2l0IGEvTGF5b3V0 VGVzdHMvbWVkaWEvdmlkZW8tZW5kZWQtZXZlbnQtc2xvdy1tb3Rpb24tcGxheWJhY2suaHRtbCBi L0xheW91dFRlc3RzL21lZGlhL3ZpZGVvLWVuZGVkLWV2ZW50LXNsb3ctbW90aW9uLXBsYXliYWNr Lmh0bWwKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwMDAwMDAwMC4uMzU5ZjE2NjFlZGViZGUyMGExNWQ2MjMyODkwNzJiNTVjMDI4ZTc1 NAotLS0gL2Rldi9udWxsCisrKyBiL0xheW91dFRlc3RzL21lZGlhL3ZpZGVvLWVuZGVkLWV2ZW50 LXNsb3ctbW90aW9uLXBsYXliYWNrLmh0bWwKQEAgLTAsMCArMSwyNCBAQAorPGh0bWw+CisgICAg PGhlYWQ+CisgICAgICAgIDxzY3JpcHQgc3JjPW1lZGlhLWZpbGUuanM+PC9zY3JpcHQ+CisgICAg ICAgIDxzY3JpcHQgc3JjPXZpZGVvLXRlc3QuanM+PC9zY3JpcHQ+CisgICAgICAgIDxzY3JpcHQ+ CisgICAgICAgICAgICBmdW5jdGlvbiBzdGFydCgpIHsKKyAgICAgICAgICAgICAgICBmaW5kTWVk aWFFbGVtZW50KCk7CisgICAgICAgICAgICAgICAgd2FpdEZvckV2ZW50QW5kRW5kKCdlbmRlZCcp OworICAgICAgICAgICAgICAgIHZpZGVvLnNyYyA9IGZpbmRNZWRpYUZpbGUoInZpZGVvIiwgImNv bnRlbnQvdGVzdCIpOworICAgICAgICAgICAgICAgIHdhaXRGb3JFdmVudE9uY2UoJ2NhbnBsYXl0 aHJvdWdoJywKKyAgICAgICAgICAgICAgICAgICAgZnVuY3Rpb24gKCkKKyAgICAgICAgICAgICAg ICAgICAgeworICAgICAgICAgICAgICAgICAgICAgICAgdmlkZW8uY3VycmVudFRpbWUgPSA1Owor ICAgICAgICAgICAgICAgICAgICAgICAgdmlkZW8ucGxheWJhY2tSYXRlID0gMC41OworICAgICAg ICAgICAgICAgICAgICAgICAgdmlkZW8ucGxheSgpOworICAgICAgICAgICAgICAgICAgICAgICAg ZmFpbFRlc3RJbigyNTAwKTsKKyAgICAgICAgICAgICAgICAgICAgfSk7CisgICAgICAgICAgICAg ICAgfQorICAgICAgICA8L3NjcmlwdD4KKyAgICA8L2hlYWQ+CisgICAgPGJvZHkgb25sb2FkPSJz dGFydCgpIj4KKyAgICAgICAgPHZpZGVvIC8+CisgICAgPC9ib2R5PgorPC9odG1sPgo=