You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by romedius <gi...@git.apache.org> on 2016/11/04 13:24:14 UTC

[GitHub] cordova-plugin-media pull request #120: CB-11513: (ios) Fixed: iOS memory wa...

GitHub user romedius opened a pull request:

    https://github.com/apache/cordova-plugin-media/pull/120

    CB-11513: (ios) Fixed: iOS memory warning stops sound, but does not send status to JS-client

    ### Platforms affected
    iOs
    
    ### What does this PR do?
    If iOS issues a memory warning while playing a sound, all sounds are terminated. However the ios/CDVSound.m plugin does not update the state of its JS-clients, so the last state the Client knows is Media.MEDIA_RUNNING, which is untrue after a memory warning. This results in incorrect clientside status.
    
    Problem: A user's software component may send pause() instead of play() (toggle button) effectively rendering the sound unplayable without recovery. This happened in our case.
    
    Fix: Do not discard currently playing sounds. The first memory warning arrives exactly when the App enters the yellow area and consumes 500MB however it may consume 750MB and more according to Xcode so terminating sounds on "memory warning" is incorrect behavior.
    
    ### What testing has been done on this change?
    Tested on iOs simulator.
    How to reproduce  the Issue: Add 1 or 2 large animated GIFs with 200+ frames to the UIWebView, this will consume 500MB quickly while playing the GIFs, or simulate the memory warning using the iOS Simulator while playing a sound.
    
    ### Checklist
    - [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
    - [x] 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.
    - [x] Added automated test coverage as appropriate for this change.
    
    https://issues.apache.org/jira/browse/CB-11513

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/romedius/cordova-plugin-media patch-1

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-plugin-media/pull/120.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #120
    
----
commit ad25d1059c44ce2229c40dabf6b7136f89356952
Author: Romedius Weiss <ro...@gmail.com>
Date:   2016-11-04T13:17:04Z

    Fix for CB-11513
    
    https://issues.apache.org/jira/browse/CB-11513

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-media issue #120: CB-11513: (ios) Fixed: iOS memory warning s...

Posted by pincombe <gi...@git.apache.org>.
Github user pincombe commented on the issue:

    https://github.com/apache/cordova-plugin-media/pull/120
  
    I would also love to see this get merged. It works and it is crazy that a memory warning would stop the audio from playing.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-media issue #120: CB-11513: (ios) Fixed: iOS memory warning s...

Posted by cordova-qa <gi...@git.apache.org>.
Github user cordova-qa commented on the issue:

    https://github.com/apache/cordova-plugin-media/pull/120
  
    Cordova CI Build has completed successfully.
    
    **Commit**     - [Link](https://github.com/apache/cordova-plugin-media/pull/120/commits/ad25d1059c44ce2229c40dabf6b7136f89356952)
    **Dashboard** - [Link](http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83/)
    
    | Builder Name  | Console Output | Test Report | Device Logs  |
    |     :---:     |     :---:      |   :---:     |     :---:    |
    | [Windows 8.1 Store]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-8.1-store/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-8.1-store/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-8.1-store/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-8.1-store/artifact/) |
    | [Windows 10  Store]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-10-store/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-10-store/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-10-store/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-10-store/artifact/) |
    | [Windows 8.1 Phone]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-8.1-phone/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-8.1-phone/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-8.1-phone/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=windows-8.1-phone/artifact/) |
    | [iOS]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=ios/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=ios/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=ios/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=ios/artifact/) |
    | [Android]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=android/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=android/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=android/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-pr/83//PLATFORM=android/artifact/) |
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-media issue #120: CB-11513: (ios) Fixed: iOS memory warning s...

Posted by romedius <gi...@git.apache.org>.
Github user romedius commented on the issue:

    https://github.com/apache/cordova-plugin-media/pull/120
  
    @stevengill if you have some spare time, could you take a look at this patch too, or point me to the right maintainer?
    Thank you!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-media issue #120: CB-11513: (ios) Fixed: iOS memory warning s...

Posted by romedius <gi...@git.apache.org>.
Github user romedius commented on the issue:

    https://github.com/apache/cordova-plugin-media/pull/120
  
    original author of this bugfix: @katzlbt


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-media issue #120: CB-11513: (ios) Fixed: iOS memory warning s...

Posted by ghenry22 <gi...@git.apache.org>.
Github user ghenry22 commented on the issue:

    https://github.com/apache/cordova-plugin-media/pull/120
  
    I have merged this into my project and tested and confirm that it does what it says on the box.
    
    Before: simulate memory warning on the iOS simulator and audio stops instantly on the native side.  App continues to function otherwise but no longer plays any audio
    
    After: simulate a memory warning on the iOS simulator and audio continues to play as expected, all works as normal.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-media issue #120: CB-11513: (ios) Fixed: iOS memory warning s...

Posted by ghenry22 <gi...@git.apache.org>.
Github user ghenry22 commented on the issue:

    https://github.com/apache/cordova-plugin-media/pull/120
  
    could this get merged? I have been maintaining a forked plugin for months with this PR merged and it works well.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org