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 397759 - Xingmux may not write a proper Xing/VBR header
Xingmux may not write a proper Xing/VBR header
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
0.10.4
Other Linux
: Normal blocker
: 0.10.6
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-01-17 20:38 UTC by Christian
Modified: 2007-10-15 21:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
output of gst-inspect xingmux (1.76 KB, text/plain)
2007-02-06 07:57 UTC, Christian
  Details
proposed patch (639 bytes, patch)
2007-07-04 14:53 UTC, Gautier Portet
committed Details | Review

Description Christian 2007-01-17 20:38:33 UTC
Banshee (0.11.4) writes several incorrect MP3 tags when encoding to LAME in VBR-mode. Firstly, the 'encoding type' is set to CBR-MP3, secondly, the average bitrate is very much wrong, thirdly, the number of samples varies according to VBR-quality and thus also the length is wrong.
Comment 1 Josiah Ritchie - flickerfly 2007-01-19 00:07:42 UTC
Changelog entry:

2007-01-17  Ruben Vermeersch  <ruben@savanne.be>

        * data/audio-profiles/mp3-lame.xml: Change the vbr-mode parameter, which
        doesn't exist, to vbr. Fixes BGO #397753.

Has this work on bug 397753 affected this bug?
Comment 2 Christian 2007-01-19 21:49:00 UTC
I'm not sure whether I'm gonna start using CVS-versions of Banshee again, now. I'll let you know then if it did.
Comment 3 Ruben Vermeersch 2007-01-22 17:27:38 UTC
I'm going to close the bug until then, as I assume that it's fixed. Feel free to reopen it if it still occurs.
Comment 4 Christian 2007-01-22 17:41:46 UTC
The least you could have done is checked for _yourself_ whether the MP3 headers written by Banshee are correct. I consider this very lazy behavior for a developer involved in a project. I'm just a user who took the time to file a bugreport; it is _your_ responsibility to investigate the report, not _mine_.
Comment 5 Christian 2007-01-22 18:24:43 UTC
Please excuse the tone of above comment. Caught me on the wrong foot today, I didn't mean to insult you.
Comment 6 Christian 2007-02-05 21:09:24 UTC
Here is an MP3 (Lame-produced) with incorrect tags as written by Banshee and one with correct tags produced by Lame.exe as backend to foobar2k. Screenshots of the file properties dialogue in foobar, showing the (incorrect) tags, are also included:

http://rapidshare.com/files/15084360/wrong_tags_from_Banshee.tar.bz2.html
Comment 7 Aaron Bockover 2007-02-05 21:16:36 UTC
It's better to just attach the file to the bug. Using something rapidshare is extremely annoying. It made me wait for 3 minutes before I could even access the download, and scanning the page to get around the ads and "buy instant download" crack is quite a deterrent. 

3.7MB isn't *that* much.
Comment 8 Aaron Bockover 2007-02-05 21:17:26 UTC
Wow, then I had to select a mirror, enter a captcha, and then wait some more. Fantastic.
Comment 9 Aaron Bockover 2007-02-05 21:26:01 UTC
A lot of this should be addressed in 0.11.5+. I made a number of changes to the audio profiles. 

All Banshee is doing is passing information (*just* high level metadata - artist, album, title, track number) to the GStreamer pipeline, so if it gets low level information wrong (i.e. CBR encoding when it should be VBR), then it's a problem in GStreamer.

That said - the pipeline Banshee tries to run for VBR is effectively this, which is correct:

lame vbr=<your_selected_vbr_mode> vbr-quality=<your_vbr_quality> ! xingmux ! id3v2mux

The catch is that a VALID VBR header can ONLY be written by the xingmux element, which is NOT available on a lot of systems. lame itself cannot write a valid VBR/Xing header. Because xingmux isn't a standard element on a lot of systems, it is *optionally* inserted into the pipeline, only if it exists. 

id3v2mux then writes higher level tags (ID3), like artist, album, title, etc.

Verify that you do indeed have xingmux installed. Without it, you should get a valid VBR file in terms of the *audio* encoding, but metadata programs like what fb2k is showing, may not be able to detect it, and will probably show it as CBR:

$ gst-inspect-0.10 xingmux

Ultimately this is not a Banshee bug.

...

The following expression generates the ultimate pipeline.

(gst-construct-pipeline
    "audioconvert"
    (gst-construct-element "lame"
         "mode" 4
         (if (= vbr_mode 0) 
            ("bitrate" bitrate)
            ("vbr" vbr_mode "vbr-quality" (- 9 vbr_quality))))
    (if (and 
        (!= vbr_mode 0) 
        (gst-element-is-available "xingmux")) 
            "xingmux" 
            "")
    (if (gst-element-is-available "id3v2mux")
        "id3v2mux"
        "id3mux"))

You can see the whole XML/S-Expr profile definition for lame here:

http://svn.gnome.org/viewcvs/banshee/trunk/banshee/data/audio-profiles/mp3-lame.xml.in?view=markup
Comment 10 Christian 2007-02-05 21:34:51 UTC
Thanks, I will look at it - btw, this file was encoded using Banshee 0.11.5.

BTW (2) bugzilla didn't allow to upload anything bigger than 1024 kbs if it isn't a patch and asked me to upload it somewhere else. Sorry, that I could not find anything better than rapidshare, I hate it, too, but I don't usually have to upload anything to anywhere.
Comment 11 Aaron Bockover 2007-02-05 21:39:17 UTC
Ah. I know other bugzilla's have something like a 5MB limit. A shame BGO's is so small. Either way, you can attach multiple files - no need for the tarball. It's preferred that way anyway since I could view the screenshots without leaving my browser. Just FYI for future reference.

If it was encoded with 0.11.5, then it's got to be a GStreamer issue. The lame/VBR support in 0.11.5 was extended and tested heavily by a number of people who were having profile-related issues in previous releases.

Please check the output of:

$ gst-inspect-0.10 xingmux
Comment 12 Christian 2007-02-05 21:44:56 UTC
Hmmm...

gst-inspect-0.10 xingmux
Factory Details:
  Long name:    MP3 Xing muxer
  Class:        Formatter/Metadata
  Description:  Adds a Xing header to the beginning of a VBR MP3 file
  Author(s):    Christophe Fergeau <teuf@gnome.org>
  Rank:         none (0)

Plugin Details:
  Name:                 xingheader
  Description:          Add a xing header to mp3 encoded data
  Filename:             /opt/gnome/lib/gstreamer-0.10/libgstxingheader.so
  Version:              0.10.4
  License:              LGPL
  Source module:        gst-plugins-bad
  Binary package:       GStreamer Bad Plug-ins source release
  Origin URL:           Unknown package origin
Comment 13 Christian 2007-02-05 21:47:09 UTC
So, do I have to alter the pipeline like you have shown above to make GStreamer actually use it?
Comment 14 Aaron Bockover 2007-02-05 21:50:06 UTC
Okay, interesting.

When you rip a CD, Banshee should print the _exact_ pipeline being used to the console. Mind ripping a track from a CD and pasting the result?

For example, I set LAME to VBR/Quality=7 in the preferences dialog, and when I rip a track, the following pipeline is run:

audioconvert ! lame mode=4 vbr=4 vbr-quality=2 ! id3v2mux

I do not have xingmux.

No, you should not have to do any configuration other than selecting the profile you want in the preferences dialog. I.e. just configure it in the UI - don't need to mess with pipelines.
Comment 15 Christian 2007-02-05 22:27:47 UTC
Debug: [05.02.2007 23:20:33] (Ripping CD and Encoding with Pipeline) - audioconvert ! lame mode=4 vbr=4 vbr-quality=1 ! xingmux ! id3v2mux

This has been encoded with Banshee 0.11.6 now, fresh from the openSUSE repo. Unfortunately, foobar2k still shows tags like in the sample I've uploaded; wrong encoding profile, wrong number of samples.
Comment 16 Aaron Bockover 2007-02-05 23:47:29 UTC
Okay, given that the pipeline being used should be correct, this is not a Banshee bug as far as I can tell. 

For what it's worth, Banshee (taglib-sharp) shows 44.1KHz sample rate, 2 channels, and 131 Kbps for the bitrate, which should be the average across the VBR samples.

As I do not have xingmux, I'm not quite sure of its capabilities. Perhaps there are some other properties that may need to be set on that element, though I doubt it. Can you paste the *full* output of "gst-inspect-0.10 xingmux"?

Also, just so we can officially remove this from the Banshee context, please verify that the resulting MP3 file from this command is consistent with what has been produced from Banshee:

$ gst-launch-0.10 cdparanoiasrc track=1 ! audioconvert ! lame mode=4 vbr=4 vbr-quality=1 ! xingmux ! id3v2mux ! filesink location=out.mp3

Ensure you have a CD in your drive first. You can set a device=/dev/alternate property on cdparanoiasrc to use a CD-ROM drive other than the system default. The pipeline will create an "out.mp3" file in your current directory.

This will probably end up being reassigned to GStreamer.
Comment 17 Christian 2007-02-06 07:57:30 UTC
Created attachment 81984 [details]
output of gst-inspect xingmux

Indeed the CLI-produced mp3 does have the same problem, so a GStreamer-issues it is. 

I have attached the output of gst-inspect xingmux here.
Comment 18 Aaron Bockover 2007-02-06 19:10:45 UTC
Okay, that output shows that xingmux has no parameters, so I can't really think of a way we could be using it incorrectly. This may just be a bug in xingmux. Reassigning to GStreamer.
Comment 19 Aaron Bockover 2007-02-06 19:11:38 UTC
Christian: you might want to update the Version field for this bug to your gstreamer-plugins-bad version. I set it to 0.10.x for now.
Comment 20 Sebastian Dröge (slomo) 2007-02-06 19:21:37 UTC
xingmux should probably check that it seeked properly to the beginning of the file instead of just doing a seek (which can fail) and then just push the header no matter what happened. Not sure if this is caused by this though...

The xingmux version he has is 0.10.4 according to the gst-inspect output. I'll probably take a closer look later unless someone is faster ;)
Comment 21 Sebastian Dröge (slomo) 2007-02-14 09:23:23 UTC
Hmm, the banshee pipeline adds the xing header after the tag... in the fb2k file it's the other way around which is, when looking at the specs[1], simply wrong as the xing header should be the first mpeg frame. So fb2k probably doesn't find the xing header in the banshee encoded files and only reports an estimate for the duration/number of samples/average bitrate.

OTOH the xing header that xingmux writes seems to be invalid, at least mpg123 can't read it while it can read it in the fb2k encoded file...

Not sure what problem you mean with the tags though... from what I see they're all fine.


[1] http://www.codeproject.com/audio/MPEGAudioInfo.asp#XINGHeader
Comment 22 Sebastian Dröge (slomo) 2007-02-14 09:24:02 UTC
Would also be good if one knows a free app to show those informations, I don't want to install Windows just for this ;)
Comment 23 Aaron Bockover 2007-02-14 16:53:53 UTC
(In reply to comment #21)
> Hmm, the banshee pipeline adds the xing header after the tag... 

No, it doesn't - Christian said this was the resulting pipeline based on his LAME configuration:

audioconvert ! lame mode=4 vbr=4 vbr-quality=1 ! xingmux ! id3v2mux

And the expression that builds it...

    ...
    (if (and 
        (!= vbr_mode 0) 
        (gst-element-is-available "xingmux")) 
            "xingmux" 
            "")
    (if (gst-element-is-available "id3v2mux")
        "id3v2mux"
        "id3mux"))

Not sure what you mean by your comment - the elements are in the right order (xingmux ! id3v2mux).
Comment 24 Sebastian Dröge (slomo) 2007-02-14 17:02:29 UTC
I was reffering to the position in the file. In the pipeline you can't even exchange the two elements because of incompatible caps.

In the fb2k file it's: xing header|id3v2 tag|mp3 data| ...
In the banshee file it's: id3v2 tag|xing header|mp3 data| ...
Comment 25 Christian 2007-02-14 17:26:17 UTC
(In reply to comment #21)
> Not sure what problem you mean with the tags though... from what I see they're
> all fine.

Well, not quite: the number of samples is wrong (thus also the length is wrong), the encoding mode is wrong (it's not CBR but VBR), the bitrate is wrong (it's anything but 128 kbps).
Comment 26 Sebastian Dröge (slomo) 2007-02-14 17:30:46 UTC
Ah ok, so it's only the metadata informations from the xing header (or maybe also the lame header) which are wrong. The xing headers that xingmux produces seem to be invalid anyway so that's probably the cause...
Comment 27 Aaron Bockover 2007-02-14 19:11:32 UTC
(In reply to comment #25)
> Well, not quite: the number of samples is wrong (thus also the length is
> wrong), the encoding mode is wrong (it's not CBR but VBR), the bitrate is wrong
> (it's anything but 128 kbps).

Just FYI: Tags are effectively textual metadata - they are not stream properties, like the number of samples, encoding mode, bitrate, channels, etc. So indeed there doesn't appear to be a problem with the _tags_. This is about a bug in xingmux as it's not writing out a proper XING header, thus the average bitrate cannot be computed, etc. It has nothing to do with tags (ID3).
Comment 28 Alex Lancaster 2007-04-23 08:36:04 UTC
This may actually just be a problem in gstreamer itself reading the correct duration from the file and not a problem with the xing header itself, see bug #428021.
Comment 29 Gautier Portet 2007-07-04 14:50:17 UTC
After two nights trying to understand why why why the frames and bytes counts in xingmux are wrong, I found the real cause of this problem: as mp3val keep trying to tell me, the xing frame should be 417 bytes, not 418.
I don't know for others players, but it fixes the problem with gstreamer players: totem finally get correct lenght for mp3 converter with gstreamer/xingmux.

Comment 30 Gautier Portet 2007-07-04 14:53:49 UTC
Created attachment 91184 [details] [review]
proposed patch
Comment 31 Alex Lancaster 2007-10-04 10:45:31 UTC
(In reply to comment #30)
> proposed patch

This works for me...  Without it CDs ripped under gstreamer with xingmux from gst-plugins-bad CVS still has problems with duration times and bit rates.
Comment 32 Alex Lancaster 2007-10-04 10:46:30 UTC
PS. I'm using rhythmbox for the ripping.  totem/xmms/audacious also sees the correct duration times.
Comment 33 Tim-Philipp Müller 2007-10-04 10:51:37 UTC
Marking as blocker to make sure this gets looked at before the next release.
Comment 34 Sebastian Dröge (slomo) 2007-10-04 12:07:25 UTC
The patch is actually correct... I'll get this committed later or tomorrow with some other minor fixes/cleanups.
Comment 35 Sebastian Dröge (slomo) 2007-10-05 09:20:05 UTC
2007-10-05  Sebastian Dröge  <slomo@circular-chaos.org>

        Patch by: Gautier Portet <kassoulet at gmail dot com>

        * gst/xingheader/gstxingmux.c:
        The size of the Xing header is actually 417 as it's rounded to the
        next smaller integer. Fixes #397759.

        * gst/xingheader/gstxingmux.c: (xing_generate_header),
        (xing_push_header):
        Some random cleanup, add FIXMEs and TODOs and check if the newsegment
        event to the beginning was successful before pushing the header again.
Comment 36 Christian 2007-10-15 21:18:17 UTC
Thank you! ^_^