Add support for writing the rating tag to all
supported mime types. Since the implementations quite
differ, implement mime type specific paths when necessary.
Details
tests pass
Diff Detail
- Repository
- R286 KFileMetaData
- Branch
- rating_write2
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 7776 Build 7794: arc lint + arc unit
autotests/taglibwritertest.cpp | ||
---|---|---|
409 ↗ | (On Diff #50505) | What are the differences between the various mp3 test cases? |
src/writers/taglibwriter.cpp | ||
153 ↗ | (On Diff #50505) | This should probably go into a separate file, togheter with the inverse transform. This can then be tested independently, i.e. for rating in {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10} { r = wmpRating(rating); Q_ASSERT(rating = balooRating(r)); } |
autotests/taglibwritertest.cpp | ||
---|---|---|
409 ↗ | (On Diff #50505) | It tests all possible ratings, since the commonly used numbers are somewhat arbitrary, and I wanted to make sure writing and extracting works |
src/writers/taglibwriter.cpp | ||
153 ↗ | (On Diff #50505) | We have to test the rest anyway, so I don't see much benefit in this honestly. |
src/writers/taglibwriter.cpp | ||
---|---|---|
229 | No. Mp3 supports ID3v1, ID3v2 or Ape tags, so simply casting to ID3v2 is not possible. |
src/writers/taglibwriter.cpp | ||
---|---|---|
229 | but in case file.tag() returns an ApeTag, the dynamic_cast will return a nullptr. This is a dynamic_cast, not a static_cast. |
src/writers/taglibwriter.cpp | ||
---|---|---|
229 | It always returns a nullptr |
src/writers/taglibwriter.cpp | ||
---|---|---|
80 | This requires a range check here - and then you can use a plain array for lookup. | |
89 | I think this should be split in Thats much more in line with the other calls. |
After some thinking, I have split up the function for writing specific Ape and Vorbis tags, as many more tags like "Opus" etc. are in use for Vorbis tags and not for Ape, and can be easily added this way. I am still using the property map, though.
Are you fine with this?
Can you change the id3v2 rating QHash with range checks and a plain array? Then its fine to go.