Bug 133579 – Refactoring: make MediaTime the primary time type for audiovisual times.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Refactoring: make MediaTime the primary time type for audiovisual times.

[Jer Noble]

Created attachment 232703 [details]
Patch

[Jer Noble]

Created attachment 232710 [details]
Patch

[Jer Noble]

Created attachment 232740 [details]
Patch

[Build Bot]

Comment on attachment 232740 [details]
Patch

Attachment 232740 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5139758757969920

New failing tests:
media/media-controller-playbackrate.html
media/media-fragments/TC0014.html
media/track/track-cues-pause-on-exit.html
media/media-fragments/TC0011.html
media/audio-data-url.html
http/tests/media/media-play-stream-chunked-icy.html
http/tests/media/reload-after-dialog.html
media/audio-mpeg4-supported.html
media/media-fragments/TC0006.html
media/media-fragments/TC0004.html
media/audio-mpeg-supported.html
media/track/track-cues-cuechange.html
media/track/track-cues-missed.html
http/tests/media/video-error-abort.html
http/tests/media/video-buffered-range-contains-currentTime.html
media/track/track-cue-nothing-to-render.html
http/tests/media/video-error-does-not-exist.html
media/track/track-cues-sorted-before-dispatch.html
media/media-fragments/TC0024.html
media/track/track-cues-seeking.html
media/track/track-cue-rendering-rtl.html
media/controls-drag-timebar.html
media/media-fragments/TC0015.html
media/media-fragments/TC0017.html
media/track/track-cues-enter-exit.html
plugins/quicktime-plugin-replacement.html
media/media-fragments/TC0005.html
media/media-fragments/TC0009.html
media/media-continues-playing-after-replace-source.html

[Build Bot]

Created attachment 232755 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 232740 [details]
Patch

Attachment 232740 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6613533835919360

New failing tests:
media/media-controller-playbackrate.html
media/media-fragments/TC0014.html
media/track/track-cues-pause-on-exit.html
media/media-fragments/TC0011.html
media/audio-data-url.html
media/audio-concurrent-supported.html
http/tests/media/media-play-stream-chunked-icy.html
http/tests/media/reload-after-dialog.html
media/audio-mpeg4-supported.html
media/media-fragments/TC0006.html
media/media-fragments/TC0004.html
media/media-ended.html
media/audio-mpeg-supported.html
media/track/track-cues-cuechange.html
media/track/track-cues-missed.html
http/tests/media/video-error-abort.html
http/tests/media/video-buffered-range-contains-currentTime.html
media/track/track-cue-nothing-to-render.html
http/tests/media/video-error-does-not-exist.html
media/track/track-cues-sorted-before-dispatch.html
media/media-fragments/TC0024.html
media/track/track-cues-seeking.html
media/track/track-cue-rendering-rtl.html
media/media-fragments/TC0015.html
media/media-fragments/TC0017.html
media/track/track-cues-enter-exit.html
plugins/quicktime-plugin-replacement.html
media/media-fragments/TC0005.html
media/media-fragments/TC0009.html
media/media-continues-playing-after-replace-source.html

[Build Bot]

Created attachment 232761 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 232740 [details]
Patch

Attachment 232740 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4993609610821632

New failing tests:
http/tests/media/reload-after-dialog.html
media/media-fragments/TC0014.html
media/media-fragments/TC0011.html
media/audio-data-url.html
media/audio-concurrent-supported.html
http/tests/media/media-play-stream-chunked-icy.html
media/audio-mpeg4-supported.html
media/media-fragments/TC0006.html
media/media-fragments/TC0004.html
media/audio-mpeg-supported.html
media/track/track-cues-cuechange.html
media/track/track-cues-missed.html
http/tests/media/video-buffered-range-contains-currentTime.html
media/track/track-cue-nothing-to-render.html
http/tests/media/video-error-does-not-exist.html
http/tests/media/video-error-abort.html
media/track/track-cue-rendering-rtl.html
media/media-fragments/TC0015.html
media/track/track-cues-enter-exit.html
plugins/quicktime-plugin-replacement.html
media/media-fragments/TC0005.html
media/media-fragments/TC0009.html
media/media-continues-playing-after-replace-source.html

[Build Bot]

Created attachment 232765 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Jer Noble]

Created attachment 232832 [details]
Patch

[Jer Noble]

Created attachment 232839 [details]
Patch

[Build Bot]

Comment on attachment 232839 [details]
Patch

Attachment 232839 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5914462947115008

New failing tests:
media/video-fast-seek.html
media/audio-data-url.html
media/sources-fallback-codecs.html
media/controls-drag-timebar.html
media/video-seek-past-end-paused.html

[Build Bot]

Created attachment 232847 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 232839 [details]
Patch

Attachment 232839 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4868607272026112

New failing tests:
media/video-fast-seek.html
media/video-seek-past-end-paused.html
media/audio-data-url.html
media/sources-fallback-codecs.html
media/controls-drag-timebar.html

[Build Bot]

Created attachment 232874 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Jer Noble]

Created attachment 232883 [details]
Patch

[Build Bot]

Comment on attachment 232883 [details]
Patch

Attachment 232883 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5058565085593600

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 232899 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 232883 [details]
Patch

Attachment 232883 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4546905631621120

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 232905 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 232883 [details]
Patch

Attachment 232883 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6346548468252672

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 232916 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Jer Noble]

Created attachment 233364 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 233364 [details]
Patch

Am I missing anything or you messed up the changelogs?

[Jer Noble]

(In reply to comment #24)
> (From update of attachment 233364 [details])
> Am I missing anything or you messed up the changelogs?

I may have messed up the ChangeLogs. :)

[Eric Carlson]

Comment on attachment 233364 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:15
> +        * runtime/JSCJSValue.h:
> +        (JSC::jsNumber):
> +
> [phone]  Jer Noble  <[e-mail]>
> +
> +        Refactoring: make MediaTime the primary time type for audiovisual times.

Double entry?

> Source/WTF/ChangeLog:15
> [phone]  Jer Noble  <[e-mail]>

Ditto.

> Source/WebCore/ChangeLog:9
> +        In order to limit the number of floating-point rounding errors for media systems which
> +        can make use of rational time objects.

Nit: This should be a complete sentence.

> Source/WebCore/ChangeLog:259
> [phone]  Jer Noble  <[e-mail]>

Another double ChangeLog.

> Source/WebCore/ChangeLog:517
>  2014-06-16  Jer Noble  <[e-mail]>

Is this yet another entry, of is diff just confused?

> Source/WebCore/html/HTMLMediaElement.cpp:1654
> +    RefPtr<TextTrackCue> cue = prpCue;

:-O

[Jer Noble]

(In reply to comment #26)
> (From update of attachment 233364 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=233364&action=review
> 
> > Source/JavaScriptCore/ChangeLog:15
> > +        * runtime/JSCJSValue.h:
> > +        (JSC::jsNumber):
> > +
> > [phone]  Jer Noble  <[e-mail]>
> > +
> > +        Refactoring: make MediaTime the primary time type for audiovisual times.
> 
> Double entry?

Yes, my last rebase apparently screwed up all my change logs. :)

> > Source/WTF/ChangeLog:15
> > [phone]  Jer Noble  <[e-mail]>
> 
> Ditto.
> 
> > Source/WebCore/ChangeLog:9
> > +        In order to limit the number of floating-point rounding errors for media systems which
> > +        can make use of rational time objects.
> 
> Nit: This should be a complete sentence.

Ok.

> > Source/WebCore/ChangeLog:259
> > [phone]  Jer Noble  <[e-mail]>
> 
> Another double ChangeLog.
> 
> > Source/WebCore/ChangeLog:517
> >  2014-06-16  Jer Noble  <[e-mail]>
> 
> Is this yet another entry, of is diff just confused?

Probably yet another entry.

> > Source/WebCore/html/HTMLMediaElement.cpp:1654
> > +    RefPtr<TextTrackCue> cue = prpCue;
> 
> :-O

Yeah, I might just pull this one out into its own bug. :-/

[Jer Noble]

Created attachment 233469 [details]
Patch for landing

[Jer Noble]

(In reply to comment #27)
> (In reply to comment #26)
> > (From update of attachment 233364 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=233364&action=review
> > > Source/WebCore/ChangeLog:9
> > > +        In order to limit the number of floating-point rounding errors for media systems which
> > > +        can make use of rational time objects.
> > 
> > Nit: This should be a complete sentence.
> 
> Ok.

Whoops, missed this edit in the most recent patch. Will fix before landing.

[Build Bot]

Comment on attachment 233469 [details]
Patch for landing

Attachment 233469 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5585821847519232

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 233494 [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

[Build Bot]

Comment on attachment 233469 [details]
Patch for landing

Attachment 233469 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6174618914127872

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 233499 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 233469 [details]
Patch for landing

Attachment 233469 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5948311718592512

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 233503 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 233469 [details]
Patch for landing

Attachment 233469 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6738000511762432

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 233507 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Jer Noble]

Created attachment 237104 [details]
Patch for landing

[Jer Noble]

Created attachment 237109 [details]
Patch for landing

[Build Bot]

Comment on attachment 237104 [details]
Patch for landing

Attachment 237104 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5392739839508480

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237114 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 237104 [details]
Patch for landing

Attachment 237104 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5084812159221760

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237118 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 237104 [details]
Patch for landing

Attachment 237104 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6612633763446784

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237129 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Jer Noble]

Created attachment 237177 [details]
Patch for landing

This should take care of the mountainlion test errors.

[Build Bot]

Comment on attachment 237177 [details]
Patch for landing

Attachment 237177 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6070680294522880

New failing tests:
http/tests/media/remove-while-loading.html
http/tests/media/video-cancel-load.html
media/sources-fallback-codecs.html
http/tests/media/video-accept-encoding.html
http/tests/media/video-served-as-text.html
media/audio-data-url.html

[Build Bot]

Created attachment 237190 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 237177 [details]
Patch for landing

Attachment 237177 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5712001938489344

New failing tests:
http/tests/media/remove-while-loading.html
http/tests/media/video-cancel-load.html
media/sources-fallback-codecs.html
http/tests/media/video-accept-encoding.html
http/tests/media/video-throttled-load-metadata.html
http/tests/media/video-served-as-text.html
media/audio-data-url.html

[Build Bot]

Created attachment 237197 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Jer Noble]

Created attachment 237359 [details]
Patch for landing

One more try to fix ML tests.

[Jer Noble]

Created attachment 237363 [details]
Patch for landing

And a fix to the GTK EWS.

[Jer Noble]

Created attachment 237371 [details]
Patch for landing

And a fix to the win bot introduced by an unresolved merge conflict.

[Build Bot]

Comment on attachment 237371 [details]
Patch for landing

Attachment 237371 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6504654259093504

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237394 [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

[Build Bot]

Comment on attachment 237371 [details]
Patch for landing

Attachment 237371 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4695375126462464

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237400 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 237371 [details]
Patch for landing

Attachment 237371 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4728079691808768

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237404 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 237371 [details]
Patch for landing

Attachment 237371 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6304209746001920

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237405 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Jer Noble]

Created attachment 237529 [details]
Patch for landing

Add the failing mountainlion results to platform and fix them in a future patch.

[Build Bot]

Comment on attachment 237529 [details]
Patch for landing

Attachment 237529 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6293582923169792

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237545 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 237529 [details]
Patch for landing

Attachment 237529 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6669600867483648

New failing tests:
media/audio-data-url.html
media/sources-fallback-codecs.html

[Build Bot]

Created attachment 237547 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Jer Noble]

Created attachment 237625 [details]
Patch for landing

Add the failing mountainlion results to TestExpectations instead.

[Jer Noble]

Committed r173318: <http://trac.webkit.org/changeset/173318>

[Carlos Alberto Lopez Perez]

(In reply to comment #68)
> Committed r173318: <http://trac.webkit.org/changeset/173318>

This broke the GTK Debug build: http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20%28Build%29/builds/45083/steps/compile-webkit/logs/stdio

[Jer Noble]

(In reply to comment #69)
> (In reply to comment #68)
> > Committed r173318: <http://trac.webkit.org/changeset/173318>
> 
> This broke the GTK Debug build: http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20%28Build%29/builds/45083/steps/compile-webkit/logs/stdio

Fix incoming.

[Jer Noble]

Committed build fix in r173337 <http://trac.webkit.org/r173337>

[Carlos Alberto Lopez Perez]

(In reply to comment #71)
> Committed build fix in r173337 <http://trac.webkit.org/r173337>

It worked. Thanks :)

[Alexey Proskuryakov]

Bug 136532 tracks some Mountain Lion only fallout from this refactoring.

[Jer Noble]

<rdar://problem/19268968>

[Tobias Netzel]

Comment on attachment 237625 [details]
Patch for landing

Just wanted to let you know that the following bug was introduced by this bug/changeset:

> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1205
> -    if (m_seekTo != -1)
> +    if (!m_seekTo.isValid())

    (m_seekTo != -1)
translates to
    (m_seekTo.isValid())
instead of
    (!m_seekTo.isValid())

[Tobias Netzel]

(In reply to comment #75)
This caused seek operation to never finish and the state machine would hang with a "ready state" of "have metadata".

[Jer Noble]

(In reply to comment #76)
> (In reply to comment #75)
> This caused seek operation to never finish and the state machine would hang
> with a "ready state" of "have metadata".

Good catch. I'll file a follow-up fix.

[Jer Noble]

(In reply to comment #77)
> (In reply to comment #76)
> > (In reply to comment #75)
> > This caused seek operation to never finish and the state machine would hang
> > with a "ready state" of "have metadata".
> 
> Good catch. I'll file a follow-up fix.

Filed <https://bugs.webkit.org/show_bug.cgi?id=149845>.