From 7c25d83f1cc4c7db2d2d3f4506525dd056b885e8 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2014 12:14:27 +0200 Subject: Tag: use SignedSongTime for the song duration --- src/DetachedSong.cxx | 13 +++++++++---- src/DetachedSong.hxx | 2 +- src/PlayerThread.cxx | 4 ++-- src/SongPrint.cxx | 6 +++--- src/SongSave.cxx | 2 +- src/TagPrint.cxx | 4 ++-- src/TagSave.cxx | 4 ++-- src/db/Count.cxx | 9 ++++++--- src/db/Helpers.cxx | 4 ++-- src/db/LightSong.cxx | 14 ++++++++------ src/db/LightSong.hxx | 2 +- src/db/plugins/ProxyDatabasePlugin.cxx | 5 ++++- src/db/plugins/upnp/Directory.cxx | 12 ++++++------ src/lib/despotify/DespotifyUtils.cxx | 2 +- src/output/plugins/RoarOutputPlugin.cxx | 16 ++++++++++------ src/playlist/plugins/ExtM3uPlaylistPlugin.cxx | 2 +- src/playlist/plugins/PlsPlaylistPlugin.cxx | 2 +- src/playlist/plugins/SoundCloudPlaylistPlugin.cxx | 2 +- src/queue/PlaylistEdit.cxx | 9 +++++---- src/tag/Tag.cxx | 4 ++-- src/tag/Tag.hxx | 21 ++++++++++----------- src/tag/TagBuilder.cxx | 18 +++++++++--------- src/tag/TagBuilder.hxx | 21 ++++++++++----------- src/tag/TagHandler.cxx | 2 +- test/test_translate_song.cxx | 9 ++++++--- 25 files changed, 104 insertions(+), 85 deletions(-) diff --git a/src/DetachedSong.cxx b/src/DetachedSong.cxx index fd81545c3..906e29bba 100644 --- a/src/DetachedSong.cxx +++ b/src/DetachedSong.cxx @@ -58,11 +58,16 @@ DetachedSong::IsInDatabase() const return !uri_has_scheme(_uri) && !PathTraitsUTF8::IsAbsolute(_uri); } -double +SignedSongTime DetachedSong::GetDuration() const { - if (end_time.IsPositive()) - return (end_time - start_time).ToDoubleS(); + SongTime a = start_time, b = end_time; + if (!b.IsPositive()) { + if (tag.duration.IsNegative()) + return tag.duration; - return tag.time - start_time.ToDoubleS(); + b = SongTime(tag.duration); + } + + return SignedSongTime(b - a); } diff --git a/src/DetachedSong.hxx b/src/DetachedSong.hxx index 135e9c3cc..021b5de29 100644 --- a/src/DetachedSong.hxx +++ b/src/DetachedSong.hxx @@ -213,7 +213,7 @@ public: } gcc_pure - double GetDuration() const; + SignedSongTime GetDuration() const; /** * Update the #tag and #mtime. diff --git a/src/PlayerThread.cxx b/src/PlayerThread.cxx index 37edb0a20..c4db25e98 100644 --- a/src/PlayerThread.cxx +++ b/src/PlayerThread.cxx @@ -349,7 +349,7 @@ Player::WaitForDecoder() decoder_starting = true; /* update PlayerControl's song information */ - pc.total_time = pc.next_song->GetDuration(); + pc.total_time = pc.next_song->GetDuration().ToDoubleS(); pc.bit_rate = 0; pc.audio_format.Clear(); @@ -374,7 +374,7 @@ real_song_duration(const DetachedSong &song, double decoder_duration) if (decoder_duration <= 0.0) /* the decoder plugin didn't provide information; fall back to Song::GetDuration() */ - return song.GetDuration(); + return song.GetDuration().ToDoubleS(); const SongTime start_time = song.GetStartTime(); const SongTime end_time = song.GetEndTime(); diff --git a/src/SongPrint.cxx b/src/SongPrint.cxx index e5ebfaab2..0c1b93265 100644 --- a/src/SongPrint.cxx +++ b/src/SongPrint.cxx @@ -119,7 +119,7 @@ song_print_info(Client &client, const DetachedSong &song, bool base) tag_print_values(client, song.GetTag()); - double duration = song.GetDuration(); - if (duration >= 0) - client_printf(client, "Time: %u\n", unsigned(duration + 0.5)); + const auto duration = song.GetDuration(); + if (!duration.IsNegative()) + client_printf(client, "Time: %u\n", duration.RoundS()); } diff --git a/src/SongSave.cxx b/src/SongSave.cxx index 8b532bedd..895e9805b 100644 --- a/src/SongSave.cxx +++ b/src/SongSave.cxx @@ -100,7 +100,7 @@ song_load(TextFile &file, const char *uri, if ((type = tag_name_parse(line)) != TAG_NUM_OF_ITEM_TYPES) { tag.AddItem(type, value); } else if (strcmp(line, "Time") == 0) { - tag.SetTime(atoi(value)); + tag.SetDuration(SignedSongTime::FromS(atof(value))); } else if (strcmp(line, "Playlist") == 0) { tag.SetHasPlaylist(strcmp(value, "yes") == 0); } else if (strcmp(line, SONG_MTIME) == 0) { diff --git a/src/TagPrint.cxx b/src/TagPrint.cxx index 636eedf4e..4937fa622 100644 --- a/src/TagPrint.cxx +++ b/src/TagPrint.cxx @@ -52,8 +52,8 @@ tag_print_values(Client &client, const Tag &tag) void tag_print(Client &client, const Tag &tag) { - if (tag.time >= 0) - client_printf(client, SONG_TIME "%i\n", tag.time); + if (!tag.duration.IsNegative()) + client_printf(client, SONG_TIME "%i\n", tag.duration.RoundS()); tag_print_values(client, tag); } diff --git a/src/TagSave.cxx b/src/TagSave.cxx index c47d778d3..107aca7db 100644 --- a/src/TagSave.cxx +++ b/src/TagSave.cxx @@ -27,8 +27,8 @@ void tag_save(BufferedOutputStream &os, const Tag &tag) { - if (tag.time >= 0) - os.Format(SONG_TIME "%i\n", tag.time); + if (!tag.duration.IsNegative()) + os.Format(SONG_TIME "%f\n", tag.duration.ToDoubleS()); if (tag.has_playlist) os.Format("Playlist: yes\n"); diff --git a/src/db/Count.cxx b/src/db/Count.cxx index 4fd53a73b..fecd901e5 100644 --- a/src/db/Count.cxx +++ b/src/db/Count.cxx @@ -64,7 +64,10 @@ static bool stats_visitor_song(SearchStats &stats, const LightSong &song) { stats.n_songs++; - stats.total_time_s += song.GetDuration(); + + const auto duration = song.GetDuration(); + if (!duration.IsNegative()) + stats.total_time_s += duration.ToS(); return true; } @@ -79,8 +82,8 @@ CollectGroupCounts(TagCountMap &map, TagType group, const Tag &tag) SearchStats())); SearchStats &s = r.first->second; ++s.n_songs; - if (tag.time > 0) - s.total_time_s += tag.time; + if (!tag.duration.IsNegative()) + s.total_time_s += tag.duration.ToS(); found = true; } diff --git a/src/db/Helpers.cxx b/src/db/Helpers.cxx index 089b2b17d..8df6c265c 100644 --- a/src/db/Helpers.cxx +++ b/src/db/Helpers.cxx @@ -40,8 +40,8 @@ static void StatsVisitTag(DatabaseStats &stats, StringSet &artists, StringSet &albums, const Tag &tag) { - if (tag.time > 0) - stats.total_duration += tag.time; + if (!tag.duration.IsNegative()) + stats.total_duration += tag.duration.ToS(); for (const auto &item : tag) { switch (item.type) { diff --git a/src/db/LightSong.cxx b/src/db/LightSong.cxx index 3e0f361d8..5cdebc133 100644 --- a/src/db/LightSong.cxx +++ b/src/db/LightSong.cxx @@ -20,14 +20,16 @@ #include "LightSong.hxx" #include "tag/Tag.hxx" -double +SignedSongTime LightSong::GetDuration() const { - if (end_time.IsPositive()) - return (end_time - start_time).ToDoubleS(); + SongTime a = start_time, b = end_time; + if (!b.IsPositive()) { + if (tag->duration.IsNegative()) + return tag->duration; - if (tag->time <= 0) - return 0; + b = SongTime(tag->duration); + } - return tag->time - start_time.ToDoubleS(); + return SignedSongTime(b - a); } diff --git a/src/db/LightSong.hxx b/src/db/LightSong.hxx index 26f68d18c..bbd449fbe 100644 --- a/src/db/LightSong.hxx +++ b/src/db/LightSong.hxx @@ -87,7 +87,7 @@ struct LightSong { } gcc_pure - double GetDuration() const; + SignedSongTime GetDuration() const; }; #endif diff --git a/src/db/plugins/ProxyDatabasePlugin.cxx b/src/db/plugins/ProxyDatabasePlugin.cxx index c6b57d248..97b544203 100644 --- a/src/db/plugins/ProxyDatabasePlugin.cxx +++ b/src/db/plugins/ProxyDatabasePlugin.cxx @@ -199,7 +199,10 @@ ProxySong::ProxySong(const mpd_song *song) #endif TagBuilder tag_builder; - tag_builder.SetTime(mpd_song_get_duration(song)); + + const unsigned duration = mpd_song_get_duration(song); + if (duration > 0) + tag_builder.SetDuration(SignedSongTime::FromS(duration)); for (const auto *i = &tag_table[0]; i->d != TAG_NUM_OF_ITEM_TYPES; ++i) Copy(tag_builder, i->d, song, i->s); diff --git a/src/db/plugins/upnp/Directory.cxx b/src/db/plugins/upnp/Directory.cxx index e43cd48a6..683022a10 100644 --- a/src/db/plugins/upnp/Directory.cxx +++ b/src/db/plugins/upnp/Directory.cxx @@ -59,28 +59,28 @@ ParseItemClass(const char *name, size_t length) } gcc_pure -static int +static SignedSongTime ParseDuration(const char *duration) { char *endptr; unsigned result = ParseUnsigned(duration, &endptr); if (endptr == duration || *endptr != ':') - return 0; + return SignedSongTime::Negative(); result *= 60; duration = endptr + 1; result += ParseUnsigned(duration, &endptr); if (endptr == duration || *endptr != ':') - return 0; + return SignedSongTime::Negative(); result *= 60; duration = endptr + 1; result += ParseUnsigned(duration, &endptr); if (endptr == duration || *endptr != 0) - return 0; + return SignedSongTime::Negative(); - return result; + return SignedSongTime::FromS(result); } /** @@ -183,7 +183,7 @@ protected: const char *duration = GetAttribute(attrs, "duration"); if (duration != nullptr) - tag.SetTime(ParseDuration(duration)); + tag.SetDuration(ParseDuration(duration)); state = RES; } diff --git a/src/lib/despotify/DespotifyUtils.cxx b/src/lib/despotify/DespotifyUtils.cxx index 47a83e49b..f67679c50 100644 --- a/src/lib/despotify/DespotifyUtils.cxx +++ b/src/lib/despotify/DespotifyUtils.cxx @@ -105,7 +105,7 @@ mpd_despotify_tag_from_track(const ds_track &track) tag.AddItem(TAG_ALBUM, track.album); tag.AddItem(TAG_DATE, date); tag.AddItem(TAG_COMMENT, comment); - tag.SetTime(track.length / 1000); + tag.SetDuration(SignedSongTime::FromMS(track.length)); return tag.Commit(); } diff --git a/src/output/plugins/RoarOutputPlugin.cxx b/src/output/plugins/RoarOutputPlugin.cxx index ae6bdf1b1..04836cc46 100644 --- a/src/output/plugins/RoarOutputPlugin.cxx +++ b/src/output/plugins/RoarOutputPlugin.cxx @@ -363,16 +363,20 @@ RoarOutput::SendTag(const Tag &tag) const ScopeLock protect(mutex); - size_t cnt = 1; + size_t cnt = 0; struct roar_keyval vals[32]; char uuid_buf[32][64]; char timebuf[16]; - snprintf(timebuf, sizeof(timebuf), "%02d:%02d:%02d", - tag.time / 3600, (tag.time % 3600) / 60, tag.time % 60); - - vals[0].key = const_cast("LENGTH"); - vals[0].value = timebuf; + if (!tag.duration.IsNegative()) { + const unsigned seconds = tag.duration.ToS(); + snprintf(timebuf, sizeof(timebuf), "%02d:%02d:%02d", + seconds / 3600, (seconds % 3600) / 60, seconds % 60); + + vals[cnt].key = const_cast("LENGTH"); + vals[cnt].value = timebuf; + ++cnt; + } for (const auto &item : tag) { if (cnt >= 32) diff --git a/src/playlist/plugins/ExtM3uPlaylistPlugin.cxx b/src/playlist/plugins/ExtM3uPlaylistPlugin.cxx index 15e8125e3..b459696f1 100644 --- a/src/playlist/plugins/ExtM3uPlaylistPlugin.cxx +++ b/src/playlist/plugins/ExtM3uPlaylistPlugin.cxx @@ -89,7 +89,7 @@ extm3u_parse_tag(const char *line) return Tag(); TagBuilder tag; - tag.SetTime(duration); + tag.SetDuration(SignedSongTime::FromS(unsigned(duration))); /* unfortunately, there is no real specification for the EXTM3U format, so we must assume that the string after the diff --git a/src/playlist/plugins/PlsPlaylistPlugin.cxx b/src/playlist/plugins/PlsPlaylistPlugin.cxx index 7df7d134b..f7724f522 100644 --- a/src/playlist/plugins/PlsPlaylistPlugin.cxx +++ b/src/playlist/plugins/PlsPlaylistPlugin.cxx @@ -85,7 +85,7 @@ pls_parser(GKeyFile *keyfile, std::forward_list &songs) int length = g_key_file_get_integer(keyfile, "playlist", key, nullptr); if (length > 0) - tag.SetTime(length); + tag.SetDuration(SignedSongTime::FromS(length)); songs.emplace_front(uri, tag.Commit()); g_free(uri); diff --git a/src/playlist/plugins/SoundCloudPlaylistPlugin.cxx b/src/playlist/plugins/SoundCloudPlaylistPlugin.cxx index d9f62300b..ec4d240a5 100644 --- a/src/playlist/plugins/SoundCloudPlaylistPlugin.cxx +++ b/src/playlist/plugins/SoundCloudPlaylistPlugin.cxx @@ -215,7 +215,7 @@ handle_end_map(void *ctx) soundcloud_config.apikey.c_str(), nullptr); TagBuilder tag; - tag.SetTime(data->duration / 1000); + tag.SetDuration(SignedSongTime::FromMS(data->duration)); if (data->title != nullptr) tag.AddItem(TAG_NAME, data->title); diff --git a/src/queue/PlaylistEdit.cxx b/src/queue/PlaylistEdit.cxx index 20d459732..22a88dc46 100644 --- a/src/queue/PlaylistEdit.cxx +++ b/src/queue/PlaylistEdit.cxx @@ -463,18 +463,19 @@ playlist::SetSongIdRange(PlayerControl &pc, unsigned id, } DetachedSong &song = queue.Get(position); - if (song.GetTag().time > 0) { + + const auto duration = song.GetTag().duration; + if (!duration.IsNegative()) { /* validate the offsets */ - const unsigned duration = song.GetTag().time; - if (start.ToMS() / 1000u > duration) { + if (start > duration) { error.Set(playlist_domain, int(PlaylistResult::BAD_RANGE), "Invalid start offset"); return false; } - if (end.ToMS() / 1000u > duration) + if (end >= duration) end = SongTime::zero(); } diff --git a/src/tag/Tag.cxx b/src/tag/Tag.cxx index 1b338ae8d..92ac4214c 100644 --- a/src/tag/Tag.cxx +++ b/src/tag/Tag.cxx @@ -61,7 +61,7 @@ tag_name_parse_i(const char *name) void Tag::Clear() { - time = -1; + duration = SignedSongTime::Negative(); has_playlist = false; tag_pool_lock.lock(); @@ -75,7 +75,7 @@ Tag::Clear() } Tag::Tag(const Tag &other) - :time(other.time), has_playlist(other.has_playlist), + :duration(other.duration), has_playlist(other.has_playlist), num_items(other.num_items), items(nullptr) { diff --git a/src/tag/Tag.hxx b/src/tag/Tag.hxx index 74221417f..f1d3d5767 100644 --- a/src/tag/Tag.hxx +++ b/src/tag/Tag.hxx @@ -22,6 +22,7 @@ #include "TagType.h" // IWYU pragma: export #include "TagItem.hxx" // IWYU pragma: export +#include "Chrono.hxx" #include "Compiler.h" #include @@ -35,12 +36,10 @@ */ struct Tag { /** - * The duration of the song (in seconds). A value of zero - * means that the length is unknown. If the duration is - * really between zero and one second, you should round up to - * 1. + * The duration of the song. A negative value means that the + * length is unknown. */ - int time; + SignedSongTime duration; /** * Does this file have an embedded playlist (e.g. embedded CUE @@ -57,13 +56,13 @@ struct Tag { /** * Create an empty tag. */ - Tag():time(-1), has_playlist(false), + Tag():duration(SignedSongTime::Negative()), has_playlist(false), num_items(0), items(nullptr) {} Tag(const Tag &other); Tag(Tag &&other) - :time(other.time), has_playlist(other.has_playlist), + :duration(other.duration), has_playlist(other.has_playlist), num_items(other.num_items), items(other.items) { other.items = nullptr; other.num_items = 0; @@ -79,7 +78,7 @@ struct Tag { Tag &operator=(const Tag &other) = delete; Tag &operator=(Tag &&other) { - time = other.time; + duration = other.duration; has_playlist = other.has_playlist; std::swap(items, other.items); std::swap(num_items, other.num_items); @@ -87,8 +86,8 @@ struct Tag { } /** - * Returns true if the tag contains no items. This ignores the "time" - * attribute. + * Returns true if the tag contains no items. This ignores + * the "duration" attribute. */ bool IsEmpty() const { return num_items == 0; @@ -98,7 +97,7 @@ struct Tag { * Returns true if the tag contains any information. */ bool IsDefined() const { - return !IsEmpty() || time >= 0; + return !IsEmpty() || !duration.IsNegative(); } /** diff --git a/src/tag/TagBuilder.cxx b/src/tag/TagBuilder.cxx index bc8ea2ae7..85060e9ee 100644 --- a/src/tag/TagBuilder.cxx +++ b/src/tag/TagBuilder.cxx @@ -29,7 +29,7 @@ #include TagBuilder::TagBuilder(const Tag &other) - :time(other.time), has_playlist(other.has_playlist) + :duration(other.duration), has_playlist(other.has_playlist) { items.reserve(other.num_items); @@ -40,7 +40,7 @@ TagBuilder::TagBuilder(const Tag &other) } TagBuilder::TagBuilder(Tag &&other) - :time(other.time), has_playlist(other.has_playlist) + :duration(other.duration), has_playlist(other.has_playlist) { /* move all TagItem pointers from the Tag object; we don't need to contact the tag pool, because all we do is move @@ -58,7 +58,7 @@ TagBuilder & TagBuilder::operator=(const TagBuilder &other) { /* copy all attributes */ - time = other.time; + duration = other.duration; has_playlist = other.has_playlist; items = other.items; @@ -74,7 +74,7 @@ TagBuilder::operator=(const TagBuilder &other) TagBuilder & TagBuilder::operator=(TagBuilder &&other) { - time = other.time; + duration = other.duration; has_playlist = other.has_playlist; items = std::move(other.items); @@ -84,7 +84,7 @@ TagBuilder::operator=(TagBuilder &&other) TagBuilder & TagBuilder::operator=(Tag &&other) { - time = other.time; + duration = other.duration; has_playlist = other.has_playlist; /* move all TagItem pointers from the Tag object; we don't @@ -105,7 +105,7 @@ TagBuilder::operator=(Tag &&other) void TagBuilder::Clear() { - time = -1; + duration = SignedSongTime::Negative(); has_playlist = false; RemoveAll(); } @@ -115,7 +115,7 @@ TagBuilder::Commit(Tag &tag) { tag.Clear(); - tag.time = time; + tag.duration = duration; tag.has_playlist = has_playlist; /* move all TagItem pointers to the new Tag object without @@ -162,8 +162,8 @@ TagBuilder::HasType(TagType type) const void TagBuilder::Complement(const Tag &other) { - if (time <= 0) - time = other.time; + if (duration.IsNegative()) + duration = other.duration; has_playlist |= other.has_playlist; diff --git a/src/tag/TagBuilder.hxx b/src/tag/TagBuilder.hxx index 6de52b775..aff581313 100644 --- a/src/tag/TagBuilder.hxx +++ b/src/tag/TagBuilder.hxx @@ -21,6 +21,7 @@ #define MPD_TAG_BUILDER_HXX #include "TagType.h" +#include "Chrono.hxx" #include "Compiler.h" #include @@ -35,12 +36,10 @@ struct Tag; */ class TagBuilder { /** - * The duration of the song (in seconds). A value of zero - * means that the length is unknown. If the duration is - * really between zero and one second, you should round up to - * 1. + * The duration of the song. A negative value means that the + * length is unknown. */ - int time; + SignedSongTime duration; /** * Does this file have an embedded playlist (e.g. embedded CUE @@ -56,7 +55,7 @@ public: * Create an empty tag. */ TagBuilder() - :time(-1), has_playlist(false) {} + :duration(SignedSongTime::Negative()), has_playlist(false) {} ~TagBuilder() { Clear(); @@ -73,8 +72,8 @@ public: TagBuilder &operator=(Tag &&other); /** - * Returns true if the tag contains no items. This ignores the "time" - * attribute. + * Returns true if the tag contains no items. This ignores + * the "duration" attribute. */ bool IsEmpty() const { return items.empty(); @@ -85,7 +84,7 @@ public: */ gcc_pure bool IsDefined() const { - return time >= 0 || has_playlist || !IsEmpty(); + return !duration.IsNegative() || has_playlist || !IsEmpty(); } void Clear(); @@ -109,8 +108,8 @@ public: */ Tag *CommitNew(); - void SetTime(int _time) { - time = _time; + void SetDuration(SignedSongTime _duration) { + duration = _duration; } void SetHasPlaylist(bool _has_playlist) { diff --git a/src/tag/TagHandler.cxx b/src/tag/TagHandler.cxx index 014834b88..20a51f064 100644 --- a/src/tag/TagHandler.cxx +++ b/src/tag/TagHandler.cxx @@ -27,7 +27,7 @@ add_tag_duration(unsigned seconds, void *ctx) { TagBuilder &tag = *(TagBuilder *)ctx; - tag.SetTime(seconds); + tag.SetDuration(SignedSongTime::FromS(seconds)); } static void diff --git a/test/test_translate_song.cxx b/test/test_translate_song.cxx index 324a79d29..e3c1bcb79 100644 --- a/test/test_translate_song.cxx +++ b/test/test_translate_song.cxx @@ -155,10 +155,13 @@ Client::AllowFile(gcc_unused Path path_fs, gcc_unused Error &error) const static std::string ToString(const Tag &tag) { - char buffer[64]; - sprintf(buffer, "%d", tag.time); + std::string result; - std::string result = buffer; + if (!tag.duration.IsNegative()) { + char buffer[64]; + sprintf(buffer, "%d", tag.duration.ToMS()); + result.append(buffer); + } for (const auto &item : tag) { result.push_back('|'); -- cgit v1.2.3