Add internet radio services and Radio Browser search#2048
Add internet radio services and Radio Browser search#2048jonaski merged 1 commit intostrawberrymusicplayer:masterfrom
Conversation
|
Crashed when adding a radio to the playlist here: |
|
Thanks for the thorough review, really appreciated! |
|
Some things I notice when testing here:
|
|
Hi @jonaski, thanks again for the thorough review! I've pushed two commits addressing all your feedback:
Would appreciate another review when you get a chance! |
|
Sorry for the delay in getting back on this but it's been quite busy. |
|
@jonaski Could you re-run the FreeBSD job for the latest CI run (24883340558)? It failed with an infrastructure issue, not a code issue — the FreeBSD VM couldn't reach the So no packages could be installed and the job aborted before any build step ran. All other platforms (Linux, macOS, Windows, OpenBSD) passed. I don't have admin rights to re-run it myself. Thanks! |
5c88c9b to
82a4bb0
Compare
|
Hi @jonaski, no rush at all and totally understand things are busy on your end! Just a small update: I rebased the branch onto Whenever you find a moment, is there anything else you'd like me to address, or were you able to finish the second round of testing? Also gentle reminder on the FreeBSD CI re-run when convenient — it failed only due to an infrastructure issue ( Thanks again for all your time on this! |
82a4bb0 to
a9f5e9b
Compare
|
I've squashed the commits and I fixed a couple of things, added radio browser to |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds multiple internet radio integrations (notably Radio Browser search) plus a new Radio settings page and radio UI improvements.
Changes:
- Introduces Radio Browser service (DNS discovery + search API) and a new tabbed “Radio Browser” search UI.
- Adds new radio settings for SomaFM quality and Radio Browser defaults.
- Updates radio/song plumbing (new
Song::Source, playlist handling, cover handling) and fixes double-click behavior on service nodes.
Reviewed changes
Copilot reviewed 33 out of 39 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utilities/coverutils.cpp | Treat Radio Browser sources like other stream sources for cover filename hashing. |
| src/settings/settingsdialog.h | Adds a new Settings dialog page enum for Radio. |
| src/settings/settingsdialog.cpp | Registers the new Radio settings page in the dialog. |
| src/settings/radiosettingspage.ui | New UI for SomaFM + Radio Browser settings. |
| src/settings/radiosettingspage.h | Declares RadioSettingsPage and country list helpers. |
| src/settings/radiosettingspage.cpp | Implements settings load/save and country list population. |
| src/radios/somafmservice.cpp | Reads SomaFM preferred stream quality from settings instead of hardcoding. |
| src/radios/radioviewcontainer.ui | Reworks the radio view into a tabbed UI, adding Radio Browser search tab. |
| src/radios/radioviewcontainer.h | Exposes search_view() accessor for the new tab widget. |
| src/radios/radioview.h | Adds double-click handler override to customize service-node behavior. |
| src/radios/radioview.cpp | Implements double-click expand/collapse for service nodes. |
| src/radios/radioservices.cpp | Registers RadioBrowserService and tweaks refresh state handling. |
| src/radios/radioservice.h | Adds JSON array extraction helpers for services returning arrays. |
| src/radios/radioservice.cpp | Implements ExtractJsonArray() parsing and validation. |
| src/radios/radiomodel.h | Adds sources() accessor to enumerate available sources. |
| src/radios/radiochannel.h | Extends RadioChannel with metadata fields + QDataStream operators. |
| src/radios/radiobrowserservice.h | New Radio Browser backend service interface and signals. |
| src/radios/radiobrowserservice.cpp | Implements DNS discovery, fallback server testing, search, and country fetch. |
| src/radios/radiobrowsersearchview.ui | New UI for search field, filters, results table, and pagination. |
| src/radios/radiobrowsersearchview.h | Declares the search view widget, slots, and playlist integration signal. |
| src/radios/radiobrowsersearchview.cpp | Implements search UI logic, settings defaults, and playlist actions. |
| src/radios/radiobrowsersearchmodel.h | New table model for displaying search results and drag support. |
| src/radios/radiobrowsersearchmodel.cpp | Implements result model, headers, and mime data creation. |
| src/playlist/playlistitem.cpp | Treats Radio Browser like other radio streams for playlist items. |
| src/covermanager/albumcoverchoicecontroller.cpp | Excludes Radio Browser from manual cover saving like other stream sources. |
| src/core/song.h | Adds Song::Source::RadioBrowser and updates kSourceCount. |
| src/core/song.cpp | Updates radio source checks, display strings, icons, domain, share URL, cache dir. |
| src/core/mainwindow.cpp | Wires Radio Browser search view to the RadioBrowserService and playlist adding. |
| src/constants/somafmsettings.h | New constants header for SomaFM settings keys/defaults. |
| src/constants/radioparadisesettings.h | New (currently minimal) constants header for Radio Paradise settings. |
| src/constants/radiobrowsersettings.h | New constants header for Radio Browser settings keys/defaults. |
| data/icons.qrc | Adds radiobrowser icon assets to the Qt resource bundle. |
| CMakeLists.txt | Adds new radio sources, headers, and UI forms to the build. |
Comments suppressed due to low confidence (4)
src/settings/radiosettingspage.cpp:1
- Radio Browser settings keys are partly centralized (
RadioBrowserSettings::*) butdefault_sort/default_countryare still repeated as string literals. To avoid drift between the settings page and the search view, add constants for these keys inradiobrowsersettings.hand use them consistently in both Load/Save and the search view initialization.
src/settings/radiosettingspage.cpp:1 - Radio Browser settings keys are partly centralized (
RadioBrowserSettings::*) butdefault_sort/default_countryare still repeated as string literals. To avoid drift between the settings page and the search view, add constants for these keys inradiobrowsersettings.hand use them consistently in both Load/Save and the search view initialization.
src/radios/radiobrowsersearchview.cpp:1 - Starting a new search clears the model but doesn’t cancel in-flight requests or guard against stale responses. If the user types quickly (debounced by
search_timer_) or changes filters while a previous request is still running, an olderSearchFinishedcan arrive later and append results for the wrong query into the cleared model. Consider aborting outstanding search replies when triggering a new search (ideally without aborting country loading), or tagging requests with a monotonically increasing search token and ignoring results that don’t match the current token.
/*
src/radios/radioservices.cpp:1
channels_refresh_is only reset tofalsewhenchannelsis non-empty. If a refresh legitimately yields an empty list (or a service returns none), the refresh flag may remain set indefinitely and can lead to incorrect refresh state/behavior. Setchannels_refresh_ = falsefor the successful completion path regardless of channel count (e.g., move it outside theif/else, or set it in both branches where the backend response is accepted).
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| RadioBrowser = 12 | ||
| }; | ||
| static const int kSourceCount = 16; | ||
| static const int kSourceCount = 13; |
| if (dns_lookup_->error() != QDnsLookup::NoError || dns_lookup_->hostAddressRecords().isEmpty()) { | ||
| // DNS failed, try fallback servers | ||
| dns_lookup_->deleteLater(); | ||
| dns_lookup_ = nullptr; | ||
|
|
||
| if (fallback_index_ < kFallbackServers.size()) { | ||
| TestServer(kFallbackServers.at(fallback_index_)); | ||
| ++fallback_index_; | ||
| } | ||
| else { | ||
| Q_EMIT SearchError(tr("Failed to discover Radio Browser server.")); | ||
| } | ||
| return; | ||
| } |
| ++fallback_index_; | ||
| if (fallback_index_ < kFallbackServers.size()) { | ||
| TestServer(kFallbackServers.at(fallback_index_)); |
| } | ||
|
|
||
| // Use first resolved hostname | ||
| const auto records = dns_lookup_->hostAddressRecords(); |
| QUrlQuery url_query; | ||
| if (!query.isEmpty()) url_query.addQueryItem(u"name"_s, query); | ||
| if (!country.isEmpty()) url_query.addQueryItem(u"countrycode"_s, country); | ||
| if (!tag.isEmpty()) url_query.addQueryItem(u"tag"_s, tag); | ||
| if (!language.isEmpty()) url_query.addQueryItem(u"language"_s, language); | ||
| url_query.addQueryItem(u"limit"_s, QString::number(limit)); | ||
| url_query.addQueryItem(u"offset"_s, QString::number(offset)); | ||
| url_query.addQueryItem(u"hidebroken"_s, u"true"_s); |
| void RadioBrowserService::SearchReply(QNetworkReply *reply, const int task_id, const int limit) { | ||
|
|
||
| if (replies_.contains(reply)) replies_.removeAll(reply); | ||
| reply->deleteLater(); | ||
|
|
||
| const QJsonArray array = ExtractJsonArray(reply); | ||
| task_manager_->SetTaskFinished(task_id); | ||
|
|
||
| if (array.isEmpty()) { | ||
| Q_EMIT SearchFinished(RadioChannelList(), false); | ||
| return; | ||
| } |
| if (json_error.error != QJsonParseError::NoError) { | ||
| Error(QStringLiteral("Failed to parse Json data from %1: %2").arg(name_, json_error.errorString())); | ||
| return QJsonArray(); | ||
| } |
| if (!json_document.isArray()) { | ||
| Error(QStringLiteral("%1: Json document is not an array.").arg(name_), json_document); | ||
| return QJsonArray(); | ||
| } |
a9f5e9b to
34c6498
Compare
Summary
Implementation details
RadioBrowserServicefollows the existingRadioServicepattern with DNS-based server discovery and fallback serversRadioBrowserSearchViewprovides search, filters, and pagination in a new "Radio Browser" tab alongside the existing "Channels" tabRadioServicebase class gainsExtractJsonArray()for services returning JSON arraysQSettingswith constants in dedicated headersKnown limitations
Test plan
🤖 Generated with Claude Code