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

v1.0.0 #713

Merged
merged 16 commits into from
Apr 1, 2021
Merged

v1.0.0 #713

merged 16 commits into from
Apr 1, 2021

Conversation

antoine-pous
Copy link
Collaborator

@antoine-pous antoine-pous commented Dec 21, 2020

This is a work in progress, it need some fixes and requires a lot of tests.

What's new:

  • Moved from JS to Typescript (ES6)
  • Removed legacy JS lib and types definitions
  • All functions which call the Java bridge use Promises
  • Added an opt-out option rejectOnUnsupportedFeature {Boolean} which ensure that using an unsupported feature will be rejected. If disabled (default behavior) the function will be resolved
  • Added deterministic state functions:
    • enable / disable
    • setSpeakerphoneOn / setSpeakerphoneOff
    • enableInSilenceMode / disableInSilenceMode
    • setActive / setInactive
  • Added SoundBasePath type which can be 'MAIN_BUNDLE' | 'DOCUMENT' | 'LIBRARY' | 'CACHES' | string
  • Added SoundOptions interface with rejectOnUnsupportedFeature?: boolean and enableSMTCIntegration?: boolean
  • Implemented PR Android - Set Volume Button Controls On Android Audio Stream #693 fix: xcode 12 compatibility #704
  • Now RNSoundModule.createMediaPlayer will execute the callback error with more explicit messages. Resource not found covered too much errors, this update will give a better view of what's happening exactly. Resource not found remain as last error when the function is not able to determine which action is requested, maybe should we put a better error message. Error payload have also a new key named resource which provide the full path of the fetched file to enhance debugging.

Known issues

  • when IsAndroid is true predefined directories are undefined using React Native with Expo

Remaining changes:

  • Updating the documentation
  • Updating the changelog
  • Unit tests & CI
  • Implement PR which are awaiting for approval

This is clearly a braking change and i don't see any reason to continue to support legacy stuff when most of supported versions of RN, by this lib, are not supported anymore. If people are not able to upgrade their dependencies and follow the new standards it's sad but IMO a library must remain updated, they can still use legacy versions of the library if needed.

Feel free to suggest any changes, i need this update then i'll finish it ASAP

To test it in your projects:

Using Yarn: yarn add antoine-pous/react-native-sound#1.0.0
Using NPM: npm install git://github.com/antoine-pous/react-native-sound.git#1.0.0 --save

Usage example

const sound: Sound = new Sound('sound.mp3', 'MAIN_BUNDLE', {rejectOnUnsupportedFeature: true})
sound.isLoaded().then(() => {
  sound.play()
})

@antoine-pous antoine-pous marked this pull request as draft January 5, 2021 12:59
@antoine-pous antoine-pous marked this pull request as ready for review January 5, 2021 12:59
@antoine-pous antoine-pous marked this pull request as draft January 5, 2021 12:59
@sasweb
Copy link

sasweb commented Jan 12, 2021

Hey @antoine-pous!
Happy that someone takes care here. Are you one of the new maintainers?
Can you already tell a rough time estimation when this can be merged?
Thanks for your effort.

@antoine-pous
Copy link
Collaborator Author

antoine-pous commented Jan 12, 2021

@Pawelsas I'm not a maintainer, actually I need some help to test everything and see how to implement CI.

I'm new to React Native then many things remain a bit hard to understand.

Feel free to use it following instructions and report any issue directly on this PR, I'll check about them ASAP :)

@sasweb
Copy link

sasweb commented Jan 12, 2021

@antoine-pous really cool! I haven't looked in the implementation details yet. Are you planning to update the preloading and memory handling on Android as well? After some research I noticed that audio preloading and/or playback will fail at some point when you either preload to many sounds or play too many sounds within one session.

@antoine-pous
Copy link
Collaborator Author

I never reached it, then i don't know how to deal with it :')

@RomualdPercereau RomualdPercereau marked this pull request as ready for review April 1, 2021 08:43
@RomualdPercereau
Copy link
Collaborator

Thank you Antoine for your amazing work. It looks like a very good start for keeping react-native-sound.
@Pawelsas do you have any demonstration of memory failure when loading too many sounds?

@antoine-pous
Copy link
Collaborator Author

@RomualdPercereau I've not updated this PR since a while, but I think it will be more easy to fix any issue now. My project moved from RN to UE4, but don't hesitate to ping me if you need some help for fixes 👍

I hope this PR will ensure the future of this package :)

@antoine-pous antoine-pous deleted the 1.0.0 branch April 1, 2021 15:15
@antoine-pous antoine-pous restored the 1.0.0 branch April 1, 2021 15:15
@antoine-pous antoine-pous deleted the 1.0.0 branch April 1, 2021 15:15
@RomualdPercereau
Copy link
Collaborator

Thank you @antoine-pous, your work is a very good start for the next versions. I moved it in a new branch : https://github.com/zmxv/react-native-sound/tree/1.0.0.

I was wondering why, the functions not related to a specific sound like: enableInSilenceMode, setMode were in the Sound class? So I moved them outside the class to be able to use them globally, also changed Sound to default export. (199f2da)

It should probably be the same for setSpeakerphoneOn and setSpeakerphoneOff do you have a clue to avoid these functions to be related to a sound?

The new usage would be:

import Sound, { enableInSilenceMode } from "react-native-sound"

const sound: Sound = new Sound("test.mp3", "MAIN_BUNDLE")
sound.isLoaded().then(() => {
  sound.play()
})

enableInSilenceMode()

@antoine-pous
Copy link
Collaborator Author

antoine-pous commented Apr 21, 2021

Maybe would it be better to wrap those functions in Sound class and move tracks in a Track class, then into the Sound class you maintain a map of the whole loaded sounds and everything can be managed from the Sound class :)

@RomualdPercereau
Copy link
Collaborator

Yes, could be a way to have a dedicated namespace for these functions. It could give this kind of implementation. I don't know if Sound should be default in that case? @antoine-pous

import Sound, { Track } from "react-native-sound"

const track: Track = new Track("test.mp3", "MAIN_BUNDLE")
track.isLoaded().then(() => {
  track.play()
})

Sound.enableInSilenceMode()

@RomualdPercereau RomualdPercereau linked an issue Apr 22, 2021 that may be closed by this pull request
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.

3 participants