Bug 129926 – Add Remote Control command support to HTMLMediaElement

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

[Jer Noble]

Add Remote Control command support to HTLMediaElement

[Jer Noble]

Created attachment 226172 [details]
Patch

[Eric Carlson]

Comment on attachment 226172 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=226172&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:3059
> +    } else
> +        setPlaybackRate(nextScanRate());
> +}

Do we really want to increase the scan rate to infinity (and beyond)?

> Source/WebCore/html/HTMLMediaElement.cpp:5937
> +        { } // Do nothing

Nit: I don't think "{ }" is necessary, won't a semi-colon work?

[Jer Noble]

(In reply to comment #2)
> (From update of attachment 226172 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226172&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.cpp:3059
> > +    } else
> > +        setPlaybackRate(nextScanRate());
> > +}
> 
> Do we really want to increase the scan rate to infinity (and beyond)?

We'll max out at ScanMaximumRate (8x) inside of nextScanRate().

> > Source/WebCore/html/HTMLMediaElement.cpp:5937
> > +        { } // Do nothing
> 
> Nit: I don't think "{ }" is necessary, won't a semi-colon work?

Clang++ complains about only a semicolon, and explicitly suggests "{ }" for an empty statement.

[Eric Carlson]

Comment on attachment 226172 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=226172&action=review

>>> Source/WebCore/html/HTMLMediaElement.cpp:3059
>>> +}
>> 
>> Do we really want to increase the scan rate to infinity (and beyond)?
> 
> We'll max out at ScanMaximumRate (8x) inside of nextScanRate().

Oops, I looked right past that!

[Jer Noble]

Committed r165628: <http://trac.webkit.org/changeset/165628>