Feature upgrade to take advantage of v5 Cadence features

Cadence v5 comes with plentiful new features that we can and should take advantage of.

This should have several subcomponents:

Replace current one-step-request with Cadence's bestmatch API (and trust the server's opinion of relevance over our own; As with search, we should improve the experience for all users, not just CadenceBot.
If possible, add a command to display the album art for the currently playing song (Cadence returns a base64'd image, if Discord allows us to display it then we should do so)
Consider adding a listener count?
- Cadence will return its own, which counts CadenceBot as one listener - Conceivably, we could parse our own data structures and return something like "Cadence has 17 direct listeners, and there are 5 servers currently listening to CadenceBot"?
Replace the configured stream URL with fetching the listenurl from Cadence.
- Conceivably, we could provide a config mechanism to allow overriding this - default-config sets it to false, and if you change it to a string, we use that instead?
Similarly, consider whether we should grab Cadence's stream bitrate and rebroadcast in the same quality by default (Requires Cadence v5.1+).
Support the history command to get the play history (Requires Cadence v5.1+)
Consider replacing polling for metadata with support for the radiodata/sse event source - If this is not trivial (though I expect it to be fairly easy), this can be pushed out to expedite an official v1.6.0 release (making this 1.6.1)

@kenellorando, do feel free to comment on my plans to use all your new stuff.

Something I considered, but didn't include, was doing some "version negotiation" sort of trickery where the bot checks the server version on startup and uses it to enable/disable an API like history (if we don't see a version that's at least 5.1.0, don't offer that command). That'd be interesting, but so far CadenceBot has been somewhat tied to Cadence versions - I don't try to support backwards compatibility to run against a Cadence v2 API in 1.5.x, for example. So the question becomes whether we expect the API to change often enough that CadenceBot might need to navigate different server versions without upgrade...

@za419 I'll think through each from a devil's advocate perspective of a Discord server owner, henceforth "user".

Replace current one-step-request with Cadence's bestmatch API (and trust the server's opinion of relevance over our own; As with search, we should improve the experience for all users, not just CadenceBot.

Yes to this, not so much that a user would find it any functionally different, just that it's might be the easiest, most conspicuous way to simplify your code.

If possible, add a command to display the album art for the currently playing song (Cadence returns a base64'd image, if Discord allows us to display it then we should do so)

Not sure about a single command for artwork. If we imagine how CadenceBot is actually going to get used in the server, we should ask "are Discord audiences going to realistically be wanting to see the album art alone?", to which I think maybe not. What I think could be an ideal use of the album artwork is simply to integrate it with the nowplaying command, so it displays all information (title, artist, artwork all at once). In any case, implementation might be straightforward if we know what the best way to approach converting the encoded data to an image is. I wonder if we need to convert it into a file then do a Discord file upload?

Since album art can potentially get large, we need to be careful about the volume of data we pull from the Cadence deployment. Spamming the command in a discord channel might consume enough bandwidth from the radio to DDOS it. Maybe the bot should cache it once. Also maybe I should probably put the rate limiter on top of Cadence's artwork API itself.

Consider adding a listener count? Cadence will return its own, which counts CadenceBot as one listener - Conceivably, we could parse our own data structures and return something like "Cadence has 17 direct listeners, and there are 5 servers currently listening to CadenceBot"?

Another maybe, because it could be perceived as a flaw if a voice channel with ten people in it and the bot returns There is one listener. It's technically correct but may potentially be seen as an inaccurate feature.

Replace the configured stream URL with fetching the listenurl from Cadence. Conceivably, we could provide a config mechanism to allow overriding this - default-config sets it to false, and if you change it to a string, we use that instead?

Yes, this one is 100% must do. It's a core change prerequisite for the integration idea of "each stack gets its own bot".

Similarly, consider whether we should grab Cadence's stream bitrate and rebroadcast in the same quality by default (Requires Cadence v5.1+).

Maybe, but I don't think people would be able to hear the difference in Discord audio quality (defaults to 96kbps for non-paying users).

Support the history command to get the play history (Requires Cadence v5.1+)

Definitely. High tier change.

Consider replacing polling for metadata with support for the radiodata/sse event source - If this is not trivial (though I expect it to be fairly easy), this can be pushed out to expedite an official v1.6.0 release (making this 1.6.1)

No opinion: I also have no idea if this would be easy or not for a Discord bot, but I can at least say it certainly made the browser UI code simpler and more accurate. One important note: cadenceradio.com sometimes experienced SSE connections dropping after long enough periods of time, stopping automatic updates, so I also had to implement some code to automatically attempt reconnect. Why the connections drop, I haven't figured it out yet. It might just be a symptom of running it on a Kubernetes cluster. I can't tell yet. I guess overall, I wouldn't prioritize this one over others if existing code works fine. So we can try implementing this, just with care).

I just realized the Bot should have an incredibly better development experience now with the advent of containerized Cadence.

Is there a ELI5 way for this so I can contribute an opinion, or is there documentation I can fixate on since I'm in fixation phase this week

@kenellorando I shall reply to you later today to further conversation and narrow down the scope further for this issue before I get to it.

I can also break it down if someone else is interested in helping out after #103 gets closed, but for obvious reasons 103 is the priority.

@magix430, feel free to ask questions to clarify things. I will (briefly) use some of my lunch break to infodump in your direction to hopefully make this a little clearer to someone less familiar with the ecosystem, though I doubt I'll hit ELI5 levels of simplicity (in all due fairness, I do write in an unnecessarily verbose style, so the prospect of anything I say qualifying is rather unlikely).

Documentation

@kenellorando, in his finite wisdom, has finally provided an actual API reference so there's a better way to see how the Cadence server talks to clients than inspecting the source code (thanks again, Ken!). In addition, you could check his release history, where in due character he's provided a good, if concise, changelog, with links to all the PRs making these changes where he goes into excellent depth with what he's doing - As, of course, is characteristic of Ken's diligence to history.

If you do go that route, the last supported version of the Cadence server for CadenceBot 1.5.x is also the last v4.x Cadence (because semver) - That being 4.3.3. Everything since then is new - There is a lot of "noise" from UI changes and architectural improvements that CadenceBot as a client doesn't care about (even though we as Cadence devs should).

More detailed explanation of my scope

All line references to code will be done against current master, as of a6ce2eb9458e037254fc92301c3645eff5aa4efd.

Using `bestmatch`

CadenceBot supports a feature called one-step-request - A traditional request follows the flow of:

Cadence search <song> - Look at the list, choose the number for the song you want
Cadence request <number>

one-step-request adds automation, so the bot can automatically follow this flow (bot.js lines 926-1102). This is one of the parts where we use a rather obscure trick of "mock messages", where we invent a message out of thin air and submit it to ourselves for processing, because my code architecture in CadenceBot is trash and this is the only way to reuse code from another command. But, it works, as long as the bot can automatically choose a single song from the list (see bot.js lines 1902-2004 for the definition of how we do so) - Or, the bot will return the list of options it can't choose from and ask you to follow step 2 of a traditional request to complete the job.

Cadence v5 adds the bestmatch API, which is essentially a serverside one-step-request - Though it has different semantics. We pull all database results, then try to use some pattern matching to try to pick a good match (For example, if you search for "Only my Railgun by fripSide", and we actually taught the v5 server to respond to that, which is something we lost from v3 and v4, CadenceBot would automatically select a song whose title matches 'only my railgun', not obeying case or punctuation, and whose artist matches 'fripside', not obeying case or punctuation), and fail if we cannot confidently state one choice, while the server just grabs the song the database ranked highest.

Now, I'll be frank and say I prefer the one-step-request semantic, but my thought, with @kenellorando's buy-in, was that instead of implementing the same feature two ways, we should have CadenceBot respect the server's behavior and improve the server to match 1.5.x CadenceBot (to whatever extent our behavior is desirable).

Of course, we may also decide that these are similar, but not identical - The bot does prefer to ask for user input if it cannot confidently choose a single song, while the server just wants to play something, even if it is ambiguous - And maybe we decide that's desirable and we keep things as they are. But, if we choose to do that, it should be on purpose, not because we're too lazy to support bestmatch, so I want to at least open it for discussion here.

Album art

Cadence v5 allows the client to fetch album art, and the webclient displays it in the now-playing data. When I wrote the task, I envisioned a Cadence album art sort of command to fetch it, but @kenellorando noted it should just be in nowplaying, which come to think of it is what I envisioned when I took note to do this during the KenX talk.

So, yes, do that. And, while this does provide a good reason for the album art to be transmitted in higher resolution than the web client displays, we should also be kind to the server and cache it on our side.

Listener count

Cadence allows clients to ask for the listener count. I was musing whether CadenceBot should offer that info, despite CadenceBot being an egregious fault in how it works - CadenceBot is a single listener (for design reasons - CadenceBot connects once to the stream server and transmits from that connection to all of its own users, to save on bandwidth on both sides), no matter how many channels its playing in or how many people are listening in those channels. Therefore, I was musing about the possibility of coming up with a way to say how many listeners the Radio has "directly", plus how many servers the bot is playing in, in an attempt to tell users quietly that the second is not included in the first.

Eh.... I fail to see this as a critical feature. It'd be pretty easy to do, would be the only reason to do it. If we don't find a response format that we like, I won't push back on scrapping it.

Automatic stream url

Currently, the bot listens to a music stream which is at a URL provided by config. However, Cadence v5 provides an API to fetch the URL for the stream associated with the API, so we could reduce this down to just configuring the API server and pulling the stream URL on startup.

@kenellorando thinks this is critical-path for integration (which is a future project) - I don't quite agree, though it would be very nice to have (auto-integration would just involve putting in a few more config keys if we don't auto-grab this, since it's not hardcoded anywhere).

My thought was that we could leverage config to have this pulled automatically by default, but allow a user to override it if they want to for some reason - I fail to see how that'd be useful in prod, but as we containerize Cadence more and more and move away from single development instances towards images on your computer, I can conceive of wanting or needing to override this in development.

Auto-bitrate

CadenceBot currently allows the instance admin to specify what bitrate to transmit in - Either auto (do what the Discord server does) or a specific rate. Cadence v5.1+ provides an API to fetch the bitrate Cadence transmits in, so we could provide an option to transmit in the same rate.

Eh..... The major reason I'm not super in favor of this is that Cadence transmits OGG Vorbis (which might be why Safari/iOS are broken?), but CadenceBot transmits Opus, which is global for Discord. If we transmitted in the same codec, there would be a chance that we could coerce everything to simply retransmit the same stream, without reencoding, if we set the parameters the same - Which would reduce load, latency, and loss-of-quality - But since we have to retransmit no matter what in a different codec....

Support `history`

Being able to display the history of recently playing songs was one of the original hopes for Cadence, and in 5.1 @kenellorando has finally done it. We absolutely must implement support for it in CadenceBot.

This is both because it's nice and because of my philosophy that CadenceBot should support all reasonable features the web client does - If you can see history at cadenceradio.com, you should be able to see it via CadenceBot.

Support radiodata events

This is a little technical, but basically right now CadenceBot polls the Cadence server at an interval (by default, three seconds, though it's configurable) to fetch the metadata of the currently playing song. That means it can sometimes lag behind the actual music, and we waste some (though not a lot) of bandwidth and CPU fetching the same metadata over and over waiting to catch the transition.

radiodata/sse provides us with an alternative, where the server will instead notify us when there's an update to pull. This should be less laggy and slightly less wasteful, and certainly more modern.

Neither of us actually knows certainly how this will look, apparently, so this is a little unknown and may not make it into the 1.6.0 release this issue will be closed for.

It's also not that important, since from a user's perspective it'll be pretty equivalent...

Autoversioning

In short, the idea here is that instead of assuming Cadence supports everything we do, we can do a little intelligent stuff. I don't want to try and maintain multiple implementations and swap things out between major versions - I'm okay with an instance admin needing to upgrade CadenceBot to a new minor release to support a new major release of the Cadence server.

But, this is essentially a "feature flags" sort of idea - We know APIs like history or bitrate are 5.1+ only, so we could check the server version and remove them from our list if we run against a 5.0.x server. This is slowly becoming more desirable as Cadence gains more interest and its more likely for there to be multiple prod instances of both.

If we do this, basically upgrading a minor version of Cadence would at most require a bot restart, as long as the new minor version is one the bot is built to support.

If we don't, IAs will have to keep the bot version pegged to the server version at the feature level, or keep the bot slightly behind, to avoid breaking stuff from accessing nonexistent APIs

Wrapup

I'm out of time, so no wrap-up. Ask questions if you have them...

@kenellorando, I appear to have failed to respond here.

Replace current one-step-request with Cadence's bestmatch API (and trust the server's opinion of relevance over our own; As with search, we should improve the experience for all users, not just CadenceBot.

Yes to this, not so much that a user would find it any functionally different, just that it's might be the easiest, most conspicuous way to simplify your code.

Realistically, with how bestmatch and search currently work, bestmatch on v5 is a step down in terms of request quality from one-step-request on v4.

My take on that though is that since we're both providing the same functionality, we should implement it once, provide it the same way, and improve it on the server if we want to go that route. My only objection to that is the point I raised to @magix430 - bestmatch does and should prefer to "just pick a song", while it might still be valuable for the bot to say "I couldn't figure out which 'only my railgun' you meant, did you want the original or the 2020 edition?"

If we decide the bot should keep that feature/behavior, then we end up needing to keep the two separate anyway.

If possible, add a command to display the album art for the currently playing song (Cadence returns a base64'd image, if Discord allows us to display it then we should do so)

Not sure about a single command for artwork. If we imagine how CadenceBot is actually going to get used in the server, we should ask "are Discord audiences going to realistically be wanting to see the album art alone?", to which I think maybe not. What I think could be an ideal use of the album artwork is simply to integrate it with the nowplaying command, so it displays all information (title, artist, artwork all at once). In any case, implementation might be straightforward if we know what the best way to approach converting the encoded data to an image is. I wonder if we need to convert it into a file then do a Discord file upload?

This might as well have been picked directly from my brain during the KenX talk - Somehow I wrote "Add a command for" when I wanted the deliverable to be "Display album art". Good catch...

I'm not sure whether Discord will accept a b64 string as an attachment, or if we need to upload it to Discord that way then attach it, or convert to a file then attach it, or convert it, upload it, then attach it. That's all API work that I haven't looked at for lack of a use case... And it's complicated by what my wrapper will give me.

Basically, I'm confident it's possible and unsure how so.

Since album art can potentially get large, we need to be careful about the volume of data we pull from the Cadence deployment. Spamming the command in a discord channel might consume enough bandwidth from the radio to DDOS it. Maybe the bot should cache it once. Also maybe I should probably put the rate limiter on top of Cadence's artwork API itself.

Agreed, at least on the bot caching it. We'll update cached album art on our interval of refreshing the nowplaying presence, and disable the command if presence updates are disabled. By default, that timer is three seconds (though also here is using sse).

Rate limiter... Sure, I can see that. You're the server admin, so you know how much load from album art fetches you're willing to tolerate and if you need it limited. Perhaps that should be configurable as well to allow other admins to choose their own limits...

Consider adding a listener count

Another maybe, because it could be perceived as a flaw if a voice channel with ten people in it and the bot returns There is one listener. It's technically correct but may potentially be seen as an inaccurate feature.

Agreed - That's why I wanted to add the number of servers we play in. "There is one listener" would be perceived as inaccurate, while "There is one direct listener, and I am playing in three channels on Discord" might not be - And would at least be acknowledging the users in that channel.

And, that's something easy to do - We can fetch that second number simply as isPlaying.filter(a => a).length.

That said, this is very optional - I fail to see how it's useful, doubt it'll be all that commonly desired, and no one will ever miss it we decide against it. It's there because it's almost trivial to support it, so I wanted to pose the question of whether we should.

Replace the configured stream URL with fetching the listenurl from Cadence. Conceivably, we could provide a config mechanism to allow overriding this - default-config sets it to false, and if you change it to a string, we use that instead?

Yes, this one is 100% must do. It's a core change prerequisite for the integration idea of "each stack gets its own bot".

Eh, I wouldn't say so - We'd just have to add a second key into the config.json we need to generate when we create the per-stack bot.

It would be nice though.

Similarly, consider whether we should grab Cadence's stream bitrate and rebroadcast in the same quality by default (Requires Cadence v5.1+).

Maybe, but I don't think people would be able to hear the difference in Discord audio quality (defaults to 96kbps for non-paying users).

Probably not. I described more at length to @magix430 why I wish this was more useful.

Support the history command to get the play history (Requires Cadence v5.1+)

Definitely. High tier change.

Without question.

Consider replacing polling for metadata with support for the radiodata/sse event source - If this is not trivial (though I expect it to be fairly easy), this can be pushed out to expedite an official v1.6.0 release (making this 1.6.1)

No opinion: I also have no idea if this would be easy or not for a Discord bot, but I can at least say it certainly made the browser UI code simpler and more accurate. One important note: cadenceradio.com sometimes experienced SSE connections dropping after long enough periods of time, stopping automatic updates, so I also had to implement some code to automatically attempt reconnect. Why the connections drop, I haven't figured it out yet. It might just be a symptom of running it on a Kubernetes cluster. I can't tell yet. I guess overall, I wouldn't prioritize this one over others if existing code works fine. So we can try implementing this, just with care).

Yep, agreed. Though I'd like to note we've seen plenty of glitches with CadenceBot's nowplaying status silently failing to update over the years... It may end up being that we accidentally fix whatever's causing that while we do this, or we may get no change, or it may be worse.

We'll see, but if this works at least as well from a user perspective it would be better from a technical level.

Realistically, with how bestmatch and search currently work, bestmatch on v5 is a step down in terms of request quality from one-step-request on v4. My take on that though is that since we're both providing the same functionality, we should implement it once, provide it the same way, and improve it on the server if we want to go that route.

Put that way, I am inclined to agree now. I forgot CadenceBot was that interactive. Cadence provides all possible options (search, request, and both) now for any client to consume however they wish. We can consider bestmatch an option of convenience for a client developer that doesn't have anything yet and just wants something close enough (like a person that wants to write a quick script to queue a song on their own home web radio station).

za419 / CadenceBot