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: added support to remote files #141

Merged
merged 2 commits into from
Apr 21, 2017
Merged

feat: added support to remote files #141

merged 2 commits into from
Apr 21, 2017

Conversation

TheGhoul21
Copy link
Contributor

  • Sound.js now checks if is RelativePath using also http and https
  • RNSoundModule now uses setAudioStreamType(AudioManager.STREAM_MUSIC)
    to stream music
  • Exception now are handled differently

- Sound.js now checks if is RelativePath using also http and https
- RNSoundModule now uses setAudioStreamType(AudioManager.STREAM_MUSIC)
to stream music
- Exception now are handled differently
@TheGhoul21
Copy link
Contributor Author

#134

mediaPlayer.setDataSource(fileName);
} catch(IOException e) {
Log.e("RNSoundModule", "Exception", e);
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice - streaming is a good idea over http for sure.

mediaPlayer.setDataSource(fileName);
} catch(IOException e) {
Log.e("RNSoundModule", "Exception", e);
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you have a specific reason not to invoke a callback error when the setDataSource throws an exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it usually throws an exception when internet is missing, or when the file does not exist. Do you suggest to handle differently those two? (returning null here, will invoke the error callback anyway later on with a more generic "Resouce not found" error)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@TheGhoul21 ah - thanks, I didn't notice that it'd hit the error callback anyway. Thanks for the clarification. This seems a sensible solution

@matthieugayon
Copy link

Any chance this gets merged soon ?

@benvium
Copy link
Collaborator

benvium commented Apr 18, 2017

@matthieugayon Let me know if it works for you, and if so we should merge it.

@matthieugayon
Copy link

Ok I'll try to get back to you asap on that, most probably tomorrow

@ndbroadbent
Copy link
Contributor

ndbroadbent commented Apr 19, 2017

@TheGhoul21 can I ask, do you know what kind of caching would be happening here? Would Android be caching the data at all, or redownloading for each new mediaPlayer?

The reason I'm asking: I want to build "code push" feature for my app's sounds, so that I can add or update sounds without needing to push a new version to the app store. I'm working on a game, so it would be amazing if I could immediately push new levels + images + sounds.

EDIT: Ignore me! I should have set up CodePush before commenting. I just set it up, and was surprised to discover that it can already manage my sound files out of the box, thanks to React Native's asset manager. It's pretty incredible that I can just push out new sounds to my users.

@gshotwell
Copy link

+1 for this getting merged, it's the missing link for react native audio playing

@ndbroadbent
Copy link
Contributor

ndbroadbent commented Apr 20, 2017

This is actually great timing. As I mentioned, I've been setting up CodePush, and switching to React Native's asset manager. So I just realized that I actually do need this, because now I get a URL during development:

"http://10.0.3.2:8081/assets/src/lib/sounds/alert.mp3?platform=android&hash=442aeb48612c0aed468b12f4e2ca01bc"

It also might be good to add "Load sound from React Native Static Resources" to the feature matrix. I didn't realize it was possible to play sounds this way, until I stumbled onto that page.

@SteffeyDev
Copy link

SteffeyDev commented Apr 20, 2017

I just replaced react-native-sound with TheGhoul21/react-native-sound in my cross platform app and can confirm that that Android load from network works identical to iOS now. It should probably be in the readme somewhere that the syntax to load from network is
var audio = new Sound(networkURL, null, (error) => {});
where networkURL is a string.

@benvium benvium merged commit c417354 into zmxv:master Apr 21, 2017
@benvium
Copy link
Collaborator

benvium commented Apr 21, 2017

@SteffeyDev Thanks for the confirmation. I've merged it now. Great job @TheGhoul21

@familyfriendlymikey familyfriendlymikey mentioned this pull request May 3, 2017
aidan-doherty pushed a commit to aidan-doherty/react-native-sound that referenced this pull request Oct 26, 2020
* feat: added support to remote files

- Sound.js now checks if is RelativePath using also http and https
- RNSoundModule now uses setAudioStreamType(AudioManager.STREAM_MUSIC)
to stream music
- Exception now are handled differently

* fix: amended README specifying support for android network streaming
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.

6 participants