Playlist items get scrambled

[Arru]

Songs in playlists get mixed up (replaced by another track). Adding to playlist appears to work as intended, the problem only arises after quitting and relaunching. The mix-up shows signs of being determinate: each affected song tested gets replaced by the next song (in filename alphabetical order, secondly folder name order) on an album. It also appears to be a problem in the playlist storage or playlist to library linking, not in the library itself as cue & BPM are accurately preserved for the songs in question.

The problem seems to cause both wrong song (by "right" artist) in chosen playlist, as well as a chosen song ending up in wrong playlist, I haven't figured out the pattern for this yet but found songs that I have picked at one time, later in lists that they're not supposed to go.

Problem persists after rescanning library. Deleting library and recreating library does restore normal function of playlists, but of course this isn't an acceptable workaround.

I've run into this problem twice; it appears that once Mixxx gets into this "mode", all subsequent adds will be scrambled between restarts. The first time it occured though, a number of previously added playlist items stayed unaffected. The second time, when writing this report, all items are mixed up, additionally it appears that all playlist items get rescrambled again at each restart.

Since I don't know what's triggering this I can't provide instructions for reproducing it. I'm submitting the configuration files which i figure should contain clues of what's gone wrong.

Mixxx 1.7.0 beta 2, OSX 10.5.7, 2007 MBP

[Albert Santoni (gamegod)]

Thanks for your bug report. This will be fixed by the new library in Mixxx 1.8.0, if you can hold out that long!

[Bas van Schaik (bas-tuxes)]

I can confirm this odd behaviour and it's driving me crazy! I have a huge library and prefer to select a number of tracks when preparing for a tour, but that's impossible with this bug crawling around...

Is there any chance that this is fixed somewhere in the next few days in a development version of Mixxx? No problem if I need to recompile or manually create a SVN checkout...

[Bas van Schaik (bas-tuxes)]

Got some more debugging information: some track IDs (as defined in mixxxtrack.xml) of playlist items are incorrectly stored after closing Mixxx.

First, the playlist looks like this:
<Playlists>
  <Playlist>
   <Name>My Playlist</Name>
   <Comment></Comment>
   <List>
    <Id>18694</Id>
    <Id>7854</Id>
    <Id>7184</Id>
    <Id>18073</Id>
    <Id>18072</Id>
    (...)
  </List>
 </Playlist>
</Playlists>

Then, after closing Mixxx, it looks like this:
<Playlists>
  <Playlist>
   <Name>My Playlist</Name>
   <Comment></Comment>
   <List>
    <Id>18698</Id>
    <Id>7854</Id>
    <Id>7184</Id>
    <Id>18077</Id>
    <Id>18076</Id>
    (...)
  </List>
 </Playlist>
</Playlists>

Note the +4 offset on some IDs (18694 becomes 18698, 18073 becomes 19077).

[Albert Santoni (gamegod)]

Hi Bas,

Thanks for the additional information. We're working hard on rewriting the library, and our move to using a database will resolve this issue. I'm actually working on reimplementing playlists at the moment, so hopefully there will be something usable in our Features_sqlite branch in the next 2 weeks if you want to experiment. (Right now, it's a giant work-in-progress, so don't expect anything to work.)

The branch can be checked out here:
https://code.launchpad.net/~mixxxdevelopers/mixxx/features_sqlite

If you're interested in reading more about the new library stuff, we blogged about it a while ago:
http://mixxxblog.blogspot.com/2008/12/rewriting-library.html
Due to our focus on improving MIDI support in 1.7, the new library had to get pushed back to 1.8, but we're working hard on it once again and it should be pretty good when we're done. :)

Thanks again,
Albert

[Bas van Schaik (bas-tuxes)]

Ah, I was already wondering why Mixxx didn't use something like SQLite. Since I really need the playlist feature to work next Monday I decided to perform some more debugging and I think I actually found out what's causing this unexpected behaviour. If you might decide to release a version of Mixxx before 0.8 (with the old library implementation) this will save you some time looking for the problem...

Inside the method TrackCollection::getTrack(int id), an integer track ID (as used in mixxxtrack.xml) is used to look up a TrackInfoObject*. The method assumes that the track ID equals the index of the TrackInfoObject in the QList<TrackInfoObject*> and QList::at is used to retrieve the object in TrackCollection::getTrack(int id):
>return m_qTrackList.at(id);

This call might return a TrackInfoObject of which the track ID does not equal the requested id. This also explains why some tracks are correctly reloaded into the playlist (the track ID of these tracks equal their position in the QList) and why some others are not. Note that trackcollection.cpp contains some commented source code in TrackCollection::getTrack(id) which fails to compile but would yield a correct result when fixed. It even contains a binary search implementation!

It is also relevant to take a look at TrackCollection::addTrack(TrackInfoObject* pTrack), in which TrackInfoObjects are added to the QList using the following call:
> m_qTrackList.append(pTrack);

I'll try to fix the bug (by fixing getTrack or addTrack) and post a patch to this page.

[Bas van Schaik (bas-tuxes)]

There you go:

$ bzr diff trackcollection.cpp
=== modified file 'mixxx/src/trackcollection.cpp'
--- mixxx/src/trackcollection.cpp 2009-08-11 06:17:54 +0000
+++ mixxx/src/trackcollection.cpp 2009-08-27 19:14:56 +0000
@@ -78,7 +78,14 @@

     }
- m_qTrackList.append(pTrack);
+
+ // BEGIN WORKAROUND FOR BUG #404927 -- Bas van Schaik
+ // Ensure capacity of QList:
+ while (pTrack->getId() > m_qTrackList.size() - 1) m_qTrackList.append(NULL);
+
+ // Insert TrackInfoObject at the right index of the QList:
+ m_qTrackList[pTrack->getId()] = pTrack;
+ // END WORKAROUND -- Bas van Schaik
 }

Note that there is a possible null pointer reference in track.cpp which can be fixed using this diff:
$ bzr diff track.cpp
=== modified file 'mixxx/src/track.cpp'
--- mixxx/src/track.cpp 2009-07-15 04:24:00 +0000
+++ mixxx/src/track.cpp 2009-08-27 18:47:26 +0000
@@ -163,7 +163,8 @@
         qDebug() << "Trying to add" << m_pTrackCollection->getSize() << "songs to the library playlist";
         for (int i = 0; i < m_pTrackCollection->getSize(); i++)
         {
- m_qLibraryPlaylist.addTrack(m_pTrackCollection->getTrack(i));
+ TrackInfoObject* track = m_pTrackCollection->getTrack(i);
+ if (track != NULL) m_qLibraryPlaylist.addTrack(track);
         }

[Bas van Schaik (bas-tuxes)]

Oops, the fixes in addTrack/getTrack actually introduced a small regression (saving playlists didn't work anymore, mixxx segfaulted on exit) which is fixed by the following diff:

$ bzr diff trackcollection.cpp
=== modified file 'mixxx/src/trackcollection.cpp'
--- mixxx/src/trackcollection.cpp 2009-08-11 06:17:54 +0000
+++ mixxx/src/trackcollection.cpp 2009-08-27 19:33:51 +0000
@@ -60,9 +60,11 @@
     while (it.hasNext())
     {
         cur_track = it.next();
- QDomElement elementNew = domXML.createElement("Track");
- cur_track->writeToXML(domXML, elementNew);
- trackroot.appendChild(elementNew);
+ if (cur_track){
+ QDomElement elementNew = domXML.createElement("Track");
+ cur_track->writeToXML(domXML, elementNew);
+ trackroot.appendChild(elementNew);
+ }
     }
     root.appendChild(trackroot);
 }

[Bas van Schaik (bas-tuxes)]

Found another null pointer in trackcollection.cpp, which would have also caused segfaults without my patches (in rare cases):

@@ -157,7 +166,7 @@
     while (it.hasNext())
     {
         cur_track = it.next();
- if (cur_track->getLocation()==location)
+ if (cur_track && cur_track->getLocation()==location)
             return cur_track;
     }
     //if (cur_track && cur_track->getLocation()==location)

[Sean M. Pappalardo (pegasus-renegadetech)]

Looks like with Bas' work, this might be fixed in 1.7.1 pending review and testing.

[Arru (arvid-r)]

Good job!

[Bas van Schaik (bas-tuxes)]

Thanks, but note that this is more a workaround than a real fix. There might be other parts of Mixxx depending on the assumption that the track ID equals the position in the QList, but I'm pretty sure I took care of every one of those.

Also, maybe there's a neater way of ensuring the QList's capacity, in stead of just adding NULL references to it?

[Albert Santoni (gamegod)]

Hey Bas,

I can't get your patch to apply cleanly to our 1.7 branch, probably because I'm having to copy and paste all your hunks together to try to form a patch that I can apply.

Can you please upload us a new patch as an attachment, generated using:

bzr diff src/trackcollection.cpp src/track.cpp > bazplaylist.patch

(and including any other files you modified)

Thanks,
Albert

[Albert Santoni (gamegod)]

Oh, and I forgot, please do "bzr update" before you generate the diff. :)

[wilson.joshua.a (wilson-joshua-a)]

I am having the same problem and I am glad to see a fix is underway. How long will it be before 1.7.1 is released and can someone provide me simple instructions on how to do a temp fix so that way I don't have to keep doing this.

Also I noticed that sometimes it will bring up the same song several times when there is only one copy of a song in a library. Is this part of the same bug or what? Thanks!

[Bas van Schaik (bas-tuxes)]

Albert Santoni wrote:
> Oh, and I forgot, please do "bzr update" before you generate the diff.
> :)
>
$ bzr update
> Tree is up to date at revision 2187.

$ bzr diff
(output is attached)

The patch contains some more solutions for null references which I
encountered. Also, some additional debug information is being printed to
stdout, but you should be able to easily remove these statements.

Good luck,

  Bas

[Albert Santoni (gamegod)]

Hi Bas,

I've applied your patch, and it'll be included in 1.7.2. Thanks for
all your hard work!

Albert

On Tue, Dec 22, 2009 at 4:52 AM, Pegasus <email address hidden> wrote:
> ** Changed in: mixxx/1.7
>    Milestone: 1.7.1 => 1.7.2
>
> --
> Playlist items get scrambled
> https://bugs.launchpad.net/bugs/404927
> You received this bug notification because you are a member of Mixxx
> Development Team, which is subscribed to Mixxx.
>

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/5199