Something weird in `OpenAudioInputImpl`
Categories
(Core :: WebRTC: Audio/Video, defect, P1)
Tracking
()
People
(Reporter: chunmin, Assigned: chunmin)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file)
2.37 KB,
text/plain
|
Details |
I suspect listeners.RemoveElement(aID)
is wrong in MediaTrackGraphImpl::OpenAudioInputImpl
.
listeners.RemoveElement(aID)
is called whenlisteners.IsEmpty()
istrue
. It makes no sense to remove an element from an empty listlisteners
- The type of
listeners
isnsTArray<RefPtr<AudioDataListener>>
, and its element-type isAudioDataListener*
. If we want to remove element fromlisteners
, we should uselisteners.RemoveElement(aListener)
instead oflisteners.RemoveElement(aID)
.aID
is never put into thelisteners
and its type is wrong (*void
). There is no need to remove an element from a list that is never added to a list.
I guess what we want to do is mInputDeviceUsers.Remove(aID)
?
The side-effect I could see is that once a second device tries the register a listener into mInputDeviceUsers
, the following listener registration via OpenAudioInputImpl
no longer works.
To be specific, here is the situation I could imagine:
- Register listener
A
for deviceX
tomInputDeviceUsers
- Now
mInputDeviceUsers.Count()
is from0
to1
listeners
ofX
is empty soA
will be put intolisteners
ofX
- Now
- Register listener
B
for deviceY
tomInputDeviceUsers
- Now
mInputDeviceUsers.Count()
is from1
to2
listeners
ofY
is empty solisteners.RemoveElement(aID)
is called but it does nothing.B
cannot be put into thelisteners
ofY
- Now
- Remove listener
A
for deviceX
frommInputDeviceUsers
viaCloseAudioInputImpl
- Now
mInputDeviceUsers.Count()
is from2
to1
, sincelisteners
ofX
is empty now
- Now
- Register listener
C
for deviceX
tomInputDeviceUsers
- Now
mInputDeviceUsers.Count()
is from1
to2
listeners
ofX
is empty solisteners.RemoveElement(aID)
is called but it does nothing.- Since
mInputDeviceUsers.Count()
is at least1
now so we cannot register any new listener.
- Now
We should be able to register a new listener at the 4.
I am trying to write a test to see if the above case can happen or not.
Assignee | ||
Comment 1•3 years ago
•
|
||
A dirty test confirms comment 0. The TestAudioTrackGraph.ReOpenAudioInput
(./mach gtest TestAudioTrackGraph.ReOpenAudioInput
) will be pending on second inputTrack->OpenAudioInput
with this change. But it should work.
Fortunately, we have some assertions to prevent us from stepping into this situation. So the issue isn't that bad as I guess.
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 3•3 years ago
|
||
It's probably best to address this as part as your multi-input work.
Assignee | ||
Comment 4•3 years ago
|
||
Yes, I plan to fix this in Bug 1702646, or Bug 1238038 at least.
Updated•3 years ago
|
Description
•