Bug 126511 - Not reading ogg/flac tags
Summary: Not reading ogg/flac tags
Status: RESOLVED NOT A BUG
Alias: None
Product: amarok
Classification: Applications
Component: general (show other bugs)
Version: 2.1-SVN
Platform: openSUSE Linux
: LO normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 198346 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-04-30 11:24 UTC by Yogesh M
Modified: 2009-08-09 17:31 UTC (History)
11 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yogesh M 2006-04-30 11:24:23 UTC
Version:           1.4 (using KDE KDE 3.5.0)
Installed from:    SuSE RPMs
OS:                Linux

Whenever my application (amaroK also) tries to read tag from ogg/flac file, following message is generated:

TagLib: Vorbis::File::read() - Could not find the Vorbis comment header.
Comment 1 Yogesh M 2006-04-30 11:33:10 UTC
Sorry, forgot to mention following:
1. Tags were added through flac command line option --tag=FIELD=VALUE
2. Meta info is displayed correctly by Properties dialog of Konqueror
3. Files have .ogg extension.
Comment 2 Scott Wheeler 2006-05-02 17:11:50 UTC
Could you create a small one for testing and put it online somewhere?  (Or 
send it to me via email.)
Comment 3 Yogesh M 2006-05-03 16:10:14 UTC
On 2 May 2006 15:11:51 -0000, Scott Wheeler <wheeler@kde.org> wrote:
> Could you create a small one for testing and put it online somewhere?  (Or
> send it to me via email.)

You can get it from http://golinuxway.tripod.com/Paathshala-03.ogg

I've created this via KAudioCreator with tags fetched from CDDB; first
ripped it to wav and then encoded with this command:-
flac --best --ogg --tag=Artist=%{artist} --tag=Album=%{albumtitle}
--tag=Date=%{year} --tag=Title=%{title} --tag=Tracknumber=%{number}
--tag=Genre=%{genre} --skip=01:00 --until=+00:15 -o %o  %f

Tags of this file show in Konqueror/Properties/Meta data, and JuK; my
app and amaroK give the earlier said error.

--
Yogesh M
Chandigarh, India
Comment 4 Scott Wheeler 2006-05-08 22:30:20 UTC
This seems to be an amaroK issue.  Basically it's either relying on file extensions to determine the file type or relying on TagLib's default one (where the docs basically say, "Don't use this.  It sucks.  Use the native mime system.").

(Note to amaroK folks -- you can use a file type resolver for this, just like is currently done with the extra format support in amaroK.)
Comment 5 Yogesh M 2006-05-10 15:30:55 UTC
But my own app is also unable to read tags. Here is a fragment of code which I use for reading tags: -
        TagLib::FileRef f(file, TRUE);
        if(!f.isNull() && f.tag()) {
            TagLib::Tag *tag = f.tag();

Should I use some other method?
Comment 6 Scott Wheeler 2006-05-10 17:02:08 UTC
Yes, you should be using a FileTypeResolve as I mentioned in my last note.  :-)  (And it says that in the API docs as well as I recall.)

TagLib doesn't have a type detection system.  It just uses extensions.  I didn't bother to reimplement type detection since almost every development framework provides that already.
Comment 7 Martin Aumueller 2006-05-27 12:23:27 UTC
SVN commit 545308 by aumuell:

use KMimeType for guessing file types for tag reading
CCBUG: 126511


 M  +2 -0      ChangeLog  
 M  +81 -2     src/metadata/tplugins.cpp  
 M  +2 -2      src/metadata/wma/wmafile.h  
 M  +2 -2      src/metadata/wma/wmatag.h  


--- trunk/extragear/multimedia/amarok/ChangeLog #545307:545308
@@ -10,6 +10,8 @@
       <andres.oton@gmail.com>. (BR 103185)
 
   CHANGES:
+    * Use KMimeType for resolving file type for metadata acquisition before
+      falling back to extension based guessing.
     * Removed the "detailed mode" in the playlist-browser.
     * Also copy non-local urls to collection when dropped onto collection
       browser.
--- trunk/extragear/multimedia/amarok/src/metadata/tplugins.cpp #545307:545308
@@ -1,24 +1,103 @@
 // (c) 2005 Martin Aumueller <aumuell@reserv.at>
 // See COPYING file for licensing information
 
+#include <config.h>
+#include <debug.h>
 
+#include <qfile.h>
+#include <kmimetype.h>
+
 #include <taglib/fileref.h>
+#include <taglib/tfile.h>
 
-#include <config.h>
-
 #ifdef HAVE_MP4V2
 #include "mp4/taglib_mp4filetyperesolver.h"
+#include "mp4/mp4file.h"
 #else
 #include "m4a/taglib_mp4filetyperesolver.h"
+#include "m4a/mp4file.h"
 #endif
 
 #include "wma/taglib_wmafiletyperesolver.h"
+#include "wma/wmafile.h"
 #include "rmff/taglib_realmediafiletyperesolver.h"
+#include "rmff/taglib_realmediafile.h"
 #include "audible/taglib_audiblefiletyperesolver.h"
+#include "audible/taglib_audiblefile.h"
 #include "aac/aacfiletyperesolver.h"
 
+#include <taglib/mpegfile.h>
+#include <taglib/oggfile.h>
+#include <taglib/oggflacfile.h>
+#include <taglib/vorbisfile.h>
+#include <taglib/flacfile.h>
+
+
+class MimeTypeFileTypeResolver : public TagLib::FileRef::FileTypeResolver
+{
+    TagLib::File *createFile(const char *fileName,
+            bool readAudioProperties,
+            TagLib::AudioProperties::ReadStyle audioPropertiesStyle) const;
+};
+
+TagLib::File *MimeTypeFileTypeResolver::createFile(const char *fileName,
+        bool readProperties,
+        TagLib::AudioProperties::ReadStyle propertiesStyle) const
+{
+    QString fn = QFile::decodeName( fileName );
+    int accuracy = 0;
+
+    KMimeType::Ptr mimetype = KMimeType::findByFileContent( fn, &accuracy );
+    if( accuracy <= 0 )
+        mimetype = KMimeType::findByPath( fn );
+
+    if( mimetype->is( "audio/aac" )
+            || mimetype->is( "audio/mpeg" )
+            || mimetype->is( "audio/mpegurl" )
+            || mimetype->is( "audio/x-mpegurl" )
+            || mimetype->is( "audio/x-mp3" ))
+    {
+        return new TagLib::MPEG::File(fileName, readProperties, propertiesStyle);
+    }
+    else if( mimetype->is( "audio/mp4" ) || mimetype->is( "video/mp4" ) )
+    {
+        return new TagLib::MP4::File(fileName, readProperties, propertiesStyle);
+    }
+    else if( mimetype->is( "audio/x-ms-wma" )
+            || mimetype->is( "video/x-ms-asf" )
+            || mimetype->is( "video/x-msvideo" )
+            || mimetype->is( "video/x-ms-wmv" ) )
+    {
+        return new TagLib::WMA::File(fileName, readProperties, propertiesStyle);
+    }
+    else if( mimetype->is( "audio/vnd.rn-realaudio" )
+            || mimetype->is( "audio/x-pn-realaudio" )
+            || mimetype->is( "audio/x-pn-realaudioplugin" )
+            || mimetype->is( "audio/vnd.rn-realvideo" ) )
+    {
+        return new TagLib::RealMedia::File(fileName, readProperties, propertiesStyle);
+    }
+    else if( mimetype->is( "audio/vorbis" ) )
+    {
+        return new TagLib::Ogg::Vorbis::File(fileName, readProperties, propertiesStyle);
+    }
+    else if( mimetype->is( "audio/x-oggflac" ) )
+    {
+        return new TagLib::Ogg::FLAC::File(fileName, readProperties, propertiesStyle);
+    }
+    else if( mimetype->is( "audio/x-flac" ) )
+    {
+        return new TagLib::FLAC::File(fileName, readProperties, propertiesStyle);
+    }
+
+    debug() << "kmimetype filetype guessing failed for" << fileName << endl;
+
+    return 0;
+}
+
 void registerTaglibPlugins()
 {
+    TagLib::FileRef::addFileTypeResolver(new MimeTypeFileTypeResolver);
     TagLib::FileRef::addFileTypeResolver(new MP4FileTypeResolver);
     TagLib::FileRef::addFileTypeResolver(new WMAFileTypeResolver);
     TagLib::FileRef::addFileTypeResolver(new RealMediaFileTypeResolver);
--- trunk/extragear/multimedia/amarok/src/metadata/wma/wmafile.h #545307:545308
@@ -24,8 +24,8 @@
 
 #include <tfile.h>
 #include <tag.h>
-#include <wmaproperties.h>
-#include <wmatag.h>
+#include "wmaproperties.h"
+#include "wmatag.h"
 
 namespace TagLib {
 
--- trunk/extragear/multimedia/amarok/src/metadata/wma/wmatag.h #545307:545308
@@ -24,8 +24,8 @@
 
 #include <tmap.h>
 #include <tag.h>
-#include <wmafile.h>
-#include <wmaattribute.h>
+#include "wmafile.h"
+#include "wmaattribute.h"
 
 namespace TagLib {
 
Comment 8 Stefan Monov 2006-12-09 12:01:25 UTC
*** Bug has been marked as fixed ***.
Comment 9 Daniel Aleksandersen 2007-08-31 16:31:17 UTC
This bug is still pressent in Amarok 1.4.7. (Have not checked 2.0 svn)

FLAC in OGG container, created by flac 1.1.4. File encoded using:
$ flac --ogg --tag=Artist=Testartist --tag=Album=Testalbum -o /home/test/test.ogg "/media/audiocd/Track 1.wav"
Comment 10 Martin Aumueller 2007-08-31 20:55:07 UTC
The KMimeType stuff which was used instead of extension based guessing proved to crash at times, so that we reverted to extensions, as this seemed to be more stable.
Comment 11 Daniel Aleksandersen 2007-09-01 02:06:14 UTC
...but that does not work. What about reading the file without making any assumptions?

Bug is NOT fixed.
Comment 12 Shane King 2008-01-14 12:53:13 UTC
Reopenned at user request from IRC:

<Trevelyan`>	i can confirm its still broken in 1.4.8
Comment 13 Thorben 2008-01-14 12:55:32 UTC
This is still broken in 1.4.8.
Amarok fails to load tags/meta date for OGG Flac files.
Comment 14 Omnifarious 2008-11-10 08:52:15 UTC
This does not work in 1.4.10 either.  Please fix this bug.  I'm going to be using flac to encode all of my audio CDs and I'd hate to have to stop using Amarok.
Comment 15 Edward Hades 2008-11-10 09:40:20 UTC
Sorry, but Amarok 1.4 is no longer actively supported.

We'll work on Ogg FLAC support for Amarok 2.x.

As for your usage of FLAC to encode files: do you _really_ need Ogg FLAC, not just plain ol' FLAC?

Comment 16 Omnifarious 2008-11-10 09:45:36 UTC
Why would I rather want flac instead of ogg flac?
Comment 17 Edward Hades 2008-11-10 13:15:47 UTC
(In reply to comment #16)
> Why would I rather want flac instead of ogg flac?
> 

Because it is better supported by software (not just Amarok, but for example sox).

Why would you want otherwise?

Comment 18 Omnifarious 2008-11-10 16:57:05 UTC
It just seems cleaner to have everything in the same container format.  I suppose I could do it, but I'm just confused as to why a format that's always been intended as a relatively generic container format is more poorly supported than a specialized format.
Comment 19 Scott Wheeler 2008-11-10 17:05:18 UTC
Just a couple of notes:

- "Pure" FLAC files are much more widely supported than Ogg FLAC
- Xiph now recommends different extensions for different types: http://wiki.xiph.org/index.php/MIME_Types_and_File_Extensions
Comment 20 Jonas Vejlin 2009-04-25 20:57:51 UTC
Correct me if I am wrong but this should be close becouse 1.4 branch of amarok is not maintained any more?
Comment 21 Myriam Schweingruber 2009-04-25 21:04:01 UTC
(In reply to comment #20)
> Correct me if I am wrong but this should be close becouse 1.4 branch of amarok
> is not maintained any more?

Thanks for pointing this out, Jonas. Indeed, 1.4 is not maintained anymore, hence closing this bug.
Comment 22 Martin Aumueller 2009-04-26 23:36:57 UTC
The original reason why this bug seems to have been moved from Taglib to Amarok is, as far as I can see in src/meta/file/File_p.h (trunk/r959286), still valid: Amarok still decides merely based on a file's extension how its meta data should be interpreted. Reopening because of this.
Comment 23 Myriam Schweingruber 2009-06-30 19:37:02 UTC
*** Bug 198346 has been marked as a duplicate of this bug. ***
Comment 24 Myriam Schweingruber 2009-07-08 14:41:02 UTC
*** Bug 198857 has been marked as a duplicate of this bug. ***
Comment 25 Jeff Mitchell 2009-07-28 20:50:17 UTC
I'm a bit confused.

http://wiki.xiph.org/index.php/MIME_Types_and_File_Extensions#.flac_-_audio.2Fflac

defines .flac == audio/flac and .oga to include any type of ogg-encapsulated audio, including FLAC.

Although some MIME magic might make things more robust, as far as I can tell, you are definitely using the wrong file extension for your files, and it shouldn't be surprising that it confuses programs.
Comment 26 Myriam Schweingruber 2009-08-02 19:18:01 UTC
*** Bug 202330 has been marked as a duplicate of this bug. ***
Comment 27 Myriam Schweingruber 2009-08-09 15:33:57 UTC
Martin, Jeff: any news? Should we ask people to change their file names?
Comment 28 Jeff Mitchell 2009-08-09 17:31:02 UTC
I think I already did ask that :-)

No one has responded in a while, so I guess I'll close it for now.