1273206 - enumDev/gUM starts RTP thread in 5ms loop

Status: RESOLVED FIXED

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

Description

[Nils Ohlmeier [:drno]]

Calling enumarateDevices or gUM({audio: true}) creates an audio channel, which starts a 'ProcessThread', which then gets the RtpRtcp module added to, which then keeps telling the Process loop of the thread to wait for 5ms.

And that 'ProcessThread' only gets killed if you quit Firefox.

Note: under certain criterias (which I haven't figured out yet) the ProcessThread stops demanding callbacks every 5ms. But bringing the browser or tab (?) back into focus starts the 5ms loop again.

Here the RtpRtcpModule gets added to the 'ProcessThread' by the channel:
https://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/voice_engine/channel.cc#964

This is the code where the RtpRtcpModule demands to get called every 5ms:
https://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc#112

Call stack for enumerateDevices:
  * frame #0: 0x0000000106af1d07 XUL`webrtc::VoEBaseImpl::InitializeChannel(this=0x0000000131f76000, channel_owner=0x0000700002398fb8) + 23 at voe_base_impl.cc:516
    frame #1: 0x0000000106af1ccb XUL`webrtc::VoEBaseImpl::CreateChannel(this=0x0000000131f76000) + 219 at voe_base_impl.cc:500
    frame #2: 0x00000001058cbb9f XUL`mozilla::MediaEngineWebRTCMicrophoneSource::Init(this=0x0000000131c0f080) + 463 at MediaEngineWebRTCAudio.cpp:563
    frame #3: 0x00000001058be19a XUL`mozilla::MediaEngineWebRTCMicrophoneSource::MediaEngineWebRTCMicrophoneSource(this=0x0000000131c0f080, aThread=0x000000011febda00, aVoiceEnginePtr=0x0000000131f76000, aAudioInput=0x000000013008c200, aIndex=-1, name="default: Built-in Microphone (Internal Microphone)", uuid="default: Built-in Microphone (Internal Microphone)") + 1162 at MediaEngineWebRTC.h:448
    frame #4: 0x00000001058bcaad XUL`mozilla::MediaEngineWebRTCMicrophoneSource::MediaEngineWebRTCMicrophoneSource(this=0x0000000131c0f080, aThread=0x000000011febda00, aVoiceEnginePtr=0x0000000131f76000, aAudioInput=0x000000013008c200, aIndex=-1, name="default: Built-in Microphone (Internal Microphone)", uuid="default: Built-in Microphone (Internal Microphone)") + 77 at MediaEngineWebRTC.h:442
    frame #5: 0x00000001058bb3c4 XUL`mozilla::MediaEngineWebRTC::EnumerateAudioDevices(this=0x000000011b1534a0, aMediaSource=Microphone, aASources=0x0000700002399748) + 1812 at MediaEngineWebRTC.cpp:413
    frame #6: 0x000000010560f15f XUL`void mozilla::GetSources<mozilla::AudioDevice>(engine=0x000000011b1534a0, aSrcType=Microphone, aEnumerate=09 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00, aResult=0x0000700002399890, media_device_name=0x0000000000000000)(mozilla::dom::MediaSourceEnum, nsTArray<RefPtr<mozilla::AudioDevice::Source> >*), nsTArray<RefPtr<mozilla::AudioDevice> >&, char const*) + 175 at MediaManager.cpp:1031
    frame #7: 0x000000010560ec3f XUL`mozilla::MediaManager::EnumerateRawDevices(this=0x000000011fe02458)::$_7::operator()() + 879 at MediaManager.cpp:1387
    frame #8: 0x000000010560e7d9 XUL`mozilla::media::LambdaTask<mozilla::MediaManager::EnumerateRawDevices(unsigned long long, mozilla::dom::MediaSourceEnum, mozilla::dom::MediaSourceEnum, bool, bool)::$_7>::Run(this=0x000000011fe02440) + 25 at MediaUtils.h:220
    frame #9: 0x0000000101feda11 XUL`nsThread::ProcessNextEvent(this=0x000000012c43d800, aMayWait=true, aResult=0x0000700002399b4e) + 1969 at nsThread.cpp:1073
    frame #10: 0x000000010207414c XUL`NS_ProcessNextEvent(aThread=0x000000012c43d800, aMayWait=true) + 140 at nsThreadUtils.cpp:290
    frame #11: 0x0000000102971723 XUL`mozilla::ipc::MessagePumpForNonMainThreads::Run(this=0x0000000131efe940, aDelegate=0x0000700002399d18) + 1027 at MessagePump.cpp:382
    frame #12: 0x00000001028baca5 XUL`MessageLoop::RunInternal(this=0x0000700002399d18) + 117 at message_loop.cc:233
    frame #13: 0x00000001028bac05 XUL`MessageLoop::RunHandler(this=0x0000700002399d18) + 21 at message_loop.cc:226
    frame #14: 0x00000001028babad XUL`MessageLoop::Run(this=0x0000700002399d18) + 45 at message_loop.cc:206
    frame #15: 0x00000001028dedbc XUL`base::Thread::ThreadMain(this=0x00000001320a4100) + 284 at thread.cc:178
    frame #16: 0x00000001028df27e XUL`ThreadFunc(closure=0x00000001320a4100) + 30 at platform_thread_posix.cc:36
    frame #17: 0x00007fff8a71199d libsystem_pthread.dylib`_pthread_body + 131
    frame #18: 0x00007fff8a71191a libsystem_pthread.dylib`_pthread_start + 168
    frame #19: 0x00007fff8a70f351 libsystem_pthread.dylib`thread_start + 13

Comment 1

[Nils Ohlmeier [:drno]]

:jesup, :jib :

- do we really have to start this 'ProcessThread' when doing enumerateDevices or just gUM audio?
- do we really have to attach the RtpRtcpModule listener to that Process thread?

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Shut down all getUserMedia VoEBase channels when not in use

ProcessThread is no longer hanging around when no gUM stream is open, and also this will free up memory immediately when you shut down gUM.  Downside will be slightly slower open when no gUM stream is active; likely not perceptible.  Try:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=6921bbcae420

Comment 3

[Andreas Pehrson [:pehrsons]]

Comment on attachment 8753211 [details] [diff] [review]
Shut down all getUserMedia VoEBase channels when not in use

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

I'm surprised there were no funky intermittents coming from this :-)

I have one drive-by nit that I think you should fix.

::: dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ +556,5 @@
>    ResetProcessingIfNeeded(Ec);
>    ResetProcessingIfNeeded(Ns);
>  }
>  
> +bool

Return nsresult instead.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8753211 [details] [diff] [review]
Shut down all getUserMedia VoEBase channels when not in use

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

::: dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ +649,5 @@
> +    mVoEBase = nullptr;
> +  }
> +}
> +
> +// This shuts down the engine when no channel is open

I'll revise this comment

Comment 5

[Nils Ohlmeier [:drno]]

Tested the patch locally on my Mac. I see the following behavior now:

- the ProcessThread pops up very briefly during enumerateDevices
- the ProcessThread is active as long as gUM is active
- but the thread dis-appears after gUM usage stopped

So to me this look like a big improvement.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Note this both reduces power usage after GUM is used (and stopped), but also memory usage.

Comment 7

[Paul Adenot (:padenot)]

Comment on attachment 8753211 [details] [diff] [review]
Shut down all getUserMedia VoEBase channels when not in use

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

Good, this is better for 1271585 as well.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/91c13e523507

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/91c13e523507

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

poke to make sure I come back to assess if we want to uplift

Comment 11

[Nils Ohlmeier [:drno]]

(In reply to Randell Jesup [:jesup] from comment #10)
> poke to make sure I come back to assess if we want to uplift

I don't think this is worth the risk. But I'm alright if you still want to uplift it.

Comment 12

[Carl Marrelli]

Hey Nils, I am not sure what the above all means.  Can you please comment on if/when this fix will make it into nightly channel?

Comment 13

[Nils Ohlmeier [:drno]]

(In reply to Carl Marrelli from comment #12)
> Hey Nils, I am not sure what the above all means.  Can you please comment on
> if/when this fix will make it into nightly channel?

Hi Carl,
comment #9 means the code landed in the repository from which Firefox Nightly gets build. As this is now more then 24 hours ago you can safely assume that this fix is now contained in the latest Nightly build. So download or update your Firefox Nightly and give it a try.