Some fixes and enhancements about cue files in satellite setup

[datasone]

This pr contains a series of commits which fixes issues around cue files in satellite setup, and some enhancements related to cue files. As ProxyDatabasePlugin uses libmpdclient for retrieving information from server, a pull request has also been created in libmpdclient repository.

Fixes

• SongPrint: include real_uri in song_print_info
• db/ProxyDatabasePlugin: read real_uri from server's song info
• (libmpdclient) Add real_uri into mpd_song

These commits add real_uri info into printed song info, and add the ability to read it in libmpdclient and ProxyDatabasePlugin. CUE virtual playlists is currently implemented by accessing real_uri file with range. But the real_uri is not included in the protocol now, thus ProxyDatabasePlugin is not able to retreive this information, and cannot access the corresponding file. This should fix #907.

• input/CurlInputPlugin: fix malformed value for HTTP Range header

CURLOPT_RANGE value requires the format "X-Y", where X or Y can be omitted. The current value provided to libcurl is only offset without the hyphen character and will cause HTTP error while accessing song files with range, the format string went wrong in 415de497, and this commit fixes it.

Enhancements

• db/ProxyDatabasePlugin: read millisecond values for range start and end
• (libmpdclient) Add range start and end in milliseconds into mpd_song

Range start and end values in mpd_song is stored in seconds only, which makes range times in ProxySong truncated to seconds. This leads to precision loss on cue tracks. These commits adds start_ms and end_ms, which represents range start and end in milliseconds, into libmpdclient parsing procedure.

• player/Thread: merge tag for songs have range instead of replacing with chunk->tag

On current code base (with patches above), while playing the first track of the cue virtual playlist on the satellite client, the currentsong status will lose cue metadata. It's due to tag being replaced by tag of MusicChunk in PlayerControl::PlayChunk, which is produced in DecoderBridge::SubmitAudio and DecoderBridge::SubmitTag.

In local cue files, song_tag is used to form the stream_tag in DecoderBridge::UpdateStreamTag, and then merged into chunk->tag. However, song_tag only works for local files. While playing cue tracks in satellite client, the tags are replaced by decoded tags in the whole album audio file.

This commit fixes this problem by merging song's tag with chunk->tag when the song's end_time is not zero. In the code base, end_time is only set while copying from other song objects, loading from database, parsing cue and the rangeid command. So this change will only affect behaviour when the user uses rangeid command to set range for a song in remote stream, and the user needs the tags from remote stream to replace tags in database instead of merging them. I think this is a much more rare scenario compared to the issue to be fixed.

[MaxKellermann]

I havn't yet read the actual code changes, but what I see from a first glance that all commit messages are bad, just like with your other PR https://github.com/MusicPlayerDaemon/MPD/pull/1779

[datasone]

The commit messages have been reworded for review.

[MaxKellermann]

Did you see that the CI build failed?

[datasone]

Sorry for didn't make that clear earlier, the CI build fails because of the requirement for newly added functions in libmpdclient PR isn't met.

The pr in MPD repo adds more info into the MPD protocol (the real_uri field and start/end time fields in milliseconds), and utilizes them to achieve working cue files and more precise cue tracks in satellite setup.

MPD uses libmpdclient for retrieving info from server in ProxyDatabase, and three functions: mpd_song_get_real_uri, mpd_song_get_start_ms, mpd_song_get_end_ms are added into libmpdclient (with the separate PR) for getting corresponding information.

With the log, I think the CI uses libmpdclient release directly. So the build will fail due to missing function definitions.

[MaxKellermann]

But the MPD build must not fail if you have an older libmpdclient version. You need to add compile-time version checks.

[datasone]

Thanks for the remind for code requirements, I will add necessary fixes soon.

[datasone]

Sorry for delaying till now about this pr, I haven't been able to get time on working on this.

I understand that libmpdclient version detection must be present for preventing build failures, but the LIBMPDCLIENT_CHECK_VERSION macro requires a version number for checking. And code in this pr requires some additional features in libmpdclient to work, which is introduced in a pr on that repository.

So is it necessary for me to get the libmpdclient pull request to be merged for this pr to continue being reviewed, or can I just assume that it can be merged, use the unreleased 2.21.0 libmpdclient version number to pass the MPD CI build, and get those two prs into the review process?

[jcorporation]

So is it necessary for me to get the https://github.com/MusicPlayerDaemon/libmpdclient/pull/100 to be merged for this pr to continue being reviewed, or can I just assume that it can be merged, use the unreleased 2.21.0 libmpdclient version number to pass the MPD CI build, and get those two prs into the review process?

You can use the unreleased 2.23.0 for now. If this pull request is merged, I will timely merge the patch for libmpdclient.