-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
albumtypes is converted from a string to a list of individual characters #4528
Comments
I checked the flac file metadata with
How can I fix the broken tags ? Can I disable this plugin ? |
I tried to investigate this issue a little bit. Lets hope this information will be useful I started by re-importing the library: There was nothing displayed when I ran I used Things got worse. Now every single file have these weird tags. Example for
I deleted the tags from the FLAC files for this album:
I ran an update
I added the tag back with a proper value
And I ran update for the last time
I'm not sure I get it right, but it seems that the error comes from the tag in the FLAC file. At this point I will check every single file to make sure tags Edit: I have many more CDs to rip with K3b. I will check if the tag issue comes from the initial tag set by K3b. |
I ripped a brand new CD with K3b. The tags set on the flac files are rather minimal at this point:
I ran an import and got those weird tags again:
I deleted the tracks to re-import them using
Finally, what would be changed by an update:
|
Wow; that certainly does look bad! Is there a chance you could try running the latest source version of beets to see if the bug exists there? |
|
Argh! Thank you for checking. This is certainly bad—and I'm not sure on first glance whether the issue is in beets itself or in MediaFile (possibly recent changes therein). But we should track it down… |
I hit the same problem and was debugging a bit. |
Interesting! Can you clarify what you mean by "sending strings"? Is there a particular line of code you're looking at that assigned to |
I made a quick fix in library.py at 761 in write method adding
as this issue also affects empty albumtypes. In my case I had entries in beets db with albumtype [ and albumtypes with ['[', ']'] or [].
|
I'm also hitting this on v1.6.0. It seems to get /reverted/ by a simple |
I've done some digging, and I believe the root cause here is that 1.6.0's introduction of Before now, I /think/ beets was storing (in its DB) only the singular version of any pluralised element. e.g. With the introduction of If changing MediaFile is an option (paging @sampsyo!) then I think that's the hugely easier fix. I think the harder fix is to teach beets how to round-trip list types into and out of the DB, whilst (I suspect) serialising them as /strings/ in the tagging but de-serialising into lists in the internal model. I'm currently working on this, but I'd very much like to validate my assumptions about the problem, and most important double-check that changing MediaFile isn't an option, before I propose a hairy change! |
Ok, it wasn't as hairy a change as I feared. I've pushed a 90% WorksForMe fix up to https://github.com/jpluscplusm/beets/tree/jcm_fix_albumtypes. I've tested it on 2 albums: 1 with just the one single albumtype, and 1 with 2 albumtypes. I'm getting some annoying flip/flopping on the multi-albumtype album when I issue a I'd be really grateful for some feedback on if this worth finishing, polishing up and PR'ing; or if the change is felt to be officially The Wrong Direction for beets to fix the underlying issue! |
This bug is definitely worth fixing - currently struggling with the same issue across my entire library! |
I'll also try taking a look at it too! |
@sampsyo Apologies for the ping, but I strongly believe that anyone writing tags with a 1.6.0 or later version of beets is going to run into this bug, potentially (and probably) re-tagging every file in their library with bad data! I've put together a rough fix (see #4528 (comment)) but I lack the beets-awareness to know either how to test this reliably (i.e. what tests could've caught this before (or as) the multi-type tag support was merged) or how to make sure any fix I propose doesn't itself prompt a re-tag of any consumer's entire library! Again - apologies for the ping, but I /think/ it's warranted by this bug affecting a sufficiently widespread audience. |
@jpluscplusm i'm not them nor part of beets, but i've been suffering from this bug too so first thank you and secondly i left some generic feedback on your commit. with beet 1.6.0, every there's a remaining problem with the albumtype (singular), |
Thank you @mkhl - I've realigned my fork's As per the overly-verbose commit message, I'm not yet running Beets in anger, and discovered this problem whilst getting started for the first time. Given you're running against an existing library, I'm happy to take your code as being more likely to not affect other consumers, if merged! Also, as I mentioned above, I don't have any tests to prove the correctness of this fix, one way or the other. All the existing tests (obviously!) passed before 1.6.0 was released. Having dug a little into the more e2e tests, I quickly stepped out of my depth. Thus whilst I believe you about there being no need to overload I'll kick off a WIP PR and tag you in it. I hope it gets merged :-) |
Just chiming in to say I have the exact same issue; glad to see the pull request that can stop this behavior. |
@bbaserdem Please do feel free to check out my branch and confirm it works across any and all operations you try out. Having additional test cases of people with pre-existing Beets media libraries confirm there aren't any side effects (and also that the bug's behaviour is fixed) would be really really useful! |
Anyone know if there's a good way to remediate files affected by this bug? Reimporting only seems to work for files with a musicbrainz match, but some albums that I imported as-is were affected as well. Additionally, reimporting is pretty tedious especially with albums that don't have a high match (missing tracks or similar). |
this issue itself only affects the files, so the fix would be to what you're asking is how to fix the database once one has affected some files and then if you can't easily autotag the files, which would fix them you can instead use
this will show you the changes and let you confirm them, and will then by default update both the database and the files (look at the |
tests: add a (xfailing) test for issue #4528
@jpluscplusm I use the archlinux package from community repo, so less straightforward for me to test it out. But I will try to give it a shot to see if it resolves my issues. I will report back when I get around here. |
Just wanted to chime in that I've been rebuilding my library and came across this annoying issue as well. I have used your branch @jpluscplusm and it works like a charm to fix the overall confusion of beets. The first invocation of the write function correctly writes albumtype. I've attached an overview of my library size and the results, iin case that helps further confirm the changes working. albumtypes field with PR branch
|
Would it have been feasible to catch this bug with a general purpose test? ex.
Would have some issues whenever new default fields are added/changed, but would have caught this perhaps. |
This bug has bitten me for the last time. It's actually more convenient to go back to manually organizing my music with the time it takes to modify all the failures this produces. |
Since I found about this issue I simply delete the album types tag right after I import new music:
After that I run:
Of course, this is in no way what could be called a solution and not even close to be a workaround. On the other hand I know that newly imported music don't have those buggy album type tags. I can do that because I already applied this process to my whole library (6110 files, 200Gb). Is it stupid ? Probably, but this is how I'm dealing with the situation until a proper fix gets 'pushed' to the master. |
I too have come across this bug, only noticed it when running my collection through Picard it was correcting it. |
Hello, I have an interesting observation to add, that I'm not sure anyone else has caught/mentioned yet. I'm also having the same issue, in that beets writes both the
(Please note that I have my However, when I run
This seems rather peculiar, because a subsequent Don't know if this helps any, but thought I'd share. EDIT: EDIT 2: I installed eyeD3 and checked the tags of the actual files:
It appears that the EDIT 3: Other tagging software (namely, mediainfo) displays the albumtype as the erroneous entry:
|
The workaround i've been using for this, if the database gets corrupted via a |
Add a note to the docs of the albumtypes plugin warning about issue beetbox#4528 and linking to the manual fixing description.
Add a note to the docs of the albumtypes plugin warning about issue beetbox#4528 and linking to the manual fixing description.
@JOJ0 I faced the same issue, I was able to reproduce this if I ran |
@Codeiffor beets 1.6.0? -> upgrade fix mess: #4582 (comment) HTH :-) |
Problem
After importing new files I will run
beet update
. Some of the tracks will have thealbumtypes
'transformed' from a string (album
) to a list of character (['a', 'l', 'b', 'u', 'm', ';', ' ', 'l', 'i', 'v', 'e']
). Thealbumtype
field will become a single character (album
->a
)I could not get the output with
-vv
because it doesn't seem to happen everytime. I deleted albums that previous triggered this problem and the problem didn't repeat on import and update again.Led to this problem:
Setup
albumtypes
plugin in my config.My configuration (output of
beet config
) is:The text was updated successfully, but these errors were encountered: