khrynczenko / radiostream Goto Github PK

Some mild exceptions occur that automatically close program. This should be investigated and possibly resolved to inform the user of a problem without closing the program.

Nice way to inform user of something would be to pop up a message as in new window. Example would be when user changes language settings in the program and he should restart it in order to make effect.

I think that adding compiler fags in cmake would be a good thing.
Also it believe that turning warnings into errors is the only way. Otherwise neglect creeps in and in a moment there are hundreds of unresolved warnings.

For MSVC it would encompass:

• /W4
• /WX
• /sdl
• /analyze:ruleset

For GCC this would encompass:

• -Wall
• -Wextra
• -Wpedantic
• -Werror

It this is too much we could start with just first two for MSVC and GCC. Also CI for MSVC would be much needed in case someone works with GCC because we would need to check their changes against MSVC.

Instead of having (almost always) one class per file I think it would be better to treat files as a unit containing all related classes and functions.

Volume normalization should be implemented. Most music services have that so should we.

When sorting by the Bitrate column, the values are interpreted as strings and instead of taking into account just numerical part.

Since window size can be manipulated without any restrictions I think is when user closes it size should be remembered and use automatically when program starts next time.

Stations search limit option in tools menu does affect change config,json file but requests still return only 50 stations. The number in config.json does not matter.

Additional CI for windows wouldn't hurt. AppVeyor seems to be the tool for the job.

Could be done with one liner if clang-format is installed.
find src/ include/ -iname *.hpp -o -iname *.cpp | xargs clang-format -i -style=file

radio-browser.info seems to be only one open and free database of radio stations urls. This makes it unnecessary to leave room for other possibilities.

When building project binaries of dependencies are being put into different directories from RadioStream output directory. In effect one needs to copy .so/.dll files by herself to the RadioStream output directory. Also BASS prebuilt binary and lang directory should be copied at the end of build phase.

Now it is possible to unselect a row when right clicking on it second time. If that happens and user tries to delete this selected but in fact unselected row the exceptions throws and program shuts down.

There is currently a problem when we try to set a new stream and play it directly after.
Taken from StationPlayerController.cpp

case radiostream::Event::NewStationRequested:
    {
        const auto station = std::any_cast<Station>(data);
        context_.status_.change_text(context_.localizer_.get_localized_text("Loading stream..."));
        context_.status_.change_color(StatusBar::Color::PROCESSING);
        context_.station_player_.set_station(station);
        context_.station_player_.play();
        context_.status_.change_text(context_.localizer_.get_localized_text("Stream playing"));
        context_.status_.change_color(StatusBar::Color::FINISHED);
    }
break;

Both StationPlayer::set_station() and StationPlayer::play() are implemented this way:

void StationPlayer::play()
{
    auto thread = std::thread([this]()
    {
        stream_manager_.play();
    });
    thread.detach();
}

and

void StationPlayer::set_station(const Station& station)
{
    station_ = station;
    auto thread = std::thread([this]()
    {
        stream_manager_.set_stream(station_.ip_);
        check_if_song_title_has_changed();
        notify(std::make_any<Station>(station_), radiostream::Event::StationBeingPlayedChanged);
    });
    thread.detach();
}

So it might happen that the thread that executes the play method might get created before the thread in set_station. In case of event mentioned above it makes it possible that play is executed before the set_station and in effect station will not start playing. This is unintended and should be fixed.

BASS library supports MP3/MP2/MP1/OGG/WAV/AIFF out of the box. AAC+ is one of the most popular coding formats use by Internet radio stations. Possible solution is using one of official addons from the BASS webpage.

Currently copying to clipboard is working only on Windows. So far I saw that to get to use clipboard on linux requires quite a lot of work.
Possible solution might be using some cross-platform library like clip.

I think a popup message mechanism would be nice for example to inform the user that in order to language to be changed he needs to restart the application.

I don't find it to be necessary to use nlohmann parsing if POCO includes json library for that. The positive of this change would be removing one dependency.

Currently the values for that column are either true or false. This does not look good. Nice solution could be icon like a star.

When writing in search textbox to find stations, the search process should start automatically on pressing Enter without having to click button.

When opening context menu with right-click on empty part of the stations list in the main window and then choosing delete station from list program hard crashes.

From main window -> right click in the list on empty part of it -> click delete station from list -> crash

Currently downloading and building pre-build tools/libs take most of the time. Some caching could help.

Program data should be stored separately form the executable file i.e. not in the bin folder. It should be probably stored in the $HOME directory in some folder.

Storing it in the bin folder also requires running program with root privileges which is not desired.

If settings file is missing for some reason, default file should be created so the program can still run.

Debian packages are nice way for Linux Debian distributions. Some autonomous mechanism for creating it should be implemented probably.

• CPack CMake may be able to generate them

Installation Systems that could be used:

• NSIS https://sourceforge.net/projects/nsis/
• INNO Setup http://jrsoftware.org/isinfo.php
• WiX http://wixtoolset.org/
• CPack CMake

Adding station (through File -> Add station) without set URL should be not acceptable.

If database file is missing for some reason, default one should be created so the program can still run.

https://api.radio-browser.info/

Now it is not only different endpoint, but it seems that it also redirects to HTTPS, even when HTTP one is requested.
This would require additional changes since using https is more convoluted using POCO. Maybe it would be wise to switch to some simple to use library. OpenSSL will probably be needed as another dependency.

Tools options that can be modified by an user should be initialized with values stored in config files.

Going File -> Open URL -> (Leave URL field empty) -> Ok results in program crash.
Expected behavior would be to communicate user that field wasn't set and do nothing.

File of interest:
Application.cpp -> void init_menubar()

Travis became very unreliable as a CI solution. Even with an uploaded Docker image that has all the dependencies downloaded and installed, we hit the Travis minute limit. Github Actions is build-in and easy to use and free without limits for open-source projects hence I believe it is an easy choice.

On Linux resizing columns within list boxes result in lags and stuttering.

Currently when user tries to play some station and it does not start playing due to some problems, nothing actually happens and the status bar will display that "Station is playing" which is not true.

Excepted behavior would be to properly inform the user changing the info in the status bar.

Many radio stations URLs on the web direct to the .pls and .m3u files which are playlist files instead of direct URLs to streams.

More in it on:
https://en.wikipedia.org/wiki/PLS_(file_format)
https://en.wikipedia.org/wiki/M3U

Currently only direct URLs are working.

Currently every reared is really primitive in its reading process, this might result in them having troubles with non arbitrary formatted files.

Classes under consideration:
MultimediaPlaylistReader
M3UReader
PLSReader

Please allow editing a station.
For example, I want to edit the station URL or station language.

Idea: Make possible to use different stations list providers and enable to change them at run-time.

By requester I mean object that that results in list of available stations. Result might come from request to some web-api or local database etc.
For now there is only one HTTP requester namely one that uses www.radio-browser.info web-api. Because of that there is no interface for general Requester which in result ties the program to use only this one particular wep-api.

Solution: Make Requesters into strategies. Factory for retrieving them.

When user adds to his database new station from the stations listed by radio-browser after some time its URL may get out-dated i.e. the station may change to stream under different URL. radio-browser keeps track of such changes which is why such feature for automatic update could be implemented.
The problems:

• only stations that are listed on www.radio-browser.info could make use of such feature
• is the name of station enough to resolve to correct station? what if station name has changed in www.radio-browser.info database
• www.radio-browser.info is kept updated by people, if no one updates the station with new working URL it will stay broken

Please publish a version for 32-bit PCs.

Sometimes scrollbar in the list where the user stations are shown will not appear. Possible quickfix is to make it always visible even when not necessary.

The stations that are added to listbox by an user should be treated as his favorite by default. Favorite column can be removed.