GNOME Bugzilla – Bug 325649
apetag plugin needs porting to 0.10
Last modified: 2006-02-06 15:04:46 UTC
Please describe the problem: Apparently gstreamer-0.10 will look at an application/x-apetag MP3 and fail to play it. This is very annoying, as I have no idea what idiot tagged the MP3 with monkey's audio tags or whatever cracksmoke this is; but I'd rather not randomly encounter broken MP3s that worked before. Steps to reproduce: This can be reproduced by attempting to play a 10MB MP3 I have that I probably shouldn't attach. Actual results: $ gst-launch-0.10 playbin uri=file:///home/shared/audio/incoming/Utada_Hikaru_-_Simple_and_Clean__Remix_-_English_.mp3 Setting pipeline to PAUSED ... ** Message: don't know how to handle application/x-apetag Pipeline is PREROLLING ... (hang) Expected results: expected results occur in gstreamer-0.8: $ gst-launch-0.8 playbin uri=file:///home/shared/audio/incoming/Utada_Hikaru_-_Simple_and_Clean__Remix_-_English_.mp3 RUNNING pipeline ... Waiting for the state change... (at this point the music plays) Does this happen every time? Yes Other information: It should be possible to determine that the file is an MP3 even if you can't understand the apetag. If I use 'dd' to nab the first 512KiB, it plays; this indicates that yes gstreamer-0.10 is quite capable of identifying an MP3. It should be made capable of discarding the junk at the end as well, like mpg321 does.
It's just that the apetag plugin that reads and strips the tag hasn't been ported to 0.10 yet (should be fairly easy, and is on my TODO list somewhere, but other things take priority at the moment).
Shouldn't gstreamer be capable of genericly ignoring cruft? mpg321 doesn't know about apetags, it just knows it sees junk it doesn't care about.
If you do gst-launch-0.10 filesrc location=/path/to/foo.mp3 ! mad ! audioconvert ! audioresample ! alsasink (which is roughly the equivalent to mpg321) it will ignore the 'junk' it doesn't recognise as well. However, if decodebin/playbin (anything with typefinding really) come into play, it will correctly find the tag before decoding any files, and require the tag to be stripped generically for another level of typefinding (which would then determine the type to be mpeg/audio). It's a tradeoff that's being made here for something as generic and complex (handling audio/video/subtitles and multiple streams of each etc.) as GStreamer. Of course we could just disable the apetag typefinding stuff, but then we'd most likely never port it because no one would really notice that it's missing ;-)
Created attachment 58080 [details] gsttagdemux.h
Created attachment 58081 [details] gsttagdemux.c
Created attachment 58082 [details] gstapedemux.h
Created attachment 58083 [details] gstapedemux.c So, basically GstTagDemux (registered as GstApeDemuxBase for the time being) is based on id3demux from gst-plugins-good, and GstApeDemux implements the necessary vfuncs for the actual tag identification and parsing.
Created attachment 58282 [details] updated patch: apetag.tar.gz Updated patch that should fix the templates screw-up (recursive auto-plugging etc.)
Created attachment 58386 [details] [review] updated patch Updated patch: * this time really a patch * fixes handling of track-number tag * adds handling of year and replay gain tags
Works for my extensive collection of 2 test files - I say commit it :)
I was hoping to be able to commit it to gst-plugins-good actually. The only condition from gstreamer/docs/random/moving-plugins not met for that seems to be the test case (but then, there isn't one for id3demux either...) and the core hacker sponsoring it ;) The other question is whether the GstTagDemux base class (a modified id3demux basically) looks generally sane and useful and should go into the base class collection or not (only really worth it if id3demux might be going to make use it as well really at some point in the future).
It'd help if someone would tell me how to include some binary test files and access them from the test code... then I could whack in a few ID3v1, ID3v2 etc.
Created attachment 58709 [details] [review] updated patch Patch updated to handle the files mentioned in bug #315557. GstTagDemux: - post STREAM TYPE_NOT_FOUND error if typefinding fails in activate function - post STREAM TYPE_NOT_FOUND error (not CORE CAPS error) if typefinding fails in chain function. - before calling ::parse_tags(), set GST_BUFFER_SIZE to the requested tag size and restore it afterwards, as the actual buffer might be larger than requested and parse_tags() might want to easily access the end of the tag without storing the previously calculated size anywhere, or simply assume the buffer it gets is exactly the size requested. GstApeDemux: - recognise and map 'comments' tag in addition to 'comment' tag. - footers are optional if the APE tag is at the beginning of a file
I think this should be committed. The base class can go in gst-plugins-base/gst-libs/gst/base, and we can rework GstID3Demux to use it. We need some test files to base tests around, which should go in gst-plugins-good/tests/check/media/ and a CHECK_MEDIA_DIR=$(top_builddir)/tests/check/media added to the Makefile.am so the tests can find their test files.
Committed to gst-plugins-good: 2006-02-06 Tim-Philipp Müller <tim at centricular dot net> * configure.ac: * docs/plugins/Makefile.am: * docs/plugins/gst-plugins-good-plugins-docs.sgml: * docs/plugins/gst-plugins-good-plugins-sections.txt: * docs/plugins/gst-plugins-good-plugins.hierarchy: * docs/plugins/inspect/plugin-apetag.xml: * gst/apetag/Makefile.am: * gst/apetag/gstapedemux.c: * gst/apetag/gstapedemux.h: * gst/apetag/gsttagdemux.c: * gst/apetag/gsttagdemux.h: Add APE tag demuxer (#325649).