Resolves issue with bad audio using BT headsets on iOS

Resolves issue with bad audio using BT headsets on iOS.

BUG=webrtc:6004
R=tkchin@webrtc.org

Committed: https://crrev.com/2d014be554f63128ceed8ecba007dff89595b4ae
Cr-Commit-Position: refs/heads/master@{#13165}

[henrika_webrtc, 2016-06-13 15:12:41]

Description was changed from

==========
Resolves issue with bad audio using BT headsets on iOS

BUG=
==========

to

==========
Resolves issue with bad audio using BT headsets on iOS.

BUG=b/29185303
==========

[henrika_webrtc, 2016-06-13 15:12:42]

henrika@webrtc.org changed reviewers:
+ tkchin@webrtc.org

[henrika_webrtc, 2016-06-13 15:30:43]

Zeke, PTAL

[henrika_webrtc, 2016-06-13 15:30:48]

Zeke, PTAL

[tkchin_webrtc, 2016-06-14 19:13:28]

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm (right):

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:639: // It can happen
(e.g. in combination with BT devices) that the attempt to set
It seems like we should do this on route changes, and not on generic activation.

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:646: // TODO(henrika):
this fix resolves b/29185303 but it has only been tested on
Don't reference buganizer in webrtc. File a corresponding webrtc bug if needed.

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:658: [self
unconfigureWebRTCSession:nil];
this won't work if there are multiple activations ([self setActive:NO] won't
actually deactivate the session unless the activation count is 1). You might
want to directly deactivate avaudiosession.

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:660:
[RTCAudioSessionConfiguration setWebRTCConfiguration:webRTCConfig];
I don't think we should set the default because we will always want to try the
default first and see if it is usable.

[henrika_webrtc, 2016-06-15 11:35:25]

Description was changed from

==========
Resolves issue with bad audio using BT headsets on iOS.

BUG=b/29185303
==========

to

==========
Resolves issue with bad audio using BT headsets on iOS.

BUG=webrtc:6004
==========

[henrika_webrtc, 2016-06-15 13:46:58]

Done a more simple version where I don't touch the audio session. Works well for
me.

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm (right):

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:639: // It can happen
(e.g. in combination with BT devices) that the attempt to set
But the problem is that it does not work since we don't get any route changes at
the second call. We only get it the first call and then everything works as is.
I did not come up with any other method that helped. Note that we used to
deregister all observers when closing a call before but we don't now. If you are
OK with adding back the old style, it might resolve the issue. I tried but it
seems like we now depend on these notifiers existing from start to end of the
app.

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:646: // TODO(henrika):
this fix resolves b/29185303 but it has only been tested on
On 2016/06/14 19:13:28, tkchin_webrtc wrote:
> Don't reference buganizer in webrtc. File a corresponding webrtc bug if
needed.

Done.

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:658: [self
unconfigureWebRTCSession:nil];
Missed that. I actually started out by making the change of sample rate without
trying to deactivate and that worked as well. However, Apple recommends doing it
while session is inactive. I might not have full control of the current
reference counting scheme but let me try to come up with a new version.

https://codereview.webrtc.org/2063733002/diff/20001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:660:
[RTCAudioSessionConfiguration setWebRTCConfiguration:webRTCConfig];
Agree that it is not a clean solution. Will rethink.

[tkchin_webrtc, 2016-06-15 21:21:46]

lgtm

If this simpler fix works I'm all for it.

https://codereview.webrtc.org/2063733002/diff/40001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm (right):

https://codereview.webrtc.org/2063733002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:648: double
session_sample_rate = self.sampleRate;
in ObjC methods, use ObjC style rules. Camel case instead of underscore.

e.g sessionSampleRate

https://codereview.webrtc.org/2063733002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:651: RTCLogWarning(
fyi I've made it to that ObjC files are now 100char (consistent with ObjC style)

[henrika_webrtc, 2016-06-16 12:12:26]

Thanks. Yes, it looks simple but it took me quite some time to come up with ;-)
It seems like the behaviour in this area (BT) changes over time with new iOS
releases. Let's hope that this one works well.

https://codereview.webrtc.org/2063733002/diff/40001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm (right):

https://codereview.webrtc.org/2063733002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:648: double
session_sample_rate = self.sampleRate;
On 2016/06/15 21:21:46, tkchin_webrtc wrote:
> in ObjC methods, use ObjC style rules. Camel case instead of underscore.
> 
> e.g sessionSampleRate 

Done.

https://codereview.webrtc.org/2063733002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:651: RTCLogWarning(
Got it. Keeping 80 here to be consistent with the rest of the file.

[henrika_webrtc, 2016-06-16 12:14:43]

The CQ bit was checked by henrika@webrtc.org

[henrika_webrtc, 2016-06-16 12:14:43]

The patchset sent to the CQ was uploaded after l-g-t-m from tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2063733002/#ps60001 (title:
"style fixes")

[commit-bot: I haz the power, 2016-06-16 12:14:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2063733002/60001

[commit-bot: I haz the power, 2016-06-16 12:22:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-16 12:22:39]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)

[commit-bot: I haz the power, 2016-06-16 12:27:10]

Description was changed from

==========
Resolves issue with bad audio using BT headsets on iOS.

BUG=webrtc:6004
==========

to

==========
Resolves issue with bad audio using BT headsets on iOS.

BUG=webrtc:6004
R=tkchin@webrtc.org

Committed: https://crrev.com/2d014be554f63128ceed8ecba007dff89595b4ae
Cr-Commit-Position: refs/heads/master@{#13165}
==========

[commit-bot: I haz the power, 2016-06-16 12:27:11]

Patchset 4 (id:??) landed as
https://crrev.com/2d014be554f63128ceed8ecba007dff89595b4ae
Cr-Commit-Position: refs/heads/master@{#13165}