HTMLMediaElement::CaptureStreamInternal called incorrectly

RESOLVED FIXED in Firefox 51

Status

()

P1
normal
Rank:
15
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: pehrsons, Assigned: pehrsons)

Tracking

unspecified
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
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).
(Assignee)

Updated

2 years ago
Rank: 15
Priority: -- → P1
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8788196 - Flags: review?(padenot)
Attachment #8788197 - Flags: review?(padenot)

Comment 5

2 years ago
mozreview-review
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 6

2 years ago
mozreview-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 7

2 years ago
mozreview-review
Comment on attachment 8788196 [details]
Bug 1300529 - Remove default arguments from HTMLMediaElement::CaptureStreamInternal.

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

Comment 8

2 years ago
mozreview-review
Comment on attachment 8788196 [details]
Bug 1300529 - Remove default arguments from HTMLMediaElement::CaptureStreamInternal.

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

Comment 9

2 years ago
mozreview-review
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+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 12

2 years ago
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

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c71a80c9b8e9
https://hg.mozilla.org/mozilla-central/rev/904d4282def8
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.