-
Notifications
You must be signed in to change notification settings - Fork 36
POPM (popularity meter) tag support #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Hi @jmbannon sorry for a little late reply. Basically I think it's about time this tag gets integrated in mediafile. Great! maybe you can put as much context into the PR description as possible, for documentation purposes. And then there is something @Wolkenarchitect mentioned which I don't entirely understand: #23 (comment) |
|
Ah, found it, there actually is a changelog: https://mediafile.readthedocs.io/en/latest/#changelog But it seems like @sampsyo did additions to that log in the past when pushing out a release. So not sure if you should add to it within this PR |
|
Collecting some info about this tag and how other taggers/software implement it: Basically the POPM tag looks like this:emailaddress: "value", rating: int 0-255 ,count: int
mp3tag
puddletagexiftool
windows explorermediamonkey
translating scalesthere seems to be discrepancies bertween translation of the 0-255 rating value to other scales in different tools, but that shouldn't bother the implementation of the actual POPM tag, it's simply 0-255 integer: issuecomment @wolkenarchitect
maybe that's offtopic, MP4 in mutagen's implementationhas a |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #78 +/- ##
=======================================
Coverage 93.26% 93.26%
=======================================
Files 16 16
Lines 817 817
Branches 118 118
=======================================
Hits 762 762
Misses 35 35
Partials 20 20 |
|
@JOJ0 i just rebased this. Thank you for the research around the field! I think we should probably add a test for the new field before we merge this tho. Might require us to edit the |
but the current implementation here just defines it as int right? that's not enough to align with the research, it contains email, rating, count eg mp3tag also talks about these 3 values: https://docs.mp3tag.de/mapping/#popularimeter |
|
Yes you are totally right. Might need its own I still think getting some test files first is a good approach to get this going tho :p |
POPM seems to be the standard tag used for ratings across many music apps. This PR adds support for this tag.
Closes #23