Closed Bug 1524648 Opened 10 months ago Closed 9 months ago

MediaEngineWebRTCMicrophoneSource::Deallocat asserts if another browser already has gUM on audio and video

Categories

(Core :: WebRTC: Audio/Video, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: bryce, Assigned: padenot)

Details

Attachments

(1 file)

I'm able to hit the MOZ_ASSERT(mState == kStopped); in MediaEngineWebRTCMicrophoneSource::Deallocate by trying use gUM if I've already grabbed it in another browser:

STR:

This appears to require both mic and cam on my laptop. Mic only examples seem fine.

Priority: -- → P2

I can't seem to repro this one on Linux, I'll try on Windows.

Assignee: nobody → padenot

Can't repro either.

Will try and take another look later today and see if I can find out if there's something more to the repro -- if nothing else I should be able to provide a stack trace to better contextualise what's happening.

Always asserts on my setup, though I'm still unclear as to what the confounding factor is. Let me know if it would be useful for me to try with any alterations.

Asserting stack:

>	xul.dll!mozilla::MediaEngineWebRTCMicrophoneSource::Deallocate(const RefPtr<const mozilla::AllocationHandle> &) Line 446	C++
 	xul.dll!mozilla::MediaDevice::Deallocate() Line 1017	C++
 	xul.dll!mozilla::GetUserMediaTask::Run() Line 1657	C++
 	xul.dll!MessageLoop::RunTask(already_AddRefed<nsIRunnable> aTask) Line 443	C++
 	xul.dll!MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask && pending_task) Line 450	C++
 	xul.dll!MessageLoop::DoWork() Line 523	C++
 	xul.dll!mozilla::ipc::MessagePumpForNonMainUIThreads::DoRunLoop() Line 377	C++
 	xul.dll!base::MessagePumpWin::Run(base::MessagePump::Delegate * delegate) Line 79	C++
 	xul.dll!MessageLoop::RunHandler() Line 309	C++
 	xul.dll!base::Thread::ThreadMain() Line 192	C++
 	xul.dll!`anonymous namespace'::ThreadFunc(void * closure) Line 31	C++
 	[External Code]	
 	mozglue.dll!patched_BaseThreadInitThunk(int aIsInitialThread, void * aStartAddress, void * aThreadParam) Line 735	C++
 	[External Code]	

Looks like it's hitting failing to allocate the video device in MediaManager. Once this happens it goes to deallocate the audio device. We then call through to our asserting funciton. Looks like since we've just allocated the audio device, we're not in a stopped state when we call deallocate?

It looks like we could also hit this assert if we called Shutdown on a freshly allocated MicSource, as that will also deallocate without changing state.

Yep I think you're right. In terms of state, it's probably legal to deallocate when just allocated.

Pushed by padenot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b350508a425c
Relax assertion in MediaEngineWebRTCMicrophoneSource::Deallocate to take into account the fact that starting the device can have failed. r=pehrsons
Pushed by padenot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/678688278ecc
Relax assertion in MediaEngineWebRTCMicrophoneSource::Deallocate to take into account the fact that starting the device can have failed. r=pehrsons
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Flags: needinfo?(padenot)
QA Whiteboard: [qa-67b-p2]
You need to log in before you can comment on or make changes to this bug.