Summary: | Update PlatformTimeRanges to use MediaTime rather than doubles for time values. | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> | ||||||||||||||||||||||
Component: | New Bugs | Assignee: | Jer Noble <jer.noble> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | agomez, buildbot, calvaris, cgarcia, commit-queue, eric.carlson, esprehn+autocc, glenn, gustavo, gyuyoung.kim, menard, mrobinson, philipj, pnormand, rniwa, sergio, vjaquez | ||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||
Attachments: |
|
Description
Jer Noble
2014-06-02 11:16:22 PDT
Created attachment 232395 [details]
Patch
Created attachment 232397 [details]
Patch
Comment on attachment 232397 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232397&action=review > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1404 > + MediaTime currentTime = MediaTime::createWithDouble(m_source->currentTime()); > const PlatformTimeRanges& ranges = m_buffered->ranges(); > if (!ranges.length()) > return false; Nit: no need to create "currentTime" if ranges is empty. > Source/WebCore/platform/graphics/PlatformTimeRanges.h:74 > + Range(MediaTime start, MediaTime end) Nit: any reason to not make these "const MediaTime&"? > Source/WebCore/platform/graphics/PlatformTimeRanges.h:82 > + inline bool isPointInRange(MediaTime point) const Ditto. Created attachment 232399 [details]
Patch
(In reply to comment #3) > (From update of attachment 232397 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=232397&action=review > > > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1404 > > + MediaTime currentTime = MediaTime::createWithDouble(m_source->currentTime()); > > const PlatformTimeRanges& ranges = m_buffered->ranges(); > > if (!ranges.length()) > > return false; > > Nit: no need to create "currentTime" if ranges is empty. I'll reverse the order. > > Source/WebCore/platform/graphics/PlatformTimeRanges.h:74 > > + Range(MediaTime start, MediaTime end) > > Nit: any reason to not make these "const MediaTime&"? Nope, no reason. > > Source/WebCore/platform/graphics/PlatformTimeRanges.h:82 > > + inline bool isPointInRange(MediaTime point) const > > Ditto. Ditto. Created attachment 232400 [details]
Patch
Comment on attachment 232400 [details] Patch Attachment 232400 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5635521531346944 New failing tests: media/video-pause-immediately.html Created attachment 232404 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Created attachment 232407 [details]
Patch
Created attachment 232408 [details]
Patch
Updated test to allow for very small deltas between the end buffered time and the end played time.
Created attachment 232409 [details]
Patch
Created attachment 232414 [details]
Patch
Created attachment 232432 [details]
Patch
Committed r169568: <http://trac.webkit.org/changeset/169568> |