Bug 42178 – Update media element's seeking logic

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Eric Carlson]

The logic in HTMLMediaElement::seek is out of sync with the current spec.

[Eric Carlson]

<rdar://problem/8185817>

[Eric Carlson]

Created attachment 61413 [details]
WIP patch

[Eric Carlson]

Created attachment 61416 [details]
proposed patch.

[Darin Adler]

Comment on attachment 61416 [details]
proposed patch.

> +        if (abs(startTime - time) < closest)
> +            closest = abs(startTime - time);
> +        else if (abs(endTime - time) < closest)
> +            closest = abs(endTime - time);

The function used here should be fabsf, not abs. Calling abs converts the number to an int!

Also, I am concerned about the ignored ExceptionCode. Why is it OK to ignore it? Can there be an exception.

r=me but please fix that fabsf thing

[Eric Carlson]

(In reply to comment #4)
> (From update of attachment 61416 [details])
> > +        if (abs(startTime - time) < closest)
> > +            closest = abs(startTime - time);
> > +        else if (abs(endTime - time) < closest)
> > +            closest = abs(endTime - time);
> 
> The function used here should be fabsf, not abs. Calling abs converts the number to an int!
> 
Ack, I assume it would use the "inline float" version in cmath. Thanks!

> Also, I am concerned about the ignored ExceptionCode. Why is it OK to ignore it? Can there be an exception.
> 
An exception is returned by start() and end() if the index is out of bounds, which can't happen here.

> r=me but please fix that fabsf thing
Thanks!

[Eric Carlson]

http://trac.webkit.org/changeset/63319

[WebKit Review Bot]

http://trac.webkit.org/changeset/63319 might have broken Qt Linux Release minimal

[Jakub Wieczorek]

(In reply to comment #6)
> http://trac.webkit.org/changeset/63319

Are you sure this is correct? Aren't you returning the distance between time and the nearest position rather than the nearest position?

And this:
+ else if (fabs(endTime - time) < closest)
looks wrong. This shouldn't be an else if, endTime might be closer than startTime.

You could save some function calls and unnecessary input checks by operating on the private vector instead of calling the public functions.