Summary: | File browser of amarok does not display m4a files | ||
---|---|---|---|
Product: | [Applications] amarok | Reporter: | Andrea <mariofutire> |
Component: | File Browser | Assignee: | Amarok Developers <amarok-bugs-dist> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | amaury.deganseman, leo, matej, nhn, olivier.delaune, sitter |
Priority: | NOR | ||
Version: | 2.5.0 | ||
Target Milestone: | 2.6 | ||
Platform: | Fedora RPMs | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/amarok/e80db0e2a349708465491635fdfe157f112b7ae2 | Version Fixed In: | 2.6 |
Sentry Crash Report: | |||
Attachments: |
Add both aliases for audio/x-m4a and audio/mp4
0001-FileBrowser-sequel-to-display-all-audio-video-files-.patch |
Description
Andrea
2012-07-09 19:30:20 UTC
Sounds like a MIME type problem. Is it available in your MIME types (Systemsettings -> Fille associations -> Audio) ? I have a group called mp4 which associates *.aac *.f4a *.m4a to Amarok with description MPEG-4 audio. In Dolphin they show as such and a double click there starts amarok which plays them properly. It is only Amarok browser which misses them. I've done some more research and read a bit of the code. FileBrowser calls EngineController::supportedMimeTypes() which returns for sure a list containing audio/x-m4a (I've recompiled this function separately and run it) in /usr/share/mime/audio there is a file for this mime type andrea@thinkpad:/usr/share/mime/audio$ grep m4a * mp4.xml: <alias type="audio/x-m4a"/> mp4.xml: <glob pattern="*.m4a"/> I guess the next step is the debugger.... I've tried to rename (not convert) an .m4a file to .mp3 and it is displayed. On top of that, once it goes into the playlist the correct length is displayed. when the m4a goes to the playlist, the length is 00:00. Which phonon backend do you use? The lack of lenght displayed depends on the phonon backend, and in your case, I suspect gstreamer to be the culprit. You shouldn't rename mp4 files to mp3, those are totally different containers. You can rename a m4a to mp4 eventually, but not mp3 I am using gstreamer backend. I can try xine backend. Is the backend used in the file browser as well? I renamed it, just to rule out the fact that maybe it is a bad file and if amarok looked inside, finding a problem, it skipped it from the file browser. But since it works as .mp3, I suspect as you said the mime type to be off. But again, Dolphin identifies it properly.... Maybe the 00:00 length (happening in the right panel) and the missing file in the browser are not related. I dont care about the length too much, but since my collection is mostly .m4a, it is very annoying.. I tried to debug, but the fedora package is optimised and gdb jumps around and gets lost. I need to recompile it. Which will take time. Do you have any idea where to debug this issue? I can extract the part of the code filtering the files and create a little test app. Please do not use the xine backend, it is deprecated and unmaintained since more than a year. Please use the phonon-backend-vlc instead if you want to try another one. Don't forget to restart KDE to make the backend change effective. Do you have the taglib 1.7 package with the mp4 and asf flags activated? That could be another reason for these files not showing up. See also http://taglib.github.com/ Found! It is to do with mime aliases. I've added some debug print in bool MimeTypeFilterProxyModel::filterAcceptsRow( int source_row, const QModelIndex& source_parent ) const and this is what I see ElTren.flv: video/x-flv = 1 ARamZamZam.ogg: audio/x-vorbis+ogg = 1 LaDenseDesCanards.flv: video/x-flv = 1 ElTren.m4a: audio/mp4 = 0 ARamZamZam.m4a: audio/mp4 = 0 ARamZamZam.flv: video/x-flv = 1 ElTren.ogg: audio/x-vorbis+ogg = 1 so for each file I print the name, mime and if it accepted or not by the filter. amarok expects audio/x-mp4, but the mime is audio/mp4. If I read the function QStringList EngineController::supportedMimeTypes(); I see already an attempt to deal with something similar. if( mimeTable.contains( "audio/x-flac" ) && !mimeTable.contains( "audio/flac" ) ) mimeTable << "audio/flac"; maybe we could extend it to mp4? the audio/mp4 come from /usr/share/aliases which contains audio/x-m4a audio/mp4 I am not an expert, but the check should take into account the aliases, either at KDE level or amarok parsing this file. What do you think? of course there was a typo in my comment. the item's mime type is audio/mp4, but amarok expects audio/x-m4a. Created attachment 72524 [details]
Add both aliases for audio/x-m4a and audio/mp4
It seems the official mime type is audio/mp4, which is the same as the type returned for the file.
But Phonon (at least in my case) says it supports audio/x-m4a which is an alias.
There is already a similar check to support both audio/x-flac and audio/flac. This patch extends the same logic to mp4.
Strange as I can't reproduce this problem at all with the phonon-backend-vlc 0.5.0, and normally this should not be solved in Amarok anyway but in the phonon backend and by a correct mime type list in the system. Not sure the patch belongs here... Please try the latest phonon-backend-gstreamer 4.6.1 or the phonon-backend-vlc 0.5.0 first, as suggested in my last comment. Oh also... What phonon may claim to support and what phonon can actually support are two entirely different things these days as VLC can play just about everything unless linux distros break it and GST can install support for just about everything on the fly. So as a matter of fact simply allowing all mimetypes of audio/* would probably be the best thing to do. I read a bit more the code and I think it would work in phonon-backend-vlc since, there is a hardcoded list of mime types supported by vlc which includes: audio/mp4 *and* audio/x-m4a (lines 163 to 249 of https://projects.kde.org/projects/kdesupport/phonon/phonon-vlc/repository/revisions/master/entry/src/backend.cpp) the same thing in gstreamer is done via plugins (lines 218 of https://projects.kde.org/projects/kdesupport/phonon/phonon-gstreamer/repository/revisions/master/entry/gstreamer/backend.cpp) plus some extra logic around lines 273) Then, in gstreamer-plugins-good the decoder for mp4, http://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/isomp4/qtdemux.c at line 367 they mention audio/x-m4a and not the official type audio/mp4. I have not run all that, so I might be wrong. I believe the patch belongs to phonon-gstreamer or gstreamer-plugins. But, as Harald mentioned, maybe it is simlper to show all audio files even if they dont play. I feel it is better to get an error (or no sound) for an unsupported type, rather than hiding from the list (even if it does actually play). But looking again at what happens for flac: It is amarok that adds the 2 aliases since both vlc and gstreamer backends claim to support audio/x-flac, while /usr/share/mime/aliases lists audio/flac as the official name (which is what KDE returns). Let me know if you believe I should open a bug in gstreamer or phonon-backend-gstreamer. Andrea, patch from https://git.reviewboard.kde.org/r/105524/ should fix this, please test it. Git commit 712873769dda415c6a21737428bd1a5305018271 by Matěj Laitl. Committed on 13/07/2012 at 13:05. Pushed by laitl into branch 'master'. EngineController: ditch canDecode() MetaFile::Track::isTrack() is partial replacement. Existing 3 calls to canDecode() were in fact all related to MetaFile classes, so move the method there, simplify it not to query phonon at all (and document it can return false positives). As a consequence, we show all audio and video files in file browser and in other places, even if they wouldn't be playable by the current phonon back-end. This is arguably a cleaner approach and at least lets users discover where the error is. Works quite well for me and prevents failures in many tests. This change is propelled by Bart's and Ralf's legitimate comments on http://git.reviewboard.kde.org/r/105524/ FIXED-IN: 2.6 M +1 -0 ChangeLog M +0 -42 src/EngineController.cpp M +0 -5 src/EngineController.h M +2 -1 src/browsers/filebrowser/FileBrowser.cpp M +3 -1 src/browsers/filebrowser/FileView.cpp M +5 -8 src/core-impl/collections/support/CollectionManager.cpp M +33 -6 src/core-impl/meta/file/File.cpp M +9 -0 src/core-impl/meta/file/File.h http://commits.kde.org/amarok/712873769dda415c6a21737428bd1a5305018271 Hi guys, this does not seem to change anything. The files still dont show up in the browser. Looking at the files patched it seems they refer to the playlist/collection. The browser does not seem to be affected. For me the problem is still in the interaction between MimeTypeFilterProxyModel::filterAcceptsRow( int source_row, const QModelIndex& source_parent ) const and EngineController::supportedMimeTypes() Am I misreading your comment? I don't think canDecode is ever called, the files are filtered beforehand (by MimeTypeFilterProxyModel). I've always been able to see m4a files in the playlist if 1) dragging a folder from amarok's browser 2) double clicking in dolphin but when I dig into the folder in amarok browser, m4a files don't show up (In reply to comment #18) > Hi guys, > > this does not seem to change anything. > The files still dont show up in the browser. > Looking at the files patched it seems they refer to the playlist/collection. > > The browser does not seem to be affected. > For me the problem is still in the interaction between > > MimeTypeFilterProxyModel::filterAcceptsRow( int source_row, const > QModelIndex& source_parent ) const > > and > > EngineController::supportedMimeTypes() > > Am I misreading your comment? > I don't think canDecode is ever called, the files are filtered beforehand > (by MimeTypeFilterProxyModel). > I've always been able to see m4a files in the playlist if > > 1) dragging a folder from amarok's browser > 2) double clicking in dolphin > > but when I dig into the folder in amarok browser, m4a files don't show up Oh sorry, you're right. I removed most cases where shown files are dependent on what Phonon currently uses and most of them called canDecode(). But now I see I didn't remove the one that calls supportedMimeTypes() and which is essentially this bug. Created attachment 72696 [details]
0001-FileBrowser-sequel-to-display-all-audio-video-files-.patch
Andrea, please test above patch and report whether it fixes the problem for you. The file browser should now display all audio and video files + directories, but nothing more. Your patch should still be applied to Amarok master, but only after the 0001-FileBrowser-sequel-to-display-all-audio-video-files-.patch to ensure the latter works. Much better now. I can see them in the browser. My patch look superfluous now. Thanks *** Bug 268896 has been marked as a duplicate of this bug. *** (In reply to comment #22) > Much better now. > I can see them in the browser. Great. > My patch look superfluous now. It isn't, we export supportedMimeTypes() for example in MPRIS2 and we should really incpude audio/mp4 if we have audio/x-mp4. Git commit e80db0e2a349708465491635fdfe157f112b7ae2 by Matěj Laitl. Committed on 23/07/2012 at 12:16. Pushed by laitl into branch 'master'. FileBrowser: sequel to display all audio/video files, don't use Phonon This is the real fix to the bug 303253, previous commit 712873769dda415 that claimed it fixed that fixed something else. FIXED-IN: 2.6 M +1 -1 src/CMakeLists.txt R +13 -8 src/browsers/filebrowser/DirPlaylistTrackFilterProxyModel.cpp [from: src/browsers/filebrowser/MimeTypeFilterProxyModel.cpp - 080% similarity] R +11 -20 src/browsers/filebrowser/DirPlaylistTrackFilterProxyModel.h [from: src/browsers/filebrowser/MimeTypeFilterProxyModel.h - 065% similarity] M +2 -3 src/browsers/filebrowser/FileBrowser.cpp M +2 -2 src/browsers/filebrowser/FileBrowser_p.h http://commits.kde.org/amarok/e80db0e2a349708465491635fdfe157f112b7ae2 Git commit 22460625656fb9cc092edfe9aa0fd1609088c615 by Matěj Laitl. Committed on 24/07/2012 at 12:08. Pushed by laitl into branch 'master'. EngineController: if supportedMimeTypes contains x-m4a, add mp4 too So that we announce correct mimetypes in MPRIS2. Patch from Andrea on CCed bug. M +4 -0 src/EngineController.cpp http://commits.kde.org/amarok/22460625656fb9cc092edfe9aa0fd1609088c615 |