From 98d28ede9f39af11063338e2029289687eb5215a Mon Sep 17 00:00:00 2001 From: Martchus Date: Mon, 20 Jun 2022 21:27:42 +0200 Subject: [PATCH] Add scale info to Popularity for furture extension This would allow implementing a way to convert between different scales and which in turn would allow the UI to provide an editor with a generic scale (e.g. stars) instead of only allowing to edit raw values as string. This also make it assume that a single number is meant to be the rating (instead of the user). That should make editing the rating a bit more straight forward (if one doesn't care about the user and play counter). --- id3/id3v2frame.cpp | 3 ++- tagvalue.cpp | 23 ++++++++++++++++++----- tagvalue.h | 23 ++++++++++++++++++----- tests/tagvalue.cpp | 7 +++++-- vorbis/vorbiscommentfield.cpp | 15 +++++++++++++-- 5 files changed, 56 insertions(+), 15 deletions(-) diff --git a/id3/id3v2frame.cpp b/id3/id3v2frame.cpp index 614ebaa..d3c407e 100644 --- a/id3/id3v2frame.cpp +++ b/id3/id3v2frame.cpp @@ -4,6 +4,7 @@ #include "../diagnostics.h" #include "../exceptions.h" +#include "../tagtype.h" #include #include @@ -386,7 +387,7 @@ void Id3v2Frame::parse(BinaryReader &reader, std::uint32_t version, std::uint32_ } else if (((version >= 3 && id() == Id3v2FrameIds::lRating) || (version < 3 && id() == Id3v2FrameIds::sRating))) { // parse popularimeter frame - auto popularity = Popularity(); + auto popularity = Popularity{ .scale = TagType::Id3v2Tag }; auto userEncoding = TagTextEncoding::Latin1; auto substr = parseSubstring(buffer.get(), m_dataSize, userEncoding, true, diag); auto end = buffer.get() + m_dataSize; diff --git a/tagvalue.cpp b/tagvalue.cpp index e40ab83..56f16a7 100644 --- a/tagvalue.cpp +++ b/tagvalue.cpp @@ -307,7 +307,8 @@ bool TagValue::compareTo(const TagValue &other, TagValueComparisionFlags options } else if (m_type == TagDataType::Popularity || other.m_type == TagDataType::Popularity) { if (options & TagValueComparisionFlags::CaseInsensitive) { const auto lhs = toPopularity(), rhs = other.toPopularity(); - return lhs.rating == rhs.rating && lhs.playCounter == rhs.playCounter && compareData(lhs.user, rhs.user, true); + return lhs.rating == rhs.rating && lhs.playCounter == rhs.playCounter && lhs.scale == rhs.scale + && compareData(lhs.user, rhs.user, true); } else { return toPopularity() == other.toPopularity(); } @@ -646,6 +647,7 @@ Popularity TagValue::toPopularity() const popularity.user = reader.readLengthPrefixedString(); popularity.rating = reader.readFloat64LE(); popularity.playCounter = reader.readUInt64LE(); + popularity.scale = static_cast(reader.readUInt64LE()); } catch (const std::ios_base::failure &) { throw ConversionException(argsToString("Assigned popularity is invalid")); } @@ -1081,6 +1083,7 @@ void TagValue::assignPopularity(const Popularity &value) writer.writeLengthPrefixedString(value.user); writer.writeFloat64LE(value.rating); writer.writeUInt64LE(value.playCounter); + writer.writeUInt64LE(static_cast(value.scale)); auto size = static_cast(s.tellp()); auto ptr = std::make_unique(size); s.read(ptr.get(), s.tellp()); @@ -1179,12 +1182,13 @@ const TagValue &TagValue::empty() } /*! - * \brief Returns the popularity as string in the format "user|rating|play-counter" or an empty - * string if the popularity isEmpty(). + * \brief Returns the popularity as string in the format "rating" if only a rating is present + * or in the format "user|rating|play-counter" or an empty string if the popularity isEmpty(). */ std::string Popularity::toString() const { - return isEmpty() ? std::string() : user % '|' % numberToString(rating) % '|' + playCounter; + return isEmpty() ? std::string() + : ((user.empty() && !playCounter) ? numberToString(rating) : (user % '|' % numberToString(rating) % '|' + playCounter)); } /*! @@ -1198,8 +1202,17 @@ Popularity Popularity::fromString(std::string_view str) if (parts.empty()) { return res; } else if (parts.size() > 3) { - throw ConversionException("Wrong format, expected \"user|rating|play-counter\""); + throw ConversionException("Wrong format, expected \"rating\" or \"user|rating|play-counter\""); } + // treat a single number as rating + if (parts.size() == 1) { + try { + res.rating = stringToNumber(parts.front()); + return res; + } catch (const ConversionException &) { + } + } + // otherwise, read user, rating and play counter res.user = parts.front(); if (parts.size() > 1) { res.rating = stringToNumber(parts[1]); diff --git a/tagvalue.h b/tagvalue.h index f0fc0fc..2b08d15 100644 --- a/tagvalue.h +++ b/tagvalue.h @@ -3,6 +3,7 @@ #include "./localehelper.h" #include "./positioninset.h" +#include "./tagtype.h" #include #include @@ -75,16 +76,28 @@ struct TAG_PARSER_EXPORT Popularity { double rating = 0.0; /// \brief Play counter specific to the user. std::uint64_t playCounter = 0; + /// \brief Specifies the scale used for \a rating by the tag defining that scale. + /// \remarks The value TagType::Unspecified is preserved to denote a generic scale is used (no + /// conversions to/from a generic scale to tag format specific scales have been implemented at + /// this point). + TagType scale = TagType::Unspecified; - bool isEmpty() const - { - return user.empty() && rating != 0.0 && !playCounter; - } std::string toString() const; static Popularity fromString(std::string_view str); + + /// \brief Returns whether the Popularity is empty. The \a scale and zero-values don't count. + bool isEmpty() const + { + return user.empty() && rating == 0.0 && !playCounter; + } + + /// \brief Returns whether two instances are equal. + /// \remarks Currently they must match exactly but in the future conversions between different + /// scales might be implemented and two instances would be considered equal if the ratings are + /// considered equal (even specified using different scales). bool operator==(const Popularity &other) const { - return playCounter == other.playCounter && rating == other.rating && user == other.user; + return playCounter == other.playCounter && rating == other.rating && user == other.user && scale == other.scale; } }; diff --git a/tests/tagvalue.cpp b/tests/tagvalue.cpp index d640ceb..7de1782 100644 --- a/tests/tagvalue.cpp +++ b/tests/tagvalue.cpp @@ -179,13 +179,16 @@ void TagValueTests::testDateTime() void TagValueTests::testPopularity() { - const auto tagValue = TagValue(Popularity{ .user = "foo", .rating = 42, .playCounter = 123 }); + const auto tagValue = TagValue(Popularity{ .user = "foo", .rating = 42, .playCounter = 123, .scale = TagType::VorbisComment }); const auto popularity = tagValue.toPopularity(); CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to popularity (user)", "foo"s, popularity.user); CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to popularity (rating)", 42.0, popularity.rating); CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to popularity (play counter)", std::uint64_t(123), popularity.playCounter); + CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to popularity (scale)", TagType::VorbisComment, popularity.scale); CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to string", "foo|42|123"s, tagValue.toString()); - CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to string", 42, tagValue.toInteger()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to string (only rating)", "43"s, TagValue(Popularity{ .rating = 43 }).toString()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to integer", 42, tagValue.toInteger()); + CPPUNIT_ASSERT_EQUAL_MESSAGE("conversion to unsigned integer", static_cast(42), tagValue.toUnsignedInteger()); CPPUNIT_ASSERT_THROW_MESSAGE( "failing conversion to other type", TagValue("foo|bar"sv, TagTextEncoding::Latin1).toPopularity(), ConversionException); } diff --git a/vorbis/vorbiscommentfield.cpp b/vorbis/vorbiscommentfield.cpp index 3931f57..ea5ef99 100644 --- a/vorbis/vorbiscommentfield.cpp +++ b/vorbis/vorbiscommentfield.cpp @@ -96,8 +96,19 @@ template void VorbisCommentField::internalParse(StreamType &s throw Failure(); } } else if (id().size() + 1 < size) { - // extract other values (as string) - setValue(TagValue(string(data.get() + idSize + 1, size - idSize - 1), TagTextEncoding::Utf8)); + const auto str = std::string_view(data.get() + idSize + 1, size - idSize - 1); + if (id() == VorbisCommentIds::rating()) { + try { + // set rating as Popularity to preserve the scale information + value().assignPopularity(Popularity{ .rating = stringToNumber(str), .scale = TagType::VorbisComment }); + } catch (const ConversionException &) { + // fallback to text + value().assignText(str, TagTextEncoding::Utf8); + } + } else { + // extract other values (as string) + value().assignText(str, TagTextEncoding::Utf8); + } } } else { diag.emplace_back(DiagLevel::Critical, argsToString("Field at ", static_cast(stream.tellg()), " is truncated."), context);