[webkit-reviews] review granted: [Bug 211192] USB microphone not recognized iOS Safari : [Attachment 413753] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 211192: USB microphone not recognized iOS Safari
https://bugs.webkit.org/show_bug.cgi?id=211192

Attachment 413753: Patch

https://bugs.webkit.org/attachment.cgi?id=413753&action=review




--- Comment #9 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 413753
  --> https://bugs.webkit.org/attachment.cgi?id=413753
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413753&action=review

> Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDevice.mm:44
> +    setIsDefault(defaultInput && [defaultInput.UID
isEqualToString:deviceInput.UID]);

Having two AVAudioSessionPortDescription* might be a bit confusing
Since we are only using defaultInput to check whether deviceInput is default,
we could just pass isDefault value here or add a setter for this value that
would be called later.

>
Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:5
2
> +    [[NSNotificationCenter defaultCenter] addObserver:self
selector:@selector(routeDidChange:)
name:PAL::get_AVFoundation_AVAudioSessionRouteChangeNotification()
object:session];

Is it always calling from the main thread?

>
Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:1
58
> +	   LOG_ERROR("Failed to activate audio session with error: %@.",
error.localizedDescription);

RELEASE_LOG?

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDevice.cpp:70
> +	   err = AudioObjectGetPropertyData(static_cast<UInt32>(deviceID),
&address, 0, nullptr, &dataSize, &localizedName);

Logging maybe?

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDevice.cpp:110
> +    setIsDefault(!err && defaultID == m_deviceID);

logging?

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:154
> +		  
CoreAudioCaptureDeviceManager::singleton().scheduleUpdateCaptureDevices();

Sounds good to do this indeed.
The other changes of this method are refactoring and are not really changing
any behavior, is that correct?

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:267
> +	       auto& newState = audioDevices[cptr];

s/oldState/oldDevice/
s/newState/newDevice/