903781 - Mochitest - Record media with timeslice for Media Recording API

Status: RESOLVED FIXED

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

Description

[Jason Smith [:jsmith]]

Create a mochitest that records media with a timeslice provided for the media recording API.

Comment 1

[Jason Smith [:jsmith]]

Attachment: Patch v1

Comment 2

[Jason Smith [:jsmith]]

Comment on attachment 788553 [details] [diff] [review]
Patch v1

Note - this needs to wait for bug 903758 to land before this can ran without failures. Marking review though cause this runs cleanly other than that known failure.

Comment 3

[Jason Smith [:jsmith]]

This also will cover the mochitest request in bug 903758. I realized after the fact that I already had a patch in my queue covering the testing request in there that I forgot to upload, so I'm double checking with Randy in bug 903758 if we can use this patch to cover the mochitest request and land bug 903758 with manual testing confirmation that it works.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 788553 [details] [diff] [review]
Patch v1

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

::: content/media/test/test_mediarecorder_record_timeslice.html
@@ +80,5 @@
> +    // On the last blob, we ensure ondataavailable fires before onstop
> +    } else if (dataAvailableCount === 3) {
> +      onDataAvailableFirst = true;
> +
> +    // We should never receive more than three ondataavailable events

Fix indent

Comment 5

[Jason Smith [:jsmith]]

Attachment: Patch v2

Comment 6

[Jason Smith [:jsmith]]

Comment on attachment 788737 [details] [diff] [review]
Patch v2

Carrying forward r+ with indentation fix.

Comment 7

[Jason Smith [:jsmith]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=c71fa1c45a24

Two things seen here:

- We're getting the known failure with the recording status due to the dependency not being landed yet.

- We're getting a couple of timeouts right now in a couple of cases. Looks like ondataavailable is failing to fire more than once in some cases. Randy - Any ideas why?

Comment 8

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

I just add the bug 903758 patch (not landed yet) in it and 
result seems fine
https://tbpl.mozilla.org/?tree=Try&rev=3f4b3eca03ae.

Comment 9

[Jason Smith [:jsmith]]

(In reply to Randy Lin [:rlin] from comment #8)
> I just add the bug 903758 patch (not landed yet) in it and 
> result seems fine
> https://tbpl.mozilla.org/?tree=Try&rev=3f4b3eca03ae.

The test run you just kicked off here actually shows this timeout occurring 4 times and one case where the blob data received was zero. Can you check again?

Comment 10

[Jason Smith [:jsmith]]

Specifically, here's the test failures I'm seeing in the try run:

Test Failures

Ubuntu 12 Opt

148876 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_mediarecorder_record_timeslice.html | Blob data received should be greater than zero

OS X 10.8 Debug

147545 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_mediarecorder_record_timeslice.html | Test timed out.

Win 7 Debug

151921 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_mediarecorder_record_timeslice.html | Test timed out. 

Win 8 Debug

151892 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_mediarecorder_record_timeslice.html | Test timed out. 

Android 4.0 Opt

26662 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/test/test_mediarecorder_record_timeslice.html | Test timed out.

Comment 11

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Sorry, I should wait the full result and post it. Let me dig it out.

Comment 12

[Jason Smith [:jsmith]]

Given the problems on the other mochitest, I'm going to also dig into this and see if this is a test problem. Feel free to help with investigation though.

Comment 13

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

I think this test case just wait two dataavailable event and fire stop method, 
In our design, we don't guarantee every timeslice has the dataavailable event.
If recorder start a little late, we may not have enough data to encode and no more dataavailable event for UA, so that cause timeout. Other zero blob event I think just hit the end of mediastream case, so no encoded data can provide. I think you can use longer opus file to test and can avoid those problems.

Comment 14

[Jason Smith [:jsmith]]

Attachment: Patch v3

Comment 15

[Jason Smith [:jsmith]]

Comment on attachment 795807 [details] [diff] [review]
Patch v3

This addresses the intermittent failures I saw before.

1. Moved to mozCaptureStream is order to allow the stream to not end even if the media element ends (i.e. ondataavailable events can fire still, even with size 0 blobs).

2. Switched the blob size to check to be greater than or equal to zero. This is because the element could end before canplaythrough fires, so we could start after and fire ondataavailable events potentially with zero-sized blobs.

Comment 16

[Jason Smith [:jsmith]]

Try run in progress here - https://tbpl.mozilla.org/?tree=Try&rev=4fdc88d3a055. Looking good so far though.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/d5cd8e27cb9c

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Backed out for Android mochitest-3 failures.
https://hg.mozilla.org/integration/b2g-inbound/rev/438dbb174948

https://tbpl.mozilla.org/php/getParsedLog.php?id=27180182&tree=B2g-Inbound

Comment 19

[Jason Smith [:jsmith]]

Attachment: Patch v4

Comment 20

[Jason Smith [:jsmith]]

Attachment: Patch v4

Comment 21

[Jason Smith [:jsmith]]

Comment on attachment 797321 [details] [diff] [review]
Patch v4

Looks like it's possible to have three ondataavailable events fire here per the backout - probably due to timing of when events fire. On that regard, we probably can't assume how many ondataavailable events we'll get in a mochitest. We can only ensure that:

* The state checks that should be always true with an event in ondataavailable are always correct (e.g. blob event, mime type correct)

* At least two ondataavailable events have fired before onstop fires. Our test will stop after one event is found, which means at a minimum, a 2nd ondataavailable event should fire.

Comment 22

[Jason Smith [:jsmith]]

Let me do a try run first before putting checkin-needed.

Comment 23

[Jason Smith [:jsmith]]

https://tbpl.mozilla.org/?tree=Try&rev=079b2419312f

Comment 24

[Jason Smith [:jsmith]]

Try run is green. All this patch does is remove the check that caused the backout before, so this should go in cleanly now.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/0bf1e520b9bf

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0bf1e520b9bf