Tag: use SignedSongTime for the song duration

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);
 }

src/DetachedSong.hxx

@@ -213,7 +213,7 @@ public:
 	}
 
 	gcc_pure
-	double GetDuration() const;
+	SignedSongTime GetDuration() const;
 
 	/**
 	 * Update the #tag and #mtime.

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();

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());
 }

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) {

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);
 }

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");

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;
 		}

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) {

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);
 }

src/db/LightSong.hxx

@@ -87,7 +87,7 @@ struct LightSong {
 	}
 
 	gcc_pure
-	double GetDuration() const;
+	SignedSongTime GetDuration() const;
 };
 
 #endif

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);

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;
 			}

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();
 }

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);
+	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[0].key = const_cast<char *>("LENGTH");
-	vals[0].value = timebuf;
+		vals[cnt].key = const_cast<char *>("LENGTH");
+		vals[cnt].value = timebuf;
+		++cnt;
+	}
 
 	for (const auto &item : tag) {
 		if (cnt >= 32)

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

src/playlist/plugins/PlsPlaylistPlugin.cxx

@@ -85,7 +85,7 @@ pls_parser(GKeyFile *keyfile, std::forward_list<DetachedSong> &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);

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);
 

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();
 	}
 

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)
 {

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 <algorithm>
@@ -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();
 	}
 
 	/**

src/tag/TagBuilder.cxx

@@ -29,7 +29,7 @@
 #include <stdlib.h>
 
 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;
 

src/tag/TagBuilder.hxx

@@ -21,6 +21,7 @@
 #define MPD_TAG_BUILDER_HXX
 
 #include "TagType.h"
+#include "Chrono.hxx"
 #include "Compiler.h"
 
 #include <vector>
@@ -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) {

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

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('|');