Bug 135802 - [GStreamer] playback rate is rounded to integer
Summary: [GStreamer] playback rate is rounded to integer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-11 09:37 PDT by Fabien Vallée
Modified: 2014-08-12 09:32 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.58 KB, patch)
2014-08-12 01:39 PDT, Fabien Vallée
no flags Details | Formatted Diff | Diff
Patch (3.83 KB, patch)
2014-08-12 04:34 PDT, Fabien Vallée
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fabien Vallée 2014-08-11 09:37:00 PDT
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)
Comment 1 Philippe Normand 2014-08-11 10:46:28 PDT
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.
Comment 2 Fabien Vallée 2014-08-12 01:39:05 PDT
Created attachment 236431 [details]
Patch
Comment 3 Philippe Normand 2014-08-12 01:42:15 PDT
Comment on attachment 236431 [details]
Patch

Thanks!
Comment 4 Fabien Vallée 2014-08-12 01:51:31 PDT
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 ?
Comment 5 Philippe Normand 2014-08-12 02:24:38 PDT
(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 :)
Comment 6 Fabien Vallée 2014-08-12 04:34:52 PDT
Created attachment 236440 [details]
Patch
Comment 7 Fabien Vallée 2014-08-12 04:37:29 PDT
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)
Comment 8 Philippe Normand 2014-08-12 08:28:43 PDT
If you can't land this patch yourself we can use the commit-queue bot. Let me know :)
Comment 9 WebKit Commit Bot 2014-08-12 08:44:15 PDT
Comment on attachment 236440 [details]
Patch

Rejecting attachment 236440 [details] 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.
Comment 10 Fabien Vallée 2014-08-12 08:49:53 PDT
seems I can't land it myself ;)
patch has "commit-queue?" flag now.
Thanks for the review anyway
Comment 11 WebKit Commit Bot 2014-08-12 09:32:22 PDT
Comment on attachment 236440 [details]
Patch

Clearing flags on attachment: 236440

Committed r172472: <http://trac.webkit.org/changeset/172472>
Comment 12 WebKit Commit Bot 2014-08-12 09:32:25 PDT
All reviewed patches have been landed.  Closing bug.