[GitHub] [cordova-plugin-media] klaw-PLS opened a new pull request #328: (Android) Correcting issue #325.

[GitBox <gi...@apache.org>]

klaw-PLS opened a new pull request #328:
URL: https://github.com/apache/cordova-plugin-media/pull/328


   As stated in issue #325, it looks like PR #317 missed a spot in the corrections it was making.  This commit adds that fix to the missed location. You can find more info in #325.
   
   <!--
   Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
   
   http://cordova.apache.org/contribute/contribute_guidelines.html
   
   Thanks!
   -->
   
   ### Platforms affected
   
   Android
   
   
   ### Motivation and Context
   <!-- Why is this change required? What problem does it solve? -->
   <!-- If it fixes an open issue, please link to the issue here. -->
   
   Fixes #325.
   
   ### Description
   <!-- Describe your changes in detail -->
   
   When debugging through the plugin code, I was able to determine that when a simple file name is provided for the media.startRecord() API a file path is eventually generated in the createAudioFilePath() method using either getExternalFilesDir() or getCacheDir(). In my emulator, it creates a temporary file path like the following: /storage/emulated/0/Android/data/com.<org>.<appName>/files/tmprecording-###.3gp. (The logic for creating this temporary file path was updated in PR #317.)
   
   However, when a simple file name is provided to the media.play() API, a different file path is created instead. The logic is located in the loadAudioFile() method, but for my situation it eventually calls the Environment.getExternalStorageDirectory() to create the file path. I do not have the file path on hand at the moment, but it does not match the file path created via the createAudioFilePath() method. (I am just now realizing that PR #317 specifically was trying to remove getExternalStorageDirectory() and perhaps this use was overlooked?)
   
   To work around this issue, I just updated line 709 of AudioPlayer.java to read the following instead:
   
   //this.player.setDataSource(Environment.getExternalStorageDirectory().getPath() + "/" + file);
     this.player.setDataSource(createAudioFilePath(file));
   
   Using this workaround, I no longer have an error code thrown, and when I load my app onto an actual device, I am able to record and playback audio just fine. (For some reason, recording on my emulator does not pick up my microphone input.) My main concern with this approach is that, if I am understanding it correctly, the media.play() API is supposed to be able to accept a much larger range of audio sources compared to media.startRecord(). This leads me to be concerned that reusing code from media.startRecord() will somehow restrict media sources that should otherwise be acceptable.
   
   ### Testing
   <!-- Please describe in detail how you tested your changes. -->
   
   Here is a snippet from my test program. The following functions are connected to the onclick event of buttons on my screen:
   
   function recordAudioButtonPressed() {
       var platform = device.platform;
       var mediaType = platform === "Android" ? "aac" : "m4a";
       var src = "testRecording." + mediaType;
       var mediaRec = new Media(
         src,
         function () {},
         function (err) {
           navigator.notification.alert(
             "An error occurred while recording audio: " + err.message,
             function () { },
             "Error",
             "Ok");
         });
   
       mediaRec.startRecord();
       setTimeout(function () {
         mediaRec.stopRecord();
         mediaRec.release();
       }, 3000);
     }
   
     function playAudioButtonPressed() {
       var platform = device.platform;
       var mediaType = platform === "Android" ? "aac" : "m4a";
       var src = "testRecording." + mediaType;
       var mediaPlay = new Media(
         src,
         function () {
           mediaPlay.release();
         },
         function (err) {
           navigator.notification.alert(
             "An error occurred while playing audio.",
             function () { },
             "Error",
             "Ok");
         });
   
       mediaPlay.play();
     }
   
   I am encountering this issue on Android, using emulated devices running API 29 and 30, as well as a Samsung Galaxy S9+ running Android 10 (which should be API 29 iirc).
   
   
   ### Checklist
   
   - [ ] I've run the tests to see all new and existing tests pass
   - [ ] I added automated test coverage as appropriate for this change
   - [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
   - [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
   - [x] I've updated the documentation if necessary
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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