Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add duration, currentTime, file url, oncomplete #10

Closed
wants to merge 19 commits into from

Conversation

kheftel
Copy link
Contributor

@kheftel kheftel commented Jul 9, 2020

Added some new features, fixed some bugs. Rebased it on your most recent 0.1.3 release. Hope it's not too much for one PR. Implemented and tested on iOS and Android.

Added the ability to get the duration of a loaded audio file. Restricted it to only preloadComplex sounds with 1 channel for simplicity.
Added the ability to get the current time of a playing audio file for progress listeners / updates. Restricted it to only preloadComplex sounds with 1 channel for simplicity.
Added a complete event at the end of a (complex) playing audio file.
Added the ability to ask if a particular audio is loaded (since the unload event now throws an error on android if a file is already loaded).

README.md Outdated
@@ -112,18 +115,20 @@ NativeAudio.preloadSimple({
/**
* Platform: Android/iOS
* This method will load more optimized audio files for background into memory.
* @param assetPath - relative path of the file or absolute url (http://)
* @param assetPath - relative path of the file or absolute url (file://)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I was in the impression that I could use this plugin with external resources too.
The preloadSimple comment still contains "http://" though.

Maybe even add a remark to the readme that this plugin only works with local files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I can do that.

actually I'm pretty sure that before this PR, you could only load assets from the app bundle / resources, as I tried to use file URLs and failed, and thus added the isUrl parameter to get them to work (but only for preloadComplex).

My use case is downloading audio files to the app cache directory and playing them from there, hence I added support for file URLs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added that second commit, let me know what else I can do.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍 Now we wait for someone to approve this PR 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good!

@kheftel kheftel mentioned this pull request Jul 24, 2020
@kheftel
Copy link
Contributor Author

kheftel commented Aug 4, 2020

I added a documentation change to this PR to clarify, per #9, that on Android, your asset file must be in res/raw and you must pass a filename w/o extension for preloading to work properly.

@jsmith jsmith mentioned this pull request Sep 21, 2020
@yacut
Copy link

yacut commented Sep 21, 2020

@Boosten @priyankpat Any update on this?

@kevinboosten
Copy link

@Boosten @priyankpat Any update on this?

@priyankpat has to approve and merge the PR :-)

@damngamerz
Copy link

This is really cool can we merge these changes @priyankpat

@danielehrhardt
Copy link

Please merge

@kheftel kheftel mentioned this pull request Feb 3, 2021
@taylorchance
Copy link

Please merge, I pulled his repo locally and it worked perfectly for me. This is a must needed feature and there aren't many other options.

@HunSpeedi
Copy link

Also leaving a tactical dot for a merge request: .
Thanks in advance @priyankpat & @kheftel :)

@kheftel
Copy link
Contributor Author

kheftel commented Feb 26, 2021

@priyankpat if you add me as a maintainer I'll get this merged.

@thomasvidas
Copy link

@kheftel, are you on the Capacitor Slack? If you are, DM me on there (@thomasv) and I can get you set up as a maintainer for this repo if you are interested 😄

@kheftel
Copy link
Contributor Author

kheftel commented Mar 16, 2021

@thomasvidas, I'm not, but am joining now.

@bazuka5801
Copy link
Contributor

All these changes add in new version!

@bazuka5801 bazuka5801 closed this Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants