1691899 - Logic issue in MediaTrackGraphImpl::OpenAudioInputImpl

Status: RESOLVED DUPLICATE of bug 1699233

(Core :: Audio/Video: MediaStreamGraph, defect)

Description

[Simon Giesecke [:sg] [he/him]]

MediaTrackGraphImpl::OpenAudioInputImpl contains the following code at https://searchfox.org/mozilla-central/rev/7067bbd8194f4346ec59d77c33cd88f06763e090/dom/media/MediaTrackGraph.cpp#636-642:

  nsTArray<RefPtr<AudioDataListener>>& listeners =
      mInputDeviceUsers.GetOrInsert(aID);
  if (listeners.IsEmpty() && mInputDeviceUsers.Count() > 1) {
    // We don't support opening multiple input device in a graph for now.
    listeners.RemoveElement(aID);
    return;
  }

In the if body, listeners can't contain aID, since the condition requires listeners to be empty. I suspect this should be mInputDeviceUsers.Remove(aID); instead?

Comment 1

[Simon Giesecke [:sg] [he/him]]

Fixed the regressing bug reference, this was actually added before renaming the file in https://hg.mozilla.org/mozilla-central/annotate/507d5c269d25aee79fe081b1b02a4970ddcc1fb0/dom/media/MediaStreamGraph.cpp

Comment 2

[Simon Giesecke [:sg] [he/him]]

If the assumption from comment 0 is correct, this could be changed to:

  // Only allow one device per MTG (hence, per document), but allow opening a
  // device multiple times
  mInputDeviceUsers.WithEntryHandle(aID, [&, hashtableWasEmpty = mInputDeviceUsers.Count() == 0](auto&& entry) {
    if (!entry) {
      // first open for this device
      mInputDeviceID = aID;
      // Switch Drivers since we're adding input (to input-only or full-duplex)
      AudioCallbackDriver* driver = new AudioCallbackDriver(
          this, CurrentDriver(), mSampleRate, AudioOutputChannelCount(),
          AudioInputChannelCount(), mOutputDeviceID, mInputDeviceID,
          AudioInputDevicePreference());
      LOG(LogLevel::Debug,
          ("%p OpenAudioInput: starting new AudioCallbackDriver(input) %p",
           this, driver));
      SwitchAtNextIteration(driver);

      entry.Insert(nsTArray<RefPtr<AudioDataListener>>{});
    }

      if (entry.Data().IsEmpty() && !hashtableWasEmpty) {
        // We don't support opening multiple input device in a graph for now.
        return;
      }

    MOZ_ASSERT(!entry.Data().Contains(aListener), "Don't add a listener twice.");

    entry.Data().AppendElement(aListener);
  });

Comment 3

[Simon Giesecke [:sg] [he/him]]

Or my assumption from comment 0 is wrong, and actually the if condition is wrong? The comment confuses me a bit.

Comment 4

[Simon Giesecke [:sg] [he/him]]

I guess this can be closed as a duplicate of Bug 1699233, which now has the more detailed analysis.