doublesymmetry
feat(web): add a web implementation
@dcvz here's the web implementation I've been working on. You can test it out via the following:
- Pull the branch, install deps, & build
-
yarn example web:start
This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
FYI: This has been unreviewed for about 8 months, but I decided to give it a try. I manually integrated all the web implementation parts into our app and made a few adjustments, such as preventing multiple Shaka instances being registered. I also added support for .pause (which was previously missing, super quick and dirty though) and ensured the emission of PlaybackProgressUpdated in that case.
Aside from that, the web component is quite solid. I'd love to see this integrated into the core (with some modifications) as it's very helpful and the API is almost identical.
In case someone is interested how I did that (it's ugly, since I had to copy some source files because of some resolving issues). https://github.com/showtime-xyz/showtime-frontend/compare/staging...feat/track-player
I really like RNTP (beside of some hot reload issues) and its a solid library. I would love to see this merged, otherwise I'd be forced to maintain another fork, which is not helpful for the ecosystem imho.
Some recap:
- pause was not working correctly
- pause is not emitting PlaybackProgressUpdates
- .add only accepts
Track[]while the native part also acceptTrack - multiple setupPlayer calls are not prevented
Pretty sure I did not test all of the cases (we only play a single track at a time), but I have fixed those above
FYI: This has been unreviewed for about 8 months, but I decided to give it a try. I manually integrated all the web implementation parts into our app and made a few adjustments, such as preventing multiple Shaka instances being registered. I also added support for
.pause(which was previously missing, super quick and dirty though) and ensured the emission ofPlaybackProgressUpdatedin that case.Aside from that, the web component is quite solid. I'd love to see this integrated into the core (with some modifications) as it's very helpful and the API is almost identical.
In case someone is interested how I did that (it's ugly, since I had to copy some source files because of some resolving issues). showtime-xyz/[email protected]/track-player
I really like RNTP (beside of some hot reload issues) and its a solid library. I would love to see this merged, otherwise I'd be forced to maintain another fork, which is not helpful for the ecosystem imho.
Some recap:
- pause was not working correctly
- pause is not emitting PlaybackProgressUpdates
- .add only accepts
Track[]while the native part also acceptTrack- multiple setupPlayer calls are not prevented
Pretty sure I did not test all of the cases (we only play a single track at a time), but I have fixed those above
Thanks for checking it out @hirbod. I'm glad to hear it's working for you! This is definitely on our radar, and @jspizziri is part of the external core contributors and has been keeping us up to date on the work here. We're hard at work on the v4 release but I'm planning on taking a look at this once that's out :)
@hirbod please submit PR's for all the work that you've done to improve things to my fork and I'll review and merge them in there.
I've released my fork under @5stones/react-native-track-player and that's what I'm currently using (see the 4.0.0-web.x tags. I'll be maintaining my fork until it gets merged into the mainline repo, so I'd recommend working with me there so you don't have to do it solo). I periodically rebase it on the upstream and publish a new version in addition to releasing new versions when I have bug fixes etc. As @dcvz said, the hope is that we'll ultimately get this reviewed and merged once 4.0.0 is out of RC, at which time I'd abandon my fork and we'd all switch over to mainline.
I'd greatly appreciate PR's for the issues you've identified to feat/web-3.
Thanks, @dcvz and @jspizziri, for the heads up. I'm glad to hear you guys have this on your radar.
@jspizziri, I regret not checking if your fork was actually published. Are you publishing from main? My codebase currently uses v3 for native, and, well, I suppose your implementation is from v4 :D
I can submit my minor bugfixes to your branch; they're straightforward.
@hirbod
Are you publishing from main?
It's... complicated 😄. But no, I'm maintaining feat/web-3 and publishing from publish. PR's are best sent to feat/web-3.
My codebase currently uses v3 for native, and, well, I suppose your implementation is from v4 :D
Yep, I'm using v4 (which I'd recommend for you as well). There aren't that many BC changes between 3 & 4 and we've published a number of RC's. It's pretty close to being a formal v4 release and many folks, use it in prod.
Regardless, any PR's you can send will make this a smoother process to get merged and released in the mainline in the coming months. Thanks!
@jspizziri we started working on this feature again and I'd like to push my PRs to your repo. Would you mind to rebase to upstream and prepare a new version so I can incorporate some of the fixes?
@jspizziri, after pulling in your branch, it appears that most of the issues I had are now resolved. I've PRs to your repo.
- Firstly, re-exporting was not possible (I've created a PR for this).
-
setupPlayerisn't preventing multiple instances. I'll created another PR to address this.
@hirbod , I just did a rebase. It was a little messy due to all the upgrades done recently in the example app. It seems to be working however, please let me know if you find issues.
@dcvz just an FYI. Some of the improvements in the example app recently have caused some issues with web. Notably some things with the @gorhom/bottom-sheet BottomSheetScrollViews and some of the newer package version of react-native-reanimated (they're going through a pretty big transition with their typescript support). I might've glossed over some other things.
@jspizziri what kind of issues? Like those packages are not supported on web? or just typescript issues?
@dcvz we discussed this on our call, but to leave a paper trail.
The issues seemed to be specific with some of the new minor versions of the react-native-reanimated and react-native-gesture-handler packages. There are some internal structural issues and typescript issues that I believe are likely going to be resolved by them in the near future, so I pinned them to earlier minor versions so we don't have to worry about working around those issues for the time being. There's nothing fundamentally incompatible with them and web though, just existing bugs that will be resolved soon.
Hi @jspizziri thank you for your work on adding support for the web platform.
However I'm facing error "You must call setupPlayer prior to interacting with the player." which I have already done so its working fine in android ios too.
I run your example from your fork and it worked on web only difference I could find was you are using webpack bundler and I am using metro bundler.
Is there any way to make it work with metro bundler on web platform?
@sajorahasan you're using the metro bundler on web? I didn't know people did that. You'll have to create a reproduction via a github repo that you share for me to be able to help. Although I must admit, it's probably not going to be a top priority of mine as it sounds like a niche thing.
Hi @jspizziri ohh I don't have much knowledge about react native web project. I am using ignite boilerplate and its have web platform support and every other packages used so far worked without much trouble with metro bundler.
I have uploaded demo project here: https://github.com/sajorahasan/WebTrackPlayerDemo If you have time please check that.
Hi @jspizziri I was able to fix the issue by replacing below import statement in Player.js
const shaka = await import('shaka-player/dist/shaka-player.ui');
This import statement was not resolving so I switched from import to require and moved it up top with other import statments like this.
const shaka = require('shaka-player/dist/shaka-player.ui');
I am not sure why import promise was not resolving with metro bundler.
Here is the patch file:
index 79a1ffb..94af21b 100644
--- a/node_modules/@5stones/react-native-track-player/lib/web/TrackPlayer/Player.js
+++ b/node_modules/@5stones/react-native-track-player/lib/web/TrackPlayer/Player.js
@@ -1,5 +1,8 @@
import { State } from '../../src/constants/State';
import { SetupNotCalledError } from './SetupNotCalledError';
+
+const shaka = require('shaka-player/dist/shaka-player.ui');
+
export class Player {
element;
player;
@@ -31,8 +34,7 @@ export class Player {
// shaka only runs in a browser
if (typeof window === 'undefined')
return;
- // @ts-ignore
- const shaka = await import('shaka-player/dist/shaka-player.ui');
+
// Install built-in polyfills to patch browser incompatibilities.
shaka.polyfill.installAll();
// Check to see if the browser supports the basic APIs Shaka needs.
@sajorahasan That dynamic import is required for SSR contexts. I'd recommend looking at something like nextjs for web.
@dcvz I just finished my audit of the web's API and here's what I found:
Currently Unimplemented
-
setQueue
Stubbed But Not Implemented
All the below stubbed methods can be seen here:
-
updateMetadataForTrack -
updateNowPlayingMetadata -
updateOptions
Deprecations
There are several deprecated methods that I didn't implement. I'm personally not in favor of implementing currently deprecated APIs.
Incorrectly Exposed
Currently, all of these getters/setters are public in the underlying Player class but maybe shouldn't be, we could make them protected easily enough to lock them down. However, they aren't exposed in the RNTP interface so getting at them would be a chore if someone wanted to.
-
current -
playWhenReady -
state
Once you've taken a look at this LMK how you'd like to proceed and I can raise any issues in the issue tracker that we need to and fix whatever you want.
I did however notice that Whip (HLS) fails to load the duration / progress.
Interesting, I looked into this and (at least on my device) the HLS wasn't even playing. It looks like it has to do with a limitation of the transmuxer used by shaka-player. I re-encoded the example HLS stream to be MPEG2-TS and it's working. Not sure what to make of this limitation itself or if things need to be reevaluated or if we just need to document it.
Steps To Test My Fix
Because the HLS streamed is served from the `docs` site you'll need to locally boot the `docs` project and serve a local copy when testing locally.- Modify
example/src/assets/data/playlist.jsonso that the Whipurkis"http://localhost:3001/example/hls/whip/playlist.m3u8" - Locally boot docusaurus at port
3001$ cd docs $ yarn start -p 3001
More urgently though skipping past Whip throws an error. Is it perhaps the media type - next track is a live stream.
Also interesting, the livestream is working for me now that I fixed the HLS problem, so I actually think it was related to the HLS problem and not the livestream. Going to do some more testing and I'll update this comment.
EDIT: yes, can confirm. The HLS stream error is actually resulting in this second error. Fixing the first fixed this one.
It looks like it has to do with a limitation of the transmuxer used by shaka-player.
Interesting. I guess for a first alpha version this doesn't have to be fixed.. but I'm guessing its gonna be an issue for people with HLS.. we may have to resort to hls.js here. However, I think the bigger issue is that it breaks the rest of the queue. That we should probably think about a bit further. Whether the existing API for recovering from errors is enough here.
@jspizziri perhaps a useful reference: uses shaka, dash.js and hls.js -- https://github.com/Eyevinn/html-player
@jspizziri perhaps a useful reference: uses shaka, dash.js and hls.js -- https://github.com/Eyevinn/html-player
I took a deeper look here and it's pretty interesting. It looks like they're using shaka-player for dash, dash.js for mss (the irony is not lost), and hls.js for hls. An interesting combination, and I'm guessing that the reason is that each of the libraries has limitations with a subset of the available streaming formats. Also, they don't seem to have support for good old-fashioned MP3s.
In our case, if we did something like this we'd effectively need to build and teardown the player for each track based on its type. Or instantiate multiple players and switch between them for each track. It's definitely possible, however it's quite a lot more complicated.
It might be more simple to see if we could submit a PR to mux.js to enhance its transmuxing capabilities.
Shaka definitely isn't perfect, however in terms of a single solution that supports all the various formats, DRM, etc, it definitely seems to be the best single solution. Open to being wrong about this, let me know what your thoughts are.
In my project where I control the files played, I wouldn't want to pull all those extra dependencies when I just need to play mp3s. So I'd suggest to let developers configure support for extra formats as needed.
In my project where I control the files played, I wouldn't want to pull all those extra dependencies when I just need to play mp3s. So I'd suggest to let developers configure support for extra formats as needed.
@SpadarShut , while allowing developers to configure support for extra formats sounds like an appealing option, the implementation of something like that would be quite tricky.
@SpadarShut , while allowing developers to configure support for extra formats sounds like an appealing option, the implementation of something like that would be quite tricky.
Sure, I get it. Just wanted to share my context, that it would be nice to avoid those extra deps if possible.
@dcvz I fixed the ci in this commit and I rebased on main. Apart from that I didn't touch anything (there was a merge conflict as a result of this commit).
I think we should be good to go once the iOS job finishes taking its sweet time.
@dcvz huzzah! What are your thoughts on a minor release?
Also, lmk if you'd like to see the docs site enhanced to incorporate the documentation (instead of just the readme).
@jspizziri i think I'll be making a minor release tomorrow which will include web -- if there are any things you'd like to add to the docs, let's do it. I can only think of a few places where we say we support iOS, Android and Windows.