[webkit-reviews] review denied: [Bug 128453] [GStreamer] High playback rate causes crash : [Attachment 223563] Patch

[bugzilla-daemon at webkit.org]

Philippe Normand <pnormand at igalia.com> has denied Piotr Grad
<p.grad at samsung.com>'s request for review:
Bug 128453: [GStreamer] High playback rate causes crash
https://bugs.webkit.org/show_bug.cgi?id=128453

Attachment 223563: Patch
https://bugs.webkit.org/attachment.cgi?id=223563&action=review

------- Additional Comments from Philippe Normand <pnormand at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=223563&action=review


> LayoutTests/ChangeLog:3
> +	   [GStreamer] High playback rate causes crash

Have you investigated that crash? Might be worth a GStreamer bug report.

> LayoutTests/media/video-extreme-playbackrate-crash.html:15
> +		   video.playbackRate = 10000000;

The spec (afaik) doesn't define the range of valid playbackRate values. So I
don't think we should have a test for this, what do you think Eric?

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:866
> +    // Higher rate causes crash.

This is quite high indeed... I don't think these values make sense from a user
point of view, limiting to (-10, 10) would perhaps be a more reasonable thing
to do?

And please use clampTo() from MathExtras.h