889720 - Media Recording - implement Pause/Resume DOM API

Status: VERIFIED FIXED

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

Description

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

The Bug: 803414 [Audio Recording - Web API & Implementation] just implement the start/stop function. We want to support pause/resume function also.

Comment 1

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

Attachment: patch v1

Using ChangeExplicitBlockerCount to simulate the pause/resume DOM API.

Comment 2

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

Comment on attachment 771952 [details] [diff] [review]
patch v1

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

::: content/media/MediaRecorder.cpp
@@ +172,5 @@
>    }
> +
> +  // Create a TrackUnionStream to support Pause/Resume by using ChangeExplicitBlockerCount
> +  MediaStreamGraph* gm = MediaStreamGraph::GetInstance();
> +  mTrackUnionStream = gm->CreateTrackUnionStream(mStream);

Use mStream->GetStream()->Graph() instead of calling MediaStreamGraph::GetInstance.

@@ +389,5 @@
> +  // Avoid crash on closing the browser during record.
> +  if (!GetOwner() || !mTrackUnionStream->GetWrapper())
> +    return false;
> +
> +  nsCOMPtr<nsIPrincipal> principal = mTrackUnionStream->GetWrapper()->GetPrincipal();

I think you can keep using mStream->GetPrincipal().

Comment 3

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

Attachment: patch v2

Fix nits.

Comment 4

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

Attachment: check-in patch

carry reviewer roc.

Comment 5

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

try server result.
https://tbpl.mozilla.org/?tree=Try&rev=e0500f9e55d2

Comment 6

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1990b3e75ab8

Should this have tests?

Comment 7

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/mozilla-central/rev/1990b3e75ab8

Comment 8

[Jason Smith [:jsmith]]

Verified that smoketests of this feature are passing.