ryanheise
Added support for HLS title and artist metadata
Hi, I've added support for HLS metadata as described in #732. Please let me know if you can proceed pulling this asap or if any further change is needed. Thanks.
Thanks for the pull request! Do you have a test HLS URL I can test this on?
One other thing is that I'll need to have a think about the API. The current API is based on Android's ExoPlayer which provides a different API for each type of metadata, and currently on the ICY metadata API was implemented. So to be consistent with that approach, it may involve creating more datatypes. However, it may be better at this juncture to consider unifying all of these different metadata APIs into a single metadata API. It will require some more thinking.
I've used this streaming URL for development: https://mdstrm.com/audio/60a275d1f943100826374a86/live.m3u8
About the API, the only thing that sounds weird to me is the Icy naming. That prefix may be erased, but I don't think that a breaking change is worth it. About ExoPlayer, I'm not using the android version at the moment, but ExoPlayer does have a unified API for all streaming formats with the onMediaMetadataChanged callback.
Hi @ryanheise, I've just updated this PR with support for HLS metadata on Android as well. Please check if this new commit fits well with your original philosophy, and let me know how can I help to get this PR merged soon. Thank you.
I'm wondering if onMediaMetadataChanged is the unified API as you said, perhaps we don't need onMetadata anymore? The documentation seems to indicate that in this unified API, data from MediaItem.mediaMetadata will override any data coming from onMetadata but in the case that both sources provide the same metadata keys, the values will probably also match.
I can confirm that I'm not using just_audio but a custom made player for Android, and I'm only using onMediaMetadataChanged for gathering both ICY and HLS metadata just fine.
I don't know if onMediaMetadataChanged is meant to replace the onMetadata callback for every use case, but as per ExoPlayer docs, MediaMetadata is kind of a combination of multiple Metadata entries in a generic approach, while Metadata allows access to raw metadata as in a key-value fashion.
It appears that not all ICY metadata is copied into MediaMetadata, e.g. the bitrate and interval. So perhaps the best approach would be to introduce a new unified metadata API alongside the existing ICY API without replacing it. I will understand if you're not up for modifying your own PR in that way, but if not I could perhaps have a go at it at some point.
I don't think that ExoPlayer specifics should impact how this library behaves. My take is that if title and artist metadata is being sent on the current API, then it should work for every audio format. For me, the main issue of this API is only about the "icy" naming. I would like for a library to abstract completely of audio format specifics.
It sounds like you still don't object to having a new unified API, but you would prefer I get rid of the old one as well so there are no longer any IcyMetadata, IcyInfo or IcyHeaders classes. We can reassess whether to phase the Icy-specific API out in the future, it wouldn't get removed suddenly, it would be planned and be flagged with "deprecated" in advance.
If I were to implement the new universal API now, I'd build it on top of ExoPlayer's onMediaMetadataChanged out of ease. I wouldn't want to get bogged down in the beginning trying to incorporate all of the Icy metadata from the HTTP headers that is currently not reflected in ExoPlayer's onMediaMetadataChanged, so there is some benefit to a slow deprecation of the old API. It gives me time to sort out those details later.
Hi Ryan, I just sync'd this pull request with the latest commits from minor. Could you please revisit this pull request and consider merging it?
Thank you!