CB-7684: (iOS) Fix CDVSound killing all audio when a single file finishes

[tbrebant]

Platforms affected

iOS

What does this PR do?

We arrived to the same conclusion as Nathan Stryker on Jira (https://issues.apache.org/jira/browse/CB-7684) and @ionut-movila in the PR #33 : all self.avSession setActive:NO should be removed.

The problem was not only that when a media finishes it is stopping other ones played by the plugin, but it is also killing any sound played by another plugin or by WebView's default webaudio system, triggering a descriptive:

AVAudioSession.mm:646: -[AVAudioSession setActive:withOptions:error:]: Deactivating an audio session that has running I/O. All I/O should be stopped or paused prior to deactivating the audio session.

We found that AVAudioSession is a singleton for the whole app (cf. https://developer.apple.com/library/ios/documentation/AVFoundation/Reference/AVAudioSession_ClassReference/).

Apple says:

Most apps never need to deactivate their audio session explicitly. Important exceptions include VoIP (Voice over Internet Protocol) apps, turn-by-turn navigation apps, and, in some cases, recording apps.

(cf. https://developer.apple.com/library/ios/documentation/Audio/Conceptual/AudioSessionProgrammingGuide/ConfiguringanAudioSession/ConfiguringanAudioSession.html)

What testing has been done on this change?

• Played multiple sounds (non streaming) with the plugin, then waited for one to stop: it is not stopping other sounds anymore.
• Played multiple sounds (non streaming) with the plugin, then stopped one and released it: it is not stopping other sounds anymore.
• Played a sound (non streaming) with the plugin and another one via webview's webaudio, then waited the one played by the plugin to end: it is not stopping other sounds anymore.
• Played a sound (non streaming) with the plugin and another one via webview's webaudio, then stopped and released the one played by the plugin: it is not stopping other sounds anymore.
• Checked all possibilities with Xcode monitoring application: no memory issue detected.

Ran the integrated tests. Before doing any change (version 2.3.1-dev) the result was:

After the change the result is exactly the same:

All tests was made with an iPad2 on iOS8.4.

Before merging, it would be great if someone can test:

• an application using the recording feature
• an application using the streaming feature

Checklist

• ICLA has been signed and submitted to secretary@apache.org.
• Reported an issue in the JIRA database
• Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
• Added automated test coverage as appropriate for this change.

About the last point: I don't see what kind of tests we can add for this fix.

[cordova-qa]

Cordova CI Build has one or more failures.

Commit - Link
Dashboard - Link

Builder Name	Console Output	Test Report	Device Logs
Windows 8.1 Store	Link	Link	Link
Windows 10 Store	Link	Link	Link
Windows 8.1 Phone	Link	Link	Link
iOS	Link	Link	Link
Android	Link	Link	Link

[tbrebant]

What about this? Would it be possible to have a comment or an opinion?

[kaynz]

@cordova-qa +1 on this.

[katzlbt]

It looks like this fix includes the pull request:
"Fix app halt when an audio ends w/ another playing" #105

[katzlbt]

I have integared this patch into version 2.4.0 and tested playback with iOS10. It works.

[anagrath]

This issue is still on version 3.0.0 -- I am not sure if all setActive:NO should be commented out -- it seems that there is some logic to kill the audio playback when recording? I'd much prefer a stable version update than to do my own hacking... what can I do to help?

[katzlbt]

Sending setActiveNo: to an audio session will always stop recording and playback. This is how the system handles incoming phonecalls etc.

Most apps never need to deactivate their audio session explicitly. Important exceptions include VoIP (Voice over Internet Protocol) apps, turn-by-turn navigation apps, and, in some cases, recording apps. (https://developer.apple.com/library/content/documentation/Audio/Conceptual/AudioSessionProgrammingGuide/ConfiguringanAudioSession/ConfiguringanAudioSession.html)

Probably you are out of luck for waiting it out, unless you have a year or so. This PR and another that fixes that Audio is disabled after every memory warning are open for ages (#120). I apply the patches manually like this:

            wget https://github.com/apache/cordova-plugin-media/pull/120.patch
            patch -p1 <120.patch
            
            wget https://github.com/apache/cordova-plugin-media/pull/116.patch
            patch -p1 <116.patch
            
            wget https://github.com/apache/cordova-plugin-media/pull/100.patch
            patch -p1 <100.patch

[adriano-di-giovanni]

I've just submitted a #143 PR that solves the issue implementing an CDVSound#isPlayingOrRecording method. The method checks the status of player/recorder for every CDVAudioFile in soundCache. The method is used in conjunction with the avSession != null check wherever the plugin wants to deactivate avSession.

[iamrototo]

Hello, I have a similar problem for ages : after I record an audio, I can't play any other audio files (using default HTML5 audio feature).

I have succeed to fix this issue by doing a bit more work than you did.
You can see all the changes here:

iamrototo@ca374fd

[damienleroux]

+1. have the changes been merged?

I test it and it worked for me. I forked this repo and publish (cordova add cordova-plugin-media-with-fix@5.0.1-dev-updated-with-fix-2) my own plugin based on your fix.

Thanks All

[macdonst]

@damienleroux This PR has a merge conflict. It would probably be better for you to send a new PR with the required changes. You can reference this one so we can close it as well.

[kaynz]

@macdonst Is there any realistic chance that the new PR will be merged contemporary and not ignored for another year?

[damienleroux]

Done: #162.

Thank you :)

[adriano-di-giovanni]

@damienleroux, @macdonst are you talking about my #143 being merged? If so, I've just updated my PR to fix merge conflicts that were related to plugin.xml only. Please, let me know.

[damienleroux]

@adriano-di-giovanni I didn't see you submit a PR for this issue. May be more relevant that the commit from@tbrebrant that solves the issue: I don't know.

[adriano-di-giovanni]

@damienleroux It's the #143. Did I make any mistakes? I think it's a valid alternative to the one from @tbrebant. It's also backward compatible. Please, let me know.

[damienleroux]

@adriano-di-giovanni. I can't say if you made mistakes or not. Besides, @macdonst, my PR #162 seems to fail travis integration tests but don't know why.

[kaynz]

@damienleroux Pretty sure it's an environment bug and has nothing to do with your commit: https://travis-ci.org/apache/cordova-plugin-media/jobs/324476963#L1534

[macdonst]

Closing in favour of #143 or #162 which merge cleanly.