Bug 325006

Summary: Lyrics are not fetched
Product: amarok Reporter: Ralf Jung <post>
Component: Context View/LyricsAssignee: Amarok Developers <amarok-bugs-dist>
Status: RESOLVED FIXED    
Severity: normal CC: darthcodus
Priority: NOR    
Version: 2.8.0   
Target Milestone: 2.9   
Platform: Debian testing   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: amarok debud stderr

Description Ralf Jung 2013-09-17 15:46:16 UTC
Lyrics are not being correctly fetched in Amarok 2.8 (on Debian testing). Instead, the applet always just show "Fetching...". I can see in Wireshark that it does not even attempt to contact any site (no package for "http || tcp.port==443" filter), so this is not about Lyrics not being available for some songs.

Reproducible: Always

Steps to Reproduce:
1. Start Amarok
2. Play a song
Actual Results:  
Lyrics are "Fetching..." but never appear

Expected Results:  
Lyrics should be fetched properly.

I ran "amarok -d --nofork" and checked the log for lyrics-related messages, and found the following quite suspicious:

amarok:       [ScriptManager] lyrics script started: "LyricWiki" 
amarok:       ESC[07;33m[WARNING]ESC[00;39m bool AmarokScript::ScriptImporter::loadQtBinding(const QString&) Qt Binding "qt.core" not allowed! 
amarok:       ESC[07;33m[WARNING]ESC[00;39m bool AmarokScript::ScriptImporter::loadQtBinding(const QString&) Qt Binding "qt.xml" not allowed!

Further down, I also have script errors:

amarok: ESC[00;36mBEGIN:ESC[00;39m void ScriptManager::notifyFetchLyrics(const QString&, const QString&) 
amarok:   SCRIPT "LyricWiki" :  "script error in function entityDecode: ReferenceError: Can't find variable: QDomDocument"

and a probably (?) unrelated warning

amarok:     ESC[07;33m[WARNING]ESC[00;39m [Playlist::Actions] engineNewTrackPlaying: "Life Is Too Short" does not match what the playlist controller thought it should be

I will attached the full log.
Comment 1 Ralf Jung 2013-09-17 15:46:55 UTC
Created attachment 82372 [details]
amarok debud stderr
Comment 2 Anmol Ahuja 2013-09-17 16:46:32 UTC
Amarok seems to have been compiled without qtbindings for qtscript. Did you build it yourself?
Comment 3 Ralf Jung 2013-09-17 16:51:25 UTC
No, this is taken from the repositories. qtscript is in the package dependencies, but of course it could be added automatically.

Now that I know what to look for, I found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=721882 . Could that be the cause of this problem?
Comment 4 Mark Kretschmann 2013-09-17 16:57:43 UTC
The Amarok package was built without having the Qt Script Bindings installed at compile time. This is why  none of the scripts work. It's a packaging error.

Please inform the Debian packager about this issue, thanks.
Comment 5 Anmol Ahuja 2013-09-17 17:07:14 UTC
Yep, that seems to be it, though it's really the qtbindings module that's missing. Admittedly needs more of a warning message instead of scripts silently failing.
Comment 6 Ralf Jung 2013-09-17 17:11:21 UTC
Now I am confused ;-) is it a compile-time or a run-time problem? I am currently locally re-building the package with the qtscript modules installed, to test if this will change anything. At run-time, they were installed already.

I am talking about the following modules:
libqtscript4-core, libqtscript4-gui, libqtscript4-network, libqtscript4-xml, libqtscript4-sql, libqtscript4-uitools

Also, the debian package contains the following patch, which looks like it may be related to this issue, so I reverted it for the package I am building currently:
http://patch-tracker.debian.org/patch/series/view/amarok/2.8.0-1/debian_disable_qtscriptbindings_check_fix.diff
Comment 7 Anmol Ahuja 2013-09-17 17:24:40 UTC
> Yep, that seems to be it, though it's really the qtbindings module that's
> missing.
I'm sorry, I was referring to your comment's bug report. It's a compile time problem.
And that patch should've fixed things, though it'll build fine without it too if you have the bindings installed.
Comment 8 Ralf Jung 2013-09-17 17:28:11 UTC
I see, so if I understand your correctly the QtScript bindings are a runtime-dependency that's for some reason checked for at compile-time instead, and the Debian patch to avoid needless compile-time installation of the bindings doesn't work (anymore).

Are there any plans on your side to allow compilation without those bindings installed at compile-time (so I could ask the Debian folks to just backport a patch from upstream master)?
Comment 9 Mark Kretschmann 2013-09-17 18:17:59 UTC
Ralf, it's not clear to me why their patch did not work. In Amarok itself we're going to change this to a runtime check instead of compile time, but probably only for Amarok 2.9, as we will need string changes for showing an error dialog.

So they should definitely first fix their package on their own. If there are further issues, we can assist.
Comment 10 Ralf Jung 2013-09-17 19:57:21 UTC
I updated the patch so that it now works fine. I am still not entirely sure why the original patch did not work though, but well. IMHO this patch is also suited for upstream inclusion - should I submit a review request?
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=721882#25
(If you immediately say it's okay, I can also push it to master)
Comment 11 Anmol Ahuja 2013-09-18 10:24:10 UTC
Git commit dcb84c08bc3bac35a56240727b7b8f8ba40087c9 by Anmol Ahuja.
Committed on 18/09/2013 at 10:10.
Pushed by anmolahuja into branch 'master'.

Check for QtBindings at runtime.
Only check for a minimum set of bindings. Disable scripts and display warning if not found.

M  +0    -4    CMakeLists.txt
M  +2    -0    ChangeLog
M  +1    -3    src/CMakeLists.txt
M  +0    -3    src/MainWindow.cpp
M  +35   -1    src/ScriptManager.cpp
M  +2    -0    src/ScriptManager.h
M  +5    -0    src/amarokconfig.kcfg
M  +1    -0    src/configdialog/dialogs/ScriptsConfig.cpp
M  +11   -7    src/scriptengine/ScriptImporter.cpp
M  +1    -0    src/scriptengine/ScriptImporter.h

http://commits.kde.org/amarok/dcb84c08bc3bac35a56240727b7b8f8ba40087c9
Comment 12 vedant agarwala 2014-05-21 10:45:30 UTC
Git commit 2f01af3a1357f2fa3513b25f227e3b758a472848 by Vedant Agarwala, on behalf of Anmol Ahuja.
Committed on 18/09/2013 at 10:10.
Pushed by vedanta into branch 'tagguessing'.

Check for QtBindings at runtime.
Only check for a minimum set of bindings. Disable scripts and display warning if not found.

M  +0    -4    CMakeLists.txt
M  +2    -0    ChangeLog
M  +1    -3    src/CMakeLists.txt
M  +0    -3    src/MainWindow.cpp
M  +35   -1    src/ScriptManager.cpp
M  +2    -0    src/ScriptManager.h
M  +5    -0    src/amarokconfig.kcfg
M  +1    -0    src/configdialog/dialogs/ScriptsConfig.cpp
M  +11   -7    src/scriptengine/ScriptImporter.cpp
M  +1    -0    src/scriptengine/ScriptImporter.h

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