Closed Bug 1487057 Opened 2 years ago Closed 2 years ago

Finish cleaning up audio input devices code

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox63 --- wontfix
firefox64 --- fixed

People

(Reporter: padenot, Assigned: padenot)

References

Details

Attachments

(10 files)

46 bytes, text/x-phabricator-request
pehrsons
: review+
Details | Review
46 bytes, text/x-phabricator-request
pehrsons
: review+
Details | Review
46 bytes, text/x-phabricator-request
pehrsons
: review+
Details | Review
46 bytes, text/x-phabricator-request
pehrsons
: review+
Details | Review
46 bytes, text/x-phabricator-request
pehrsons
: review+
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
Last step after bug 1404977, to reach something clean so we don't have to touch it for some time.
Priority: -- → P2
This a summary of the thread on which the methods of MediaEngineWebRTCMicrophoneSource are called:

# MediaManager thread (also called Owning thread or Media thread sometimes)

> MediaEngineWebRTCMicrophoneSource (ctor)
> nsString GetName() const override;
> nsCString GetUUID() const override;
> nsresult Allocate() override;
> nsresult Deallocate() override;
> nsresult SetTrack() override;
> nsresult Start() override;
> nsresult Stop() override;
> nsresult Reconfigure() override;
> dom::MediaSourceEnum GetMediaSource() const override
> nsresult TakePhoto() override
> uint32_t GetBestFitnessDistance() const override;
> void Shutdown() override;
> nsresult ReevaluateAllocation();
> nsresult UpdateSingleSource();
> void UpdateAECSettingsIfNeeded();
> void UpdateAGCSettingsIfNeeded();
> void UpdateNSSettingsIfNeeded();
> void ApplySettings();
> bool HasEnabledTrack() const;

# Never called (but required because of inheritance)

> bool RequiresSharing();

# Main thread

> void GetSettings() const override;

# Graph thread (also called MSG thread, audio callback thread)

> void Pull() override;
> void NotifyOutputData() override;
> void NotifyInputData() override;
> void DeviceChanged() override;
> uint32_t RequestedInputChannelCount() override
> void Disconnect() override;
> void InsertInGraph();
> void PacketizeAndProcess();
> bool PassThrough() const;
> void SetPassThrough();
> uint32_t GetRequestedInputChannelCount();
> void SetRequestedInputChannelCount();

Not too bad. The issue is with the attributes in those methods, it's really weird.
It's not maintained and probably does not work anymore.
Big but not complex:
- Remove the mutex
- Move all MSG thread to a new class (AudioInputProcessing)
- Remove the WebRTCAudioDataListener class, AudioInputProcessing is the listener
- Use message passing for all modifications to the AudioInputProcessing.
Comment on attachment 9007809 [details]
Bug 1487057 - Part 1 - Move audio related classes in MediaEngineWebRTC.h to their own header files, clean up includes slightly. r?pehrsons

Andreas Pehrson [:pehrsons] has approved the revision.
Attachment #9007809 - Flags: review+
Comment on attachment 9007810 [details]
Bug 1487057 - Part 2 - Remove RegisterForAudioMixing/NeedsMixing, they are unused. r?pehrsons

Andreas Pehrson [:pehrsons] has approved the revision.
Attachment #9007810 - Flags: review+
Comment on attachment 9007811 [details]
Bug 1487057 - Part 3 - Turn the mAllocations array into an mAllocation UniquePtr, that can be nullptr. r?pehrsons

Andreas Pehrson [:pehrsons] has approved the revision.
Attachment #9007811 - Flags: review+
Comment on attachment 9007812 [details]
Bug 1487057 - Part 4 - Remove AsyncLatencyLogger and associated code. r?pehrsons

Andreas Pehrson [:pehrsons] has approved the revision.
Attachment #9007812 - Flags: review+
Comment on attachment 9007813 [details]
Bug 1487057 - Part 5 - Remove MediaEngineWebRTCMicrophoneSource::mStarted.  r?pehrsons

Andreas Pehrson [:pehrsons] has approved the revision.
Attachment #9007813 - Flags: review+
Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/mozilla-inbound/rev/efd0c87b160b
Part 1 - Move audio related classes in MediaEngineWebRTC.h to their own header files, clean up includes slightly. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e6e230af698
Part 2 - Remove RegisterForAudioMixing/NeedsMixing, they are unused. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ff222ff2a72
Part 3 - Turn the mAllocations array into an mAllocation UniquePtr, that can be nullptr. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/304b4f68b942
Part 4 - Remove AsyncLatencyLogger and associated code. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/746f70de2691
Part 5 - Remove MediaEngineWebRTCMicrophoneSource::mStarted.  r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/582e630a2fcb
Part 6 - Reorganize attributes and document thread access better. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/273c92182c3c
Part 7 - Flatten mAllocation and fix some locking. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/45d2c462dc92
Part 8 - Split MediaEngineWebRTCMicrophoneSource in two classes, one for control one for processing. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1a790218e20
Part 9 - Make SourceMediaStream::SetEnded go through the message queue so it's in the right order w.r.t. Stop. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/9cf36402deed
Part 10 - Remove MediaEngineWebRTCAudio::mEnabled. r=pehrsons
https://hg.mozilla.org/integration/mozilla-inbound/rev/0190c5793ffe
Part 11 - Work around the fact that EndTrack uses mCommands. r=pehrsons
The last patch here was reviewed on irc.
Depends on: 1503536
You need to log in before you can comment on or make changes to this bug.