`TagParser::TagValue(float)` no longer works

[jonaski]

TagParser::TagValue(float) no longer works, was this intentional?

tag->setValue(TagParser::KnownField::Rating, TagParser::TagValue(song.rating()));

/home/jonas/Projects/strawberry/strawberry/ext/libstrawberry-tagreader/tagreadertagparser.cpp: In member function ‘virtual bool TagReaderTagParser::SaveSongRatingToFile(const QString&, const spb::tagreader::SongMetadata&) const’:
/home/jonas/Projects/strawberry/strawberry/ext/libstrawberry-tagreader/tagreadertagparser.cpp:472:87: error: call of overloaded ‘TagValue(float)’ is ambiguous
  472 |         tag->setValue(TagParser::KnownField::Rating, TagParser::TagValue(song.rating()));
      |                                                                                       ^
In file included from /usr/include/tagparser/tag.h:6,
                 from /home/jonas/Projects/strawberry/strawberry/ext/libstrawberry-tagreader/tagreadertagparser.cpp:30:
/usr/include/tagparser/tagvalue.h:153:5: note: candidate: ‘TagParser::TagValue::TagValue(TagParser::TagValue&&)’
  153 |     TagValue(TagValue &&other) = default;
      |     ^~~~~~~~
/usr/include/tagparser/tagvalue.h:152:5: note: candidate: ‘TagParser::TagValue::TagValue(const TagParser::TagValue&)’
  152 |     TagValue(const TagValue &other);
      |     ^~~~~~~~
/usr/include/tagparser/tagvalue.h:341:8: note: candidate: ‘TagParser::TagValue::TagValue(uint64_t)’
  341 | inline TagParser::TagValue::TagValue(std::uint64_t value)
      |        ^~~~~~~~~
/usr/include/tagparser/tagvalue.h:333:8: note: candidate: ‘TagParser::TagValue::TagValue(int)’
  333 | inline TagValue::TagValue(int value)
      |        ^~~~~~~~
make[2]: *** [ext/libstrawberry-tagreader/CMakeFiles/libstrawberry-tagreader.dir/build.make:126: ext/libstrawberry-tagreader/CMakeFiles/libstrawberry-tagreader.dir/tagreadertagparser.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:437: ext/libstrawberry-tagreader/CMakeFiles/libstrawberry-tagreader.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

[Martchus]

There was never an overload for float. In fact, TagValue does not support float or double at all at this point. For the rating field use the Popularity (just assign the rating of the Popularity) overload or assign a string (which can be converted to Popularity).

Not that if you would compile your code with warnings about implicit conversions treating them as errors (which I'd recommend) your code would never have compiled anyways.

I could add an overload for double but so far this wouldn't be used by any supported tag field of any tag format. So its only use would be to be converted to something else after all.

[Martchus]

By the way, the rating/popularity has only been introduced recently and so far there's no general/format-independent scale (see my comments in https://github.com/Martchus/tageditor/issues/84). In the future I likely add a generic scale. If you for now put raw values as rating, I'd recommend to set the tag-format/scale accordingly as well so things don't break when I'll introduce scaling (because TagType::Unspecified as scale will then be assumed to be a rating on a generic scale and scaled to the required raw value as needed, which of course breaks if you have already put a raw value there).

[jonaski]

So I assume TagParser::KnownField::Rating only saves POPM (popularity) for ID3 tags? Is there a way to read/write non-standard FMPS_RATING for Vorbis tags?

[Martchus]

No, KnownField::Rating is actually mapped to RATING for Vorbis tags (and at this point you need to ensure yourself to assign a rating within the Popularity struct according to https://en.wikipedia.org/wiki/Vorbis_comment and set scale to TagType::VorbisComment). The analog counts for Matroska's RATING field (see https://matroska.org/technical/tagging.html).

I haven't heared about FMPS_RATING before so that's not mapped at all.

So I would assume it would make more sense to implement scaling in tag parser before you proceed with anything fancy on your side. I suppose it would make most sense to use Matroska's scale the "general" scale and you'd then use e.g. Popularity::fromGeneralScale()/Popularity::toGeneralScale() (and there'd be an internal helper function Popularity::scale(TagType targetScale) used by tag implementations). Or maybe have the from/to functions on TagValue-level.

[Martchus]

Here's a draft how the API for having the tagparser library scale the rating would look like: https://github.com/Martchus/tagparser/pull/24

[Martchus]

Merged the scaling. I decided to make the generic scale from 1 to 5 which should fit best with most UIs (that usually use 5 stars). Fractions are supported and 0 means there's no rating. However, conversions are implemented in all directions so you can also use the ID3v2, the Vorbis or the Matroska scale.

See https://github.com/Martchus/tagparser/pull/24/files#diff-6a6913773dc5efdef4a7bf8fdc202592f248221767f340003b1419cdddb18d8fR95-R101 for details. The newly added functions also have doc strings. (The details are a bit messy as I wanted to preserve the behavior when just dealing with text.)

I suppose the initial question about the float overload is resolved as well. I will only create one if at least one supported tag field uses floats under the hood and you don't need it in your case because Popularity should be used.

[Martchus]

I've noticed that Strawberrry also allows one to set 0 or 0.5 stars. That corresponds to the Matroska scale which you can supposedly use as-is (and not my generic scale which starts at 1). The only disadvantage is that the Matroska scale has no value preserved for "no rating". So Popularity { .rating = 0.0, .scale = TagDataType::Matroska } will actually be mapped to be the lowest rating in other scales (1 in my generic scale and ID3v2 and 20 in Vorbis). If you want "no rating" instead, you needed to make it a special case using one of the other scales, e.g. just Popularity { .rating = 0.0 } (or you just delete the field completely).

However, it looks like Strawberry's UI doesn't distinguish between the lowest rating and no rating at all anyways. (At least it isn't clear to me which case selecting zero stars in its UI corresponds to.)

[Martchus]

The question of assigning a rating should be resolved by now and there was never a way to assign a float. So I'm closing the issue.