Retagging multiple albums clobbers release year tag

[GoogleCodeExporter]

When selecting multiple albums in the collection and editing their tags, 
Cantata initializes the "Year" field to empty when the individual albums have 
divergent release years. OK'ing the dialog then removes release year 
information from all tags.

It'd be better if Year was seeded with a special "Various" value of some sort 
that would leave that metadata alone unless the field is actually modified.

Original issue reported on code.google.com by eikeh...@gmail.com on 2 Apr 2014 at 5:26

[GoogleCodeExporter]

Ah, oops! Also affects disc number - but this is probably rarely used.

Should be fixed in trunk and branches/1.3 now. Can you please update and 
confirm? Thanks.

Original comment by craig.p....@gmail.com on 2 Apr 2014 at 7:29

• Changed state: Started

[GoogleCodeExporter]

Yup, now that you mention it I can confirm the disc number was affected too; I 
lost disc ordering a few times due to that (the things we put up with until we 
finally get arsed to write a bug report ... :).

I've just tried the latest trunk code, and things are a bit weird ... OK, this 
is going to make this ticket a bit messy, but here goes:

* The tag editor still doesn't initialize those fields to a greyed-out 
"(Various)" as it does e.g. for the Album field.

* OK'ing it no longer removes Year info, though, so yay.

* However, when the MPD update happens and the modified files come in, the 
results are erratic. Some album entries become "Year - Album Title" in the 
view, others are "Album Title (Year)". This can actually split tracks from an 
album over two album entries in the tree.

* Collection behavior after retagging has been a bit wonky for the last few 
days in general. After modifying tags, albums sometimes disappear from the 
collection tree. Restarting Cantata will usually make them come back. Sometimes 
they also come back magically.

Original comment by eikeh...@gmail.com on 2 Apr 2014 at 7:36

[GoogleCodeExporter]

OK, this is *really* odd ... looks like those different title styles / the 
album getting split up is related to royally screwing over the tags for a tag, 
to where every field now reads "Unknown", however it still manages to sort it 
under that artist anyway ...

Original comment by eikeh...@gmail.com on 2 Apr 2014 at 7:40

[GoogleCodeExporter]

The numeric fields never had "(Various)" as they are different widgets. They 
just stay blank. The other fields use the 'placeHolderText' property of 
line-edits.

Which album entries? Where? Cantata trunk will display albums as "AlbumName 
(Year)" but the sorting uses the numeric year value. Can you post a screenshot?

The disappearing albums - is this still happening?

-------

If you edit 1 track, doe the tags edit OK?

Under what scenario is the "Unknown" happening? 

Can you check the tags with kid3 (or similar)?

Original comment by craig.p....@gmail.com on 2 Apr 2014 at 7:55

[GoogleCodeExporter]

> The numeric fields never had "(Various)" as they are different widgets. They 
just stay blank. The other fields use the 'placeHolderText' property of 
line-edits.

Understandable, though it might be really cool to go the extra mile and achieve 
consistency there by throwing more code at it. It's more a of a polish issue 
certainly, though.


> Which album entries? Where? Cantata trunk will display albums as "AlbumName 
(Year)" but the sorting uses the numeric year value. Can you post a screenshot?

ibadi1.png: Before.
ibadi2.png: After.

Steps to reproduce:

1. Right-click the "Ibadi" parent -> Edit Tags.
2. Change genre from KPop to KRock.
3. OK.

Note: I just noticed that on the file system, the problematic track is in 
"Ibadi/Story of Us" path-wise instead of "Ibadi/2008 - Story of Us" like the 
others. This might have come about due to bug 457 at an unknown time. However, 
even after fixing this and consolidating the directories, doing a complete 
Genre switch still results in this splitting.


> If you edit 1 track, doe the tags edit OK?

Yes. Interesting addition to the above section: After changing the genre in a 
single track of an unrelated album of the same artist, the subsequent 
collection update fixed "Story of Us" again. Could there be a race between tag 
writing and an MPD update here?


> Under what scenario is the "Unknown" happening? 

See ibadi2.png; the "odd track out" is read in with Unknown.


> Can you check the tags with kid3 (or similar)?

Using mp3tag on Wine they seem fine. All of the tracks in the album have ID3 
v2.3 tags and nothing else. I am unable to find anything special about the 
track that gets kicked out.

Original comment by eikeh...@gmail.com on 2 Apr 2014 at 8:08

Attachments:

• ibadi1.png
• ibadi2.png

[GoogleCodeExporter]

So - just to be sure. The tags themselves, in the files, are OK? There's no 
data-loss issue here now?

And, when you restart Cantata - everything is OK?

...if so, this narrows down where I need to look...

Original comment by craig.p....@gmail.com on 2 Apr 2014 at 8:18

[GoogleCodeExporter]

The track with the "2008 - Story of Us" album entry. The reason for this is 
that all fields a re blank, so Cantata is guessing the tags from the file name 
and path. The folder is "2008 - Story of Us" so Cantata is using this whole 
string as the album name.

Are the tags of this file still ok on disc?

Original comment by craig.p....@gmail.com on 2 Apr 2014 at 8:20

[GoogleCodeExporter]

Yes and no:

* The file appears to be undamaged and the modification goes through 
successfully. That is, the genre field changes, and the other fields remain 
intact.

* Restarting Cantata doesn't fix things.

* But triggering an update via editing something else in the collection makes 
it recover.

Original comment by eikeh...@gmail.com on 2 Apr 2014 at 8:21

[GoogleCodeExporter]

> Are the tags of this file still ok on disc?

According to mp3tag, the tags in the file remain intact at all steps, only the 
genre changes (as changed through Cantata).

Original comment by eikeh...@gmail.com on 2 Apr 2014 at 8:25

[GoogleCodeExporter]

* ?? So, modifications of the files are OK??

* This is probably because of Cantata's cache file. Is this with the latest 
trunk? I'm guessing so..

* This will be because this changes the MPD database timestamp, causing Cantata 
to refetch the DB listing.

Original comment by craig.p....@gmail.com on 2 Apr 2014 at 8:26

[GoogleCodeExporter]

> * ?? So, modifications of the files are OK??

Yeah, afaics the writes are all OK ... even writes to the "broken" file. I've 
now changed the genre forth and back many times and the file never actually 
lost e.g. album and artist (even despite it popping up as Unknown again and 
again).

> Is this with the latest trunk? I'm guessing so..

Yup, SVN trunk, KDE build.

Original comment by eikeh...@gmail.com on 2 Apr 2014 at 8:29

[GoogleCodeExporter]

Phew! As long as no data is being lost. As I say, I know there area to look 
into now. Will investigate further tomorrow.

Original comment by craig.p....@gmail.com on 2 Apr 2014 at 8:30

[GoogleCodeExporter]

I have a feeling this is related to the even more mysterious "albums sometimes 
disappear for a while after retagging" problem mentioned earlier, too.

No hurries, thanks for looking :).

Original comment by eikeh...@gmail.com on 2 Apr 2014 at 8:32

[GoogleCodeExporter]

Can you update? Not made any changes to the place where I was going to look - 
as it looks fine to me. However, I have been unable to recreate this issue.

Please:
1. Stop Cantata
2. Delete cantata's cache ~/.cache/cantata/library/*.xml.gz
3. Ensure all files have valid tags (use kid3, or some other tag editor)
4. Restart Cantata
5. Change some tags

...I just need to make sure you are starting from a valid setup.

If you can still create the issue, then I will need to add lots of debug code 
in order to determine what is happening.

Original comment by craig.p....@gmail.com on 3 Apr 2014 at 7:05

[GoogleCodeExporter]

p.s. Year and Disc fields now have the "(Various)" indicator :-)

Original comment by craig.p....@gmail.com on 3 Apr 2014 at 7:05

[GoogleCodeExporter]

Unfortunately I'm still experiencing erratic behavior. I'll try my best to 
write down everything as accurately as possible.

Source: Current SVN trunk
Build: KDE

1. First attempt

Files: Three albums by artist "Ibadi". Looking at the tags with kid3, I could 
see that all of the tracks had both ID3 v1.1 and ID3 v2.3.0 tags. The v1.1 tags 
were of low quality (some missing or wrongly-encoded titles). The ID v2.3 tags 
all looked fine.

Steps:
a. Delete Cantata's caches.
b. Run Cantata, wait a while.
c. Uncollapse "Ibadi" top-level entry.
d. Right-click it -> Edit Tags. Change genre from "KPop" to "KRock", OK the 
dialog.

Results:

* Again one of the three albums got split up, with one track sticking out. This 
time, however, it was a different track from a different album - "Songs for 
Ophelia" instead of "Story of Us". "Story of Us" did not get split this time.

* The album entry for "Voyage" did not get marked as new in the collection, 
only the other three (two expected + the split fallout) entries for Ibadi did.

* Examining the files with kid3, I noticed that the genre was not consistent in 
all files. I'm unsure about how things arrived at this state, but given the 
above steps I was expecting it to write "KRock" to all of them. I don't know 
why this did not happen. Otherwise the tags looked good.

My interpretation is that this problem is somehow timing-dependent.


2. Second attempt

I decided to repeat this with a more controlled initial state.

Files: I used kid3 to remove the ID v1.1 tags from all files. I also used kid3 
to consistently set the genre for all tracks to "KPop".

Steps:
a. Delete Cantata's caches.
b. Run Cantata, wait a while.
c. Uncollapse "Ibadi" top-level entry.
d. Right-click it -> Edit Tags. Change genre from "KPop" to "KRock", OK the 
dialog.

Before screenshot: ibadi3.png
After screenshot: ibadi4.png

Results:

* No album got split this time - in the end. However, while I was observing 
Cantata updating, I noticed that it *did* momentarily split up one of the 
albums, and then combined it again.

* The "Voyage" album again did not get marked as new.

* According to kid3, all tracks consistently had "KRock" as genre now (I closed 
Cantata before checking) and were also fine otherwise.

Interpretation: This behavior still hints at a timing problem. Things getting 
split and then getting combined again tracks with earlier observations that 
editing an unrelated track in the collection causes the split to be cleaned up.


3. Third attempt

This was not a full attempt. As mentioned I had closed Cantata previously. Now 
I ran it again, without deleting the cache this time, and changed the genre for 
the albums back to "KPop". The results were the same as in attempt 2: Momentary 
split with a delayed recombine; "Voyage" not marked as new; genre in all tracks 
correct.


Overall conclusion: It appears that removing the ID v1.1 tags has only impacted 
the problem in so far as different timing no longer causes the split to 
"stick". It still happens briefly though, and the behavior of new marking is 
erratic.


Side note: I also have an album that gets flagged as new every time I run 
Cantata for no discernable reason.

Original comment by eikeh...@gmail.com on 3 Apr 2014 at 8:17

Attachments:

• ibadi3.png
• ibadi4.png

[GoogleCodeExporter]

> p.s. Year and Disc fields now have the "(Various)" indicator :-)

Works and is nice! :)

Original comment by eikeh...@gmail.com on 3 Apr 2014 at 8:17

[GoogleCodeExporter]

Hmmm... Bugger :-(

Can you please try the version at: 
http://www.tempfiles.net/download/201404/342032/cantata-1.html

Please do the steps above (i.e. no cache, etc) Also please delete 
~/.cache/cantata/cantata.log (if it exists). And then run cantata from the 
commandline as follows:

CANTATA_DEBUG=1 ./cantata


This will log *all* MPD communications, and some debug about updating the 
songs. This will be saved to ~/.cache/cantata/cantata.log

Please either post a link to ~/.cache/cantata/cantata.log, or attach it here. 
You might want to look at the contents of the file first (as it will contain 
your COMPLETE music listing), and remove parts that are not relevant.

Original comment by craig.p....@gmail.com on 3 Apr 2014 at 8:42

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

On it. For the record, I am going to apply this patch to the codebase which  
I also need to successfully build SVN trunk:

Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt      (revision 4483)
+++ CMakeLists.txt      (working copy)
@@ -227,6 +227,7 @@
      find_package(KDE4 REQUIRED)
      add_definitions(-DENABLE_KDE_SUPPORT)
      set(ENABLE_KDE_SUPPORT TRUE)
+    include_directories(${KDE4_INCLUDES})
  endif (ENABLE_KDE)

  if (NOT ENABLE_QT5 AND NOT WIN32 AND NOT APPLE)
@@ -496,7 +497,6 @@
  add_subdirectory(online/icons)

  if (ENABLE_KDE)
-    include_directories(${KDE4_INCLUDES})
      add_definitions(${KDE4_DEFINITIONS})
      qt4_add_resources(CANTATA_RC_SRCS ${CANTATA_RCS})
      kde4_add_ui_files(CANTATA_UI_HDRS ${CANTATA_UIS})
Index: gui/mainwindow.ui
===================================================================
--- gui/mainwindow.ui   (revision 4483)
+++ gui/mainwindow.ui   (working copy)
@@ -407,7 +407,7 @@
    <customwidget>
     <class>VolumeSlider</class>
     <extends>QSlider</extends>
-   <header>volumeslider.h</header>
+   <header>widgets/volumeslider.h</header>
    </customwidget>
   </customwidgets>
   <resources/>

Original comment by eikeh...@gmail.com on 3 Apr 2014 at 8:52

[GoogleCodeExporter]

Quick note: I just performed the test, but unfortunately forgot to set the env 
variable, so I'll have to repeat it to get the log. This time "Songs for 
Ophelia" got split again, and apparently the genre didn't get written out 
consistently - after restarting Cantata, the files had a mix of genres.

I'm not going to repair the tags with kid3 and then get the log.

Original comment by eikeh...@gmail.com on 3 Apr 2014 at 8:55

[GoogleCodeExporter]

Err - "now going to repair" rather.

Original comment by eikeh...@gmail.com on 3 Apr 2014 at 8:55

[GoogleCodeExporter]

OK, log attached.

In this test, I could again observe the momentary splitting and delayed 
recombine, and again "Voyage" did not get marked as new. According to kid3, the 
genre got consistently written out as "KRock" (changed from "KPop") this time.

I'm going to repeat the test once more, hoping to get a log of the persistant 
split of an album.

Original comment by eikeh...@gmail.com on 3 Apr 2014 at 9:01

Attachments:

• cantata.log.gz

[GoogleCodeExporter]

In this second log, I changed the genre twice without restarting Cantata.

For the KPop -> KRock switch, the results were as above.

For the KRock -> KPop switch, one album got split up into two entries again. 
According to kid3, "KPop" was consistently written to the files.

Original comment by eikeh...@gmail.com on 3 Apr 2014 at 9:04

Attachments:

• cantata2.log.gz

[GoogleCodeExporter]

Interestingly, the split that happened at the end of comment #24 is persistent 
after a Cantata restart (now running SVN trunk again however). The tags are 
fine, though. See the attached screenshot.

This suggests the collection cache contains corrupted data.

Original comment by eikeh...@gmail.com on 3 Apr 2014 at 9:08

Attachments:

• ibadi5.png

[GoogleCodeExporter]

I /think/ I might no the issue. Cantata in trunk asks MPD to only update 
folders that have changed - to do this it send "command_list_begin update 
folder1 update folder2 command_list_end" But MPD emits a datachanged signal 
after each update, causing Cantata to refresh its list *each* time. Hence it 
sometimes gets old data.

Please edit mpdconnection.cpp and change (about line 1245) from:

---------------8<--------------

void MPDConnection::update(const QSet<QString> &dirs)
{
    QByteArray send = "command_list_begin\n";
    foreach (const QString &d, dirs) {
        send += "update "+encodeName(d.endsWith('/') ? d.left(d.length()-1) : d)+"\n";
    }
    send += "command_list_end";

    if (sendCommand(send).ok) {
        if (isMopdidy()) {
            // Mopidy does not support MPD's update command. So, when user presses update DB, what we
            // do instead is clear library/dir caches, then when response to getStats is received,
            // library/dir should get refreshed...
            getStats();
        }
    }
}

---------------8<--------------

to:

---------------8<--------------

void MPDConnection::update(const QSet<QString> &dirs)
{
    Q_UNUSED(dirs)
    update();
}

---------------8<--------------


...the re-do test completely (i.e. remove cache, etc)


---
As to your patch - not sure why this is required. Do you have a volumeslider.h 
on your system that g++ is finding before Cantata's? What is the error that is 
produced?
---

Original comment by craig.p....@gmail.com on 4 Apr 2014 at 7:51

[GoogleCodeExporter]

...don't bother with the changes above, now merged into trunk. So, please just 
update and try again?

Original comment by craig.p....@gmail.com on 4 Apr 2014 at 7:36

[GoogleCodeExporter]

Sorry for the delay - I'm currently traveling and won't be able to repeat the 
test until after the weekend.

Original comment by eikeh...@gmail.com on 5 Apr 2014 at 1:47

[GoogleCodeExporter]

No probs - you mean you don't take Cantata with you??? :-)

Enjoy your weekend!

Original comment by craig.p....@gmail.com on 5 Apr 2014 at 2:42

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Good news: The collection update now takes much longer (since it's no longer 
doing a partial update, I take it) but behaves stably.

Original comment by eikeh...@gmail.com on 6 Apr 2014 at 8:38

[GoogleCodeExporter]

Great! Glad the bug is fixed. As to the longer updates - yeah, I no longer just 
ask MPD to update certain dirs. You would think seeing as the updates are 
wrapped in a bing/end block that it would do them both before informing the 
client. I'll see if I can re-add this functionality later, perhaps by just 
acting on the finished of the last update.Either that, or the simplest way 
would be to do the fast-update case when there is only 1 affected dir - this 
would not help with multiple album updates, but would for the single case.

Original comment by craig.p....@gmail.com on 7 Apr 2014 at 8:33

• Changed state: Verified

[GoogleCodeExporter]

"Premature optimization is the root of all evil", etc. :) I'm fine with the 
speed hit for the more correct behavior. Thanks!

Original comment by eikeh...@gmail.com on 7 Apr 2014 at 8:36