1120030 - Check in mp4 tests for TimestampOffset

Status: RESOLVED FIXED

(Core :: Audio/Video, defect, P2)

Description

[Bobby Holley (:bholley)]

I have these, but they're currently orange on try. I'm hoping that the changes in bug 1119456 will make them pass.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Part 1 - Factor some machinery out of test_BufferingWait into mediasource.js and make it Promise-friendly. v1

Comment 2

[Bobby Holley (:bholley)]

Attachment: Part 2 - Tests for timestampOffset. v1

Comment 3

[Bobby Holley (:bholley)]

Note that if you have the mind to find a video better than gizmo, by all means please do so. I'm just suggesting building that on top of these patches, since they've already gone to the trouble of setting up all the necessary build files etc.

Comment 4

[Jean-Yves Avenard [:jya]]

Comment on attachment 8547259 [details] [diff] [review]
Part 1 - Factor some machinery out of test_BufferingWait into mediasource.js and make it Promise-friendly. v1

Review of attachment 8547259 [details] [diff] [review]:
-----------------------------------------------------------------

I'm no javascript expert, but that looks good to me :)

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8547260 [details] [diff] [review]
Part 2 - Tests for timestampOffset. v1

Review of attachment 8547260 [details] [diff] [review]:
-----------------------------------------------------------------

Let's not use gizmo based video, they serve little value and it's hard to tell if playback of it as it provides little video cues.
As discussed on IRC, waiting for bug 1120266 to land and use those samples instead.

Comment 6

[Bobby Holley (:bholley)]

Going to get the seeking tests green first in bug 1126052. Once those are on, I can port the timestampOffset tests over to bipbop and get them landed.

Comment 7

[Bobby Holley (:bholley)]

Attachment: Test for timestampOffset. v2

Comment 8

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8ae4eebbc87

Comment 9

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (Busy with media, don't ask for DOM/JS/XPConnect things) from comment #8)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8ae4eebbc87

I did a lot of retriggers just to be safe - all green. :-)

Comment 10

[Jean-Yves Avenard [:jya]]

Comment on attachment 8560785 [details] [diff] [review]
Test for timestampOffset. v2

Review of attachment 8560785 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/mediasource/test/test_TimestampOffset_mp4.html
@@ +67,5 @@
> +
> +      // Trim the rest of the audio.
> +      ms.duration = videosb.buffered.end(0);
> +      return Promise.all([audiosb.updating ? once(audiosb, 'update') : Promise.resolve(),
> +                          videosb.updating ? once(videosb, 'update') : Promise.resolve()]);

"update" is not guaranteed to be fired, so better use "updateend".

I don't think you should resolve if any of the sb.updating were false. It would be an error if it wasn't true.

Comment 11

[Bobby Holley (:bholley)]

(In reply to Jean-Yves Avenard [:jya] from comment #10)
> "update" is not guaranteed to be fired, so better use "updateend".

Ok.

> I don't think you should resolve if any of the sb.updating were false. It
> would be an error if it wasn't true.

Per IRC discussion I just want to be safe to avoid flakiness.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b0674e3a899

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7b0674e3a899