Media Recording - implement Pause/Resume DOM API

VERIFIED FIXED in mozilla25

Status

()

Core
Audio/Video: Recording
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: rlin, Assigned: rlin)

Tracking

(Blocks: 1 bug)

unspecified
mozilla25
x86_64
All
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

The Bug: 803414 [Audio Recording - Web API & Implementation] just implement the start/stop function. We want to support pause/resume function also.
Assignee: nobody → rlin
Summary: Audio Recording - implement pause/resume → Audio Recording - implement Pause/Resume DOM API
Created attachment 771952 [details] [diff] [review]
patch v1

Using ChangeExplicitBlockerCount to simulate the pause/resume DOM API.
Attachment #771952 - Flags: review?(roc)
Summary: Audio Recording - implement Pause/Resume DOM API → Media Recording - implement Pause/Resume DOM API
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().
Created attachment 775304 [details] [diff] [review]
patch v2

Fix nits.
Attachment #771952 - Attachment is obsolete: true
Attachment #771952 - Flags: review?(roc)
Attachment #775304 - Flags: review?(roc)
Created attachment 775425 [details] [diff] [review]
check-in patch

carry reviewer roc.
Keywords: checkin-needed
Attachment #775304 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/1990b3e75ab8

Should this have tests?
Flags: in-testsuite?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1990b3e75ab8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25

Updated

5 years ago
Keywords: verifyme
QA Contact: jsmith

Updated

5 years ago
No longer blocks: 803414
Verified that smoketests of this feature are passing.

Updated

5 years ago
Status: RESOLVED → VERIFIED
Keywords: verifyme
Component: Video/Audio → Video/Audio: Recording

Updated

5 years ago
No longer blocks: 896935
You need to log in before you can comment on or make changes to this bug.