After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 325649 - apetag plugin needs porting to 0.10
apetag plugin needs porting to 0.10
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
Other All
: Normal normal
: 0.10.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2006-01-03 17:57 UTC by John Richard Moser
Modified: 2006-02-06 15:04 UTC
See Also:
GNOME target: ---
GNOME version: 2.13/2.14

gsttagdemux.h (3.24 KB, text/x-chdr)
2006-01-25 12:22 UTC, Tim-Philipp Müller
gsttagdemux.c (40.92 KB, text/x-csrc)
2006-01-25 12:22 UTC, Tim-Philipp Müller
gstapedemux.h (1.73 KB, text/x-chdr)
2006-01-25 12:23 UTC, Tim-Philipp Müller
gstapedemux.c (8.67 KB, text/x-csrc)
2006-01-25 12:25 UTC, Tim-Philipp Müller
updated patch: apetag.tar.gz (12.32 KB, application/x-gzip)
2006-01-28 19:53 UTC, Tim-Philipp Müller
updated patch (55.21 KB, patch)
2006-01-30 11:06 UTC, Tim-Philipp Müller
accepted-commit_now Details | Review
updated patch (56.59 KB, patch)
2006-02-04 17:17 UTC, Tim-Philipp Müller
committed Details | Review

Description John Richard Moser 2006-01-03 17:57:35 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
Setting pipeline to PAUSED ...
** Message: don't know how to handle application/x-apetag
Pipeline is PREROLLING ...


Expected results:
expected results occur in gstreamer-0.8:

$ gst-launch-0.8 playbin
RUNNING pipeline ...
Waiting for the state change...

(at this point the music plays)

Does this happen every time?

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.
Comment 1 Tim-Philipp Müller 2006-01-03 18:13:34 UTC
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).
Comment 2 John Richard Moser 2006-01-03 20:22:56 UTC
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.
Comment 3 Tim-Philipp Müller 2006-01-03 20:37:57 UTC
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 ;-)
Comment 4 Tim-Philipp Müller 2006-01-25 12:22:03 UTC
Created attachment 58080 [details]
Comment 5 Tim-Philipp Müller 2006-01-25 12:22:43 UTC
Created attachment 58081 [details]
Comment 6 Tim-Philipp Müller 2006-01-25 12:23:17 UTC
Created attachment 58082 [details]
Comment 7 Tim-Philipp Müller 2006-01-25 12:25:02 UTC
Created attachment 58083 [details]

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.
Comment 8 Tim-Philipp Müller 2006-01-28 19:53:06 UTC
Created attachment 58282 [details]
updated patch: apetag.tar.gz

Updated patch that should fix the templates screw-up (recursive auto-plugging etc.)
Comment 9 Tim-Philipp Müller 2006-01-30 11:06:52 UTC
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
Comment 10 Jan Schmidt 2006-01-30 21:28:11 UTC
Works for my extensive collection of 2 test files - I say commit it :)
Comment 11 Tim-Philipp Müller 2006-01-30 22:01:33 UTC
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).
Comment 12 Jan Schmidt 2006-01-30 23:20:45 UTC
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.
Comment 13 Tim-Philipp Müller 2006-02-04 17:17:03 UTC
Created attachment 58709 [details] [review]
updated patch

Patch updated to handle the files mentioned in bug #315557.

 - 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.

  - recognise and map 'comments' tag in addition to 'comment' tag.
  - footers are optional if the APE tag is at the beginning of a file
Comment 14 Jan Schmidt 2006-02-05 21:58:48 UTC
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 so the tests can find their test files.

Comment 15 Tim-Philipp Müller 2006-02-06 11:01:23 UTC
Committed to gst-plugins-good:

2006-02-06  Tim-Philipp Müller  <tim at centricular dot net>

        * docs/plugins/
        * 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/
        * gst/apetag/gstapedemux.c:
        * gst/apetag/gstapedemux.h:
        * gst/apetag/gsttagdemux.c:
        * gst/apetag/gsttagdemux.h:
          Add APE tag demuxer (#325649).