eshaz / icecast-metadata-js

Browser and NodeJS packages for playing and reading Icecast compatible streaming audio with realtime metadata updates.
166 stars 20 forks source link

Redundant stream fetching when no metadata method set #161

Closed Yahav closed 1 year ago

Yahav commented 1 year ago

According to the docs, when specifying empty metadataTypes array no metadata will be parsed. In this case it is redundant to fetch the stream twice when using the HTML5 audio method.

eshaz commented 1 year ago

Good find in this! I agree if there is no metadata needed, then the first request should not happen for the HTML5 playback method. I'll get a fix added soon.

eshaz commented 1 year ago

Looking into this further, there doesn't seem to be much of a use case for using the HTML5 playback method without metadata. If someone wants to do this, they should just use the Audio element directly and avoid this library.

Some context: I added the html5 playback method as a way to play the stream with synchronized metadata before I added the webaudio method, which is preferable because it only uses one request. The html5 method exists now as a fallback when both the mediasource and webaudio methods don't work, which I don't think is likely to happen anyway.

Closing this issue since the additional complexity of avoiding the redundant request doesn't justify the value added.

Yahav commented 1 year ago

I understand, however, as i said before, in my honest opinion what you've created here is much more than the metadata fetching capabilities, this is currently the ONLY cross platform, cross encoding player with a unified API that is capable to play Opus streams in iOS/MacOS devices.

Let me try and explain why i think that HTML5 audio is very important. Basically, native encoding/playback is always far better than any other method, it make sense to allow native playback whenever possible while falling back to other method when required. such is my case usage for example, i want to allow Opus playback nativally on devices that supports it and fallback to other methods on devices that don't, eg. iOS/MacOS. Now, as for Web Audio, it seem to load js based decoders even on platform that does support native playback which isn't a great idea and that is why the HTML5 audio integration is very important.

I don't want to negate the vision you had for this package but as i said, you've created a small miracle here :) And so, the above and the ability to completely disable the metadata mechanisms without even loading the required libraries for those will be a huge deal.

eshaz commented 1 year ago

I think that goal is still accomplished in this library without this fix.

There are three playback methods, mediasource, webaudio, and html5.

In every case, except Vorbis on iOS, or some really unusual platform, the library will use mediasource or webaudio. If you have a platform that has to use html5 given the above information please let me know, since I am not aware of what that would be. You can check the compatibility table here.

As always, metadata can be disabled by setting metadataTypes to [].

Yahav commented 1 year ago

Isn't the mediasource require an additional library (eshaz/mse-audio-wrapper)? Or is it fetched only when a native decoder isn't available?

eshaz commented 1 year ago

It does, but the MediaSource api uses the native decoders. The mse-audio-wrapper just remuxes the audio into formats compatible with the api.

Yahav commented 1 year ago

Hello Eshaz

So, this won't be fixed? It just seem a rather simple an quick fix that its a shame not to implement.

eshaz commented 1 year ago

No, I won't be fixing it at this time. If you want to put in a PR that fixes this, you're more than welcome to.