Bug 308371 - searching for module AMAROK-SQLTRACKUID -> amarok freezes for long time
Summary: searching for module AMAROK-SQLTRACKUID -> amarok freezes for long time
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Unclassified
Component: Collections/Local (show other bugs)
Version: 2.6.0
Platform: Ubuntu Packages Linux
: NOR normal (vote)
Target Milestone: 2.7
Assignee: Amarok Developers
URL:
Keywords:
: 311921 (view as bug list)
Depends on:
Blocks: 312128
  Show dependency treegraph
 
Reported: 2012-10-14 13:12 UTC by Torsten Eichstädt
Modified: 2012-12-23 18:46 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.7


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Torsten Eichstädt 2012-10-14 13:12:35 UTC
Maybe I edited the metadata of songs that were already referenced in a playlist in the amarok database.  When I opened the playlist, two songs were greyed and did not play.  Right click ->  searching for module AMAROK-SQLTRACKUID.  Then amarok was not responding for a few minutes, but the module does not seem to be installed, the two songs still do not play.

Reproducible: Always

Steps to Reproduce:
1.  Save a playlist in the DB
2.  Edit the metadata of a song in the playlist
3.  Re-open the playlist
Actual Results:  
The edited song is greyed and can not be played

Expected Results:  
I think the song is only referenced in the playlist, and the music data is not changed, so the song should play and the list entry should reflect the metadata changes
Comment 1 Mayank Madan 2012-12-19 09:50:00 UTC
I saved the playlist in database, edited the metadata of the song, still the playlist opens fine and the song can also be played. Using v2.6.90-26-gbcdd84c
Comment 2 Myriam Schweingruber 2012-12-20 21:55:58 UTC
*** Bug 311921 has been marked as a duplicate of this bug. ***
Comment 3 Myriam Schweingruber 2012-12-20 21:56:58 UTC
Thank you for testing. Please everbody, test with Amarok 2.6 beta
Comment 4 Myriam Schweingruber 2012-12-20 23:27:07 UTC
(In reply to comment #3)
> Thank you for testing. Please everbody, test with Amarok 2.6 beta

I mean Amarok 2.7 beta, of course :)
Comment 5 Myriam Schweingruber 2012-12-21 15:50:41 UTC
Comment #5 from duplicate bug 311921 (Graham, please add your comments here, as your bug is marked as a duplicate of this one):

>I have created a new playlist with Amarok 2.7 beta 1, and am still seeing vestiges of this issue.  However, while it no longer prompts me *immediately* with an error message to indicate that it needs a protocol handler, songs in my playlist *are* still greyed out and are not playable.  Double clicking on regular songs in the playlist plays those songs.  Double clicking on a greyed out song continues to show the error message noted above re: "Amarok requires an additional plugin for this operation".

>Reproduction strategy:
>1) Start Amarok, clear the playlist.
>2) Add some songs to the playlist, save it to the Amarok Database
>3) Clear the playlist, and quit Amarok.
>4) Re-start Amarok.
>5) Re-load the playlist that you saved back in (2).

>Only other useful piece of information I can think of here is that the dialog box that appears with the error message is entitled "Install GStreamer Resources".

Are you sure you have it installed correctly? I have never seen that message, so either you have a faulty package or you didn't install all amarok packages. Please also make sure you have all the necessary Gstreamer plugins installed.
Comment 6 graham 2012-12-21 21:25:53 UTC
(In reply to comment #5)
> Are you sure you have it installed correctly? I have never seen that
> message, so either you have a faulty package or you didn't install all
> amarok packages. Please also make sure you have all the necessary
> Gstreamer plugins installed.

I'm pretty sure I've got it installed correctly.  I started with the Fedora SRPM, yanked out their patches, and put the new source tarball in place.  I then made sure that I had all of the necessary devel packages and dependencies installed, built the RPM, and installed it.

Further, being that it only happens for _some_ songs but not all, I'm believing that I have all of the necessary packages/plugins installed (for both Amarok and Gstreamer).  I only have one collection, and all of my music lives in a single directory tree on my hard drive.

Question, though... Amarok wouldn't be trying to use dbus to get this information from the collection, would it?  I ask as I know that I have other performance issues on my machine due to Akonadi/KMail and how it makes use of a copious amount of dbus messages (but that's a separate issue).  I mention it, though, in that if Amarok _is_ using dbus to gather any of this information, it wouldn't surprise me if it was having trouble querying information for some of the songs.  Just a thought; throwing ideas to the wind here.

I should also ask... is there any other information I could provide which would be of assistance here?  Or, possible settings I should double-check (both within Amarok and in any other part of the system that it may be communicating with when repopulating the playlist)?
Comment 7 Ralf Engels 2012-12-22 18:18:20 UTC
I could reproduce two problems:
1. tracks being grayed out
2. AMAROK_SQLTRACKUID protocol source (from a QApt Codec Searcher)

The debug output is inconclusive in both cases but at least the problem is easy to reproduce.
It does not seem to be a settings or installation problem.
Maybe caused by Nepomuk being switched off.
Comment 8 Matěj Laitl 2012-12-22 18:37:17 UTC
(In reply to comment #7)
> I could reproduce two problems:
> 1. tracks being grayed out
> 2. AMAROK_SQLTRACKUID protocol source (from a QApt Codec Searcher)

Good to know. Ralf (or other reporters running current git), if you click on the "Edit Track Details" of such badly-recreated track, what file format does it report? And what Location?

> Maybe caused by Nepomuk being switched off.

I don't think so, basic Amarok installation shouldn't require Nepomuk at all.


(In reply to comment #6)
> Question, though... Amarok wouldn't be trying to use dbus to get this information
> from the collection, would it? I ask as I know that I have other performance issues
> on my machine due to Akonadi/KMail and how it makes use of a copious amount
> of dbus messages (but that's a separate issue).

Amarok uses phonon's methods to query available codecs and it may be using DBus quite likely, but I think this is an Amarok bug.
Comment 9 Ralf Engels 2012-12-22 19:53:21 UTC
Further investigation:

The track location of these tracks look like this: amarok-sqltrackuid://2f9277bb7e49962c1c4c5612811807a1

That explains the reported sympthoms: The tracks cannot be played (as the location is invalid) and a phonon component asks for a plugin to resolve the url.

Now, the reason for this behaviour is the design of the saved playlists.
They store the track uuid, relying on them to stay the same always.
However, as they are computed by editing the tags they uuid might change. 
Now there was once some code to check the playlist for exactly this problem if the uuid of a track changes. Obviously that code not longer works.

I remember that I was reviewing this code and thinking at that time: Wow, no explanation and this looks a little shaky.

The relevant code is at src/core-impl//collections/db/sql/SqlMeta.cpp updatePlaylistsToDb and looks fine at first glance.

The playlist should implement fallbacks for the case that the track uuid is not longer valid.
Comment 10 Matěj Laitl 2012-12-22 21:03:44 UTC
Git commit 0aebbd147cb642d207c90cb39f328b5b01139dbb by Matěj Laitl.
Committed on 22/12/2012 at 21:59.
Pushed by laitl into branch 'master'.

MetaProxy::Track::playableUrl(): don't return uid url

...when the underlying track is not loaded (yet). We return false to
isPlayable(), so we shouldn't be returning anything either.

URL may be something like amarok-sqltrackuid://2f9277bb7e49962c1c4c5612811807a1
and Phonon may choke on such urls trying to find a codec and causing
a hang.

BUGFIXES:
 * Fix hangs and inappropriate messages when trying to play a track from
   a saved playlist that has been deleted in the mean time.
FIXED-IN: 2.7

M  +2    -0    ChangeLog
M  +4    -1    src/core-impl/meta/proxy/MetaProxy.cpp

http://commits.kde.org/amarok/0aebbd147cb642d207c90cb39f328b5b01139dbb
Comment 11 Matěj Laitl 2012-12-22 22:18:42 UTC
(In reply to comment #9)
> The track location of these tracks look like this:
> amarok-sqltrackuid://2f9277bb7e49962c1c4c5612811807a1
> 
> That explains the reported sympthoms: The tracks cannot be played (as the
> location is invalid) and a phonon component asks for a plugin to resolve the
> url.

Yup. The above commit solved the symptoms, the cause is not solved entirely yet.

> Now, the reason for this behaviour is the design of the saved playlists.
> They store the track uuid, relying on them to stay the same always.
> However, as they are computed by editing the tags they uuid might change. 
> Now there was once some code to check the playlist for exactly this problem
> if the uuid of a track changes. Obviously that code not longer works.
> 
> I remember that I was reviewing this code and thinking at that time: Wow, no
> explanation and this looks a little shaky.
> 
> The relevant code is at src/core-impl//collections/db/sql/SqlMeta.cpp
> updatePlaylistsToDb and looks fine at first glance.

Ralf, I think the changes to uirUrl shoule be handled fine. (have to check whether this is the case if the change happens then the Amarok was off, but looks so) What we don't handle are tracks removed from Local Collection still mentioned in the playlists.

It won't be hard for me to add a check to detect deleted tracks, the question is what to do when this happens?
a) remove the track from playlist silently
b) prepend the title with [deleted], but leave. (unplayable, but title, artist, album still saved)
c) don't do anything special, leave stale unplayable tracks in playlists and leave the cleanup to the user
d) interactive window?
Comment 12 Matěj Laitl 2012-12-22 22:19:22 UTC
Sorry, changed status again. (why was UNCONFIRMED pre-selected??)
Comment 13 Torsten Eichstädt 2012-12-23 12:11:56 UTC
(In reply to comment #11)
> Yup. The above commit solved the symptoms, the cause is not solved entirely
> yet.
I agree.

> It won't be hard for me to add a check to detect deleted tracks, the
In the initial bug report I wrote I _edited_ the metadata of some tracks.  They were not deleted.

> question is what to do when this happens?
> a) remove the track from playlist silently
Bad idea.

> b) prepend the title with [deleted], but leave. (unplayable, but title,
> artist, album still saved)
dito.
> c) don't do anything special, leave stale unplayable tracks in playlists and
> leave the cleanup to the user
> d) interactive window?
e) Fix the _source_ of the problem.  E.g. would it be possible to take the inode no. of the tracks instead of the computed uuid?  At 1st sight I think this could be a good solution.  Could you comment, please?
Comment 14 Matěj Laitl 2012-12-23 12:59:28 UTC
(In reply to comment #13)
> (In reply to comment #11)
> > It won't be hard for me to add a check to detect deleted tracks
> 
> In the initial bug report I wrote I _edited_ the metadata of some tracks. 
> They were not deleted.

Oh I see. Then this is another problem, Amarok should be able to track metadata of the tracks fine. (unless you've made the change outside of amarok *and* you've changed the file path in the same time *and* you haven't stamped your tracks using amarok_afttagger)

However, the problem title from the bug title is fixed, so please fill another bug. (perhaps about unplayable songs in saved playlists) I wonder, do you also loose metadata like playcount when this happens? Please make the steps to reproduce really clear and bullet-proof.

> > question is what to do when this happens?
> > a) remove the track from playlist silently
> Bad idea.
> 
> > b) prepend the title with [deleted], but leave. (unplayable, but title,
> > artist, album still saved)
> dito.
> > c) don't do anything special, leave stale unplayable tracks in playlists and
> > leave the cleanup to the user
> > d) interactive window?
> e) Fix the _source_ of the problem.  E.g. would it be possible to take the
> inode no. of the tracks instead of the computed uuid?  At 1st sight I think
> this could be a good solution.  Could you comment, please?

Torsten, this a) - d) was OF COURSE in case the track has been really deleted. As I've already said, Amarok should be really able to track tracks well even if the uid changes. If not, this is a bug.

Torsten, also please don't change status and resolution of bugs, just comment and we'll set it as appropriate, You don't know our workflow and it is really dependant on workflow. The bugs are ours, not yours, we use them daily in our development work.
Comment 15 Matěj Laitl 2012-12-23 18:46:04 UTC
Git commit 2f261b02ede0ecc9a566cbcb2680e06db092dbc1 by Matěj Laitl.
Committed on 23/12/2012 at 19:42.
Pushed by laitl into branch 'master'.

Fix SqlTrack::updatePlaylistToDb(): we need to update url too

This fixes all the remaining problems originally reported in bug 308371.
I've confirmed it works both then the track uid change is triggered from
within Amarok and from external tool when Amarok is not running.

BUGFIXES:
 * Fix updating of Amarok database playlists then track uid changes.
Related: bug 312128
FIXED-IN: 2.7

M  +1    -0    ChangeLog
M  +15   -11   src/core-impl/collections/db/sql/SqlMeta.cpp
M  +1    -0    src/playlistmanager/sql/SqlPlaylist.cpp

http://commits.kde.org/amarok/2f261b02ede0ecc9a566cbcb2680e06db092dbc1