Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2366753005:
Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS (Closed)
|
Created:
4 years, 3 months ago by henrika_webrtc Modified:
4 years, 2 months ago CC:
webrtc-reviews_webrtc.org Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS
BUG=b/30944297
NOTRY=TRUE
Committed: https://crrev.com/f1363fdf578c0ca317609aaff901743cfb89b8e7
Cr-Commit-Position: refs/heads/master@{#14398}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Feedback from Chuck #
Total comments: 4
Patch Set 3 : Feedback from tkchin@ #Messages
Total messages: 16 (7 generated)
Description was changed from ========== Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS BUG= ========== to ========== Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS BUG=b/30944297 ==========
henrika@webrtc.org changed reviewers: + haysc@webrtc.org, tkchin@webrtc.org
Have not been able to trigger it locally but adding logs just in case to ensure that we don't miss anything.
lgtm https://codereview.chromium.org/2366753005/diff/1/webrtc/modules/audio_device... File webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm (right): https://codereview.chromium.org/2366753005/diff/1/webrtc/modules/audio_device... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:79: // hint to enable or disable audio that is sedondary. typo: s/sedondary/secondary/ https://codereview.chromium.org/2366753005/diff/1/webrtc/modules/audio_device... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:525: - (void)handleSilenceSecondaryAudioHintNotification: This looks like it would fit on a single line (525/526) https://codereview.chromium.org/2366753005/diff/1/webrtc/modules/audio_device... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:533: (AVAudioSessionSilenceSecondaryAudioHintType) 533/534 single line
Thanks. Should be fixed now. https://codereview.chromium.org/2366753005/diff/1/webrtc/modules/audio_device... File webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm (right): https://codereview.chromium.org/2366753005/diff/1/webrtc/modules/audio_device... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:79: // hint to enable or disable audio that is sedondary. On 2016/09/23 13:22:54, Chuck wrote: > typo: s/sedondary/secondary/ Done. https://codereview.chromium.org/2366753005/diff/1/webrtc/modules/audio_device... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:525: - (void)handleSilenceSecondaryAudioHintNotification: It does...but I used git cl format ;-) https://codereview.chromium.org/2366753005/diff/1/webrtc/modules/audio_device... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:533: (AVAudioSessionSilenceSecondaryAudioHintType) On 2016/09/23 13:22:54, Chuck wrote: > 533/534 single line Done.
FYI; red bots are not related to my CL.
lgtm https://codereview.chromium.org/2366753005/diff/20001/webrtc/modules/audio_de... File webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm (right): https://codereview.chromium.org/2366753005/diff/20001/webrtc/modules/audio_de... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:525: - (void)handleSilenceSecondaryAudioHintNotification:(NSNotification*)notification { NSNotification *) https://codereview.chromium.org/2366753005/diff/20001/webrtc/modules/audio_de... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:529: NSNumber* typeNumber = NSNumber *typeNumber
https://codereview.webrtc.org/2366753005/diff/20001/webrtc/modules/audio_devi... File webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm (right): https://codereview.webrtc.org/2366753005/diff/20001/webrtc/modules/audio_devi... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:525: - (void)handleSilenceSecondaryAudioHintNotification:(NSNotification*)notification { On 2016/09/26 09:54:30, tkchin_webrtc wrote: > NSNotification *) Done. https://codereview.webrtc.org/2366753005/diff/20001/webrtc/modules/audio_devi... webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm:529: NSNumber* typeNumber = On 2016/09/26 09:54:29, tkchin_webrtc wrote: > NSNumber *typeNumber Done.
Description was changed from ========== Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS BUG=b/30944297 ========== to ========== Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS BUG=b/30944297 NOTRY=TRUE ==========
The CQ bit was checked by henrika@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from haysc@webrtc.org, tkchin@webrtc.org Link to the patchset: https://codereview.webrtc.org/2366753005/#ps40001 (title: "Feedback from tkchin@")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS BUG=b/30944297 NOTRY=TRUE ========== to ========== Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS BUG=b/30944297 NOTRY=TRUE ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS BUG=b/30944297 NOTRY=TRUE ========== to ========== Adds support for AVAudioSessionSilenceSecondaryAudioHintNotification on iOS BUG=b/30944297 NOTRY=TRUE Committed: https://crrev.com/f1363fdf578c0ca317609aaff901743cfb89b8e7 Cr-Commit-Position: refs/heads/master@{#14398} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/f1363fdf578c0ca317609aaff901743cfb89b8e7 Cr-Commit-Position: refs/heads/master@{#14398} |
