Closed Bug 1300529 Opened 8 years ago Closed 8 years ago

HTMLMediaElement::CaptureStreamInternal called incorrectly

Categories

(Core :: Audio/Video: MediaStreamGraph, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: pehrsons, Assigned: pehrsons)

References

Details

Attachments

(2 files)

Farmer Tseng pointed out that I had changed the signature of CaptureStreamInternal to include a bool for whether or not audio should be captured from the media element, but I had forgotten to update the callsites.

The compiler doesn't complain about this apparently - the third (and optional) `MediaStreamGraph*` argument was implicitly cast to the new bool as second argument. Leaving the bool always true and the MediaStreamGraph* always to default (nullptr).
Rank: 15
Priority: -- → P1
Attachment #8788196 - Flags: review?(padenot)
Attachment #8788197 - Flags: review?(padenot)
Comment on attachment 8788197 [details]
Bug 1300529 - Remove default arguments from AudioNodeStream::Create.

https://reviewboard.mozilla.org/r/76766/#review75424

::: dom/media/webaudio/AudioNodeStream.h:65
(Diff revision 2)
>      EXTERNAL_OUTPUT = 1U << 2,
>    };
>    /**
>     * Create a stream that will process audio for an AudioNode.
>     * Takes ownership of aEngine.
>     * If aGraph is non-null, use that as the MediaStreamGraph, otherwise use

Passing in nullptr is now illegal (and a bunch of assert make sure of this), update this comment.
Attachment #8788197 - Flags: review?(padenot) → review+
Comment on attachment 8788196 [details]
Bug 1300529 - Remove default arguments from HTMLMediaElement::CaptureStreamInternal.

https://reviewboard.mozilla.org/r/76764/#review75428

::: dom/html/HTMLMediaElement.cpp:2653
(Diff revision 2)
>  
>  already_AddRefed<DOMMediaStream>
>  HTMLMediaElement::CaptureAudio(ErrorResult& aRv,
>                                 MediaStreamGraph* aGraph)
>  {
> -  RefPtr<DOMMediaStream> stream = CaptureStreamInternal(false, aGraph);
> +  RefPtr<DOMMediaStream> stream =

MOZ_RELEASE_ASSERT(aGraph);
Comment on attachment 8788196 [details]
Bug 1300529 - Remove default arguments from HTMLMediaElement::CaptureStreamInternal.

https://reviewboard.mozilla.org/r/76764/#review75430
Comment on attachment 8788196 [details]
Bug 1300529 - Remove default arguments from HTMLMediaElement::CaptureStreamInternal.

https://reviewboard.mozilla.org/r/76764/#review75442
Comment on attachment 8788196 [details]
Bug 1300529 - Remove default arguments from HTMLMediaElement::CaptureStreamInternal.

https://reviewboard.mozilla.org/r/76764/#review75444
Attachment #8788196 - Flags: review?(padenot) → review+
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c71a80c9b8e9
Remove default arguments from HTMLMediaElement::CaptureStreamInternal. r=padenot
https://hg.mozilla.org/integration/autoland/rev/904d4282def8
Remove default arguments from AudioNodeStream::Create. r=padenot
https://hg.mozilla.org/mozilla-central/rev/c71a80c9b8e9
https://hg.mozilla.org/mozilla-central/rev/904d4282def8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: