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 784530 - GStreamer no longer plays Vorbis streams with incorrectly framed headers
GStreamer no longer plays Vorbis streams with incorrectly framed headers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.12.1
Other Linux
: Normal blocker
: 1.12.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 788416 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-07-04 19:59 UTC by Tristan Miller
Modified: 2018-06-05 13:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Ogg file with incorrectly framed Vorbis stream headers (2.09 MB, video/ogg)
2017-07-04 20:32 UTC, Tristan Miller
  Details
vorbisdec: Improve "new headers while initialized" handling (5.84 KB, patch)
2018-02-13 07:41 UTC, Edward Hervey
committed Details | Review

Description Tristan Miller 2017-07-04 19:59:33 UTC
After a recent update (probably from GStreamer 1.12.0 to 1.12.1) GStreamer-based media players no longer play many of my Ogg Vorbis files.  For example:

$ gst-play-1.0 "./Amazing Blondel/Evensong/Amazing Blondel - 10 - Under The Greenwood Tree.ogg"
Press 'k' to see a list of keyboard shortcuts.
Now playing /oldhome/psy/share/music/Amazing Blondel/Evensong/Amazing Blondel - 10 - Under The Greenwood Tree.ogg
ERROR Could not decode stream. for file:///oldhome/psy/share/music/Amazing%20Blondel/Evensong/Amazing%20Blondel%20-%2010%20-%20Under%20The%20Greenwood%20Tree.ogg
ERROR debug information: gstvorbisdec.c(352): vorbis_handle_header_packet (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstVorbisDec:vorbisdec0:
couldn't read header packet (-132)
Reached end of play list.

Running ogginfo on such files reports the following problem:

$ ogginfo "./Amazing Blondel/Evensong/Amazing Blondel - 10 - Under The Greenwood Tree.ogg"
Processing file "./Amazing Blondel/Evensong/Amazing Blondel - 10 - Under The Greenwood Tree.ogg"...

New logical stream (#1, serial: 34778229): type vorbis
WARNING: Vorbis stream 1 does not have headers correctly framed. Terminal header page contains additional packets or has non-zero granulepos
Vorbis headers parsed for stream 1, information follows...
Version: 0
Vendor: Xiph.Org libVorbis I 20040629 (1.1.0)
Channels: 2
Rate: 44100
[…]

I suppose that if the Vorbis files aren't to spec, then technically it's OK for GStreamer to fail.  But it had been playing them perfectly and without complaint for the last 15 years, so it's a bit annoying that it's suddenly refusing to play them now.  (Especially since it turns out that hundreds of my Vorbis files have been corrupted in this way -- I am guessing that this is or was another bug in EasyTAG, similar to Bug 776110.)

Is there any chance that the old behaviour could be restored, so that GStreamer at least *attempts* to play the Vorbis files rather than failing outright?
Comment 1 Tim-Philipp Müller 2017-07-04 20:06:52 UTC
Could you make such a file available to us? The first 250kB or so might be enough to reproduce the problem (head --bytes=250k foo.ogg > head.ogg)
Comment 2 Tristan Miller 2017-07-04 20:32:32 UTC
Created attachment 354903 [details]
Ogg file with incorrectly framed Vorbis stream headers

(In reply to Tim-Philipp Müller from comment #1)
> Could you make such a file available to us? The first 250kB or so might be
> enough to reproduce the problem (head --bytes=250k foo.ogg > head.ogg)

Attached is such a file.
Comment 3 Tim-Philipp Müller 2017-07-04 21:17:04 UTC
Thanks.
Comment 4 Tristan Miller 2017-07-05 09:29:15 UTC
Some further information:

If I use a tag editor such as EasyTAG or vorbiscomment to extract and then re-apply the tags to the file, ogginfo no longer issues any warning about the file.  However, GStreamer applications still refuse to play it.  Example:

$ ogginfo somefile.ogg || echo "File has warnings"
File has warnings

$ gst-play-1.0 somefile.ogg
Press 'k' to see a list of keyboard shortcuts.
Now playing /tmp/somefile.ogg
ERROR Could not decode stream. for file:///tmp/somefile.ogg
ERROR debug information: gstvorbisdec.c(352): vorbis_handle_header_packet (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstVorbisDec:vorbisdec0:
couldn't read header packet (-132)
Reached end of play list.

$ vorbiscomment --list  --raw --escapes somefile.ogg >somefile.txt

$ vorbiscomment --write --raw --escapes somefile.ogg <somefile.txt

$ ogginfo somefile.ogg || echo "File has warnings"

$ gst-play-1.0 somefile.ogg
Press 'k' to see a list of keyboard shortcuts.
Now playing /tmp/somefile.ogg
ERROR Could not decode stream. for file:///tmp/somefile.ogg
ERROR debug information: gstvorbisdec.c(352): vorbis_handle_header_packet (): /GstPlayBin:playbin/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstVorbisDec:vorbisdec0:
couldn't read header packet (-132)
Reached end of play list.
Comment 5 Tristan Miller 2017-08-20 11:16:27 UTC
I believe I've discovered how the files can be repaired without re-encoding them:

$ ffmpeg -i corruptfile.ogg -acodec copy fixedfile.ogg
Comment 6 Tristan Miller 2017-10-02 08:25:20 UTC
Possibly related: Bug 788416
Comment 7 Tristan Miller 2017-12-14 21:08:42 UTC
git-bisect indicates that the problem was introduced on Thu Feb 9 12:44:51 2017 +0000 in commit b95725c37e70ad3c1ec8dadb401388db375df482 by Sebastian Dröge of code by Jochen Henneberg.  The commit message was as follows:

>    vorbisdec: reset decoder on vorbis headers update
>    
>    if the vorbis encoder receives new headers it must be
>    reset and re-initialized to continue decoding, e. g.
>    for live streams

Commit details: https://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=b95725c37e70ad3c1ec8dadb401388db375df482
Comment 8 Sebastian Dröge (slomo) 2017-12-14 21:18:51 UTC
Thanks for bisecting and making a testcase available!
Comment 9 Sebastian Dröge (slomo) 2017-12-15 08:17:30 UTC
*** Bug 788416 has been marked as a duplicate of this bug. ***
Comment 10 Sebastian Dröge (slomo) 2017-12-15 08:18:04 UTC
Another testcase, slightly different, is in Bug 788416
Comment 11 Edward Hervey 2018-02-12 16:18:45 UTC
That commit was introduced before 1.12.0, are you sure it worked with 1.12.0 ?
Comment 12 Edward Hervey 2018-02-12 17:26:23 UTC
That commit is indeed the culprit. A simple "workaround" is to put vorbisparse just before which will discard any extra headers.

How are we even meant to fix this ?

I would propose to *accumulate* the (potentially) new headers. According to the vorbis specification there should *always* be 3 headers.

So:
* If a new header comes in, store it
* Once we get a non-header packet:
** check that we indeed have 3 pending headers *and* that they are the expected types (1, 3, 5). If so, reset and process those headers
** If we don't have 3 headers, or they are not the expected type, do not reset and just discard them

This should workaround any potential reconfiguration.
Comment 13 Tristan Miller 2018-02-12 19:04:32 UTC
(In reply to Edward Hervey from comment #11)
> That commit was introduced before 1.12.0, are you sure it worked with 1.12.0
> ?

No, I'm not sure -- that's why I did the bisection and reported the results in Comment 7.

Regarding how to fix the problem, I have no idea, but I'm happy to test whatever you come up with.
Comment 14 Edward Hervey 2018-02-13 07:41:33 UTC
Created attachment 368286 [details] [review]
vorbisdec: Improve "new headers while initialized" handling

If new headers arrive after we are initialized, we need to make
sure that they are indeed valid.

A vorbis bitstream always begins with three header packets and must
be in order.

Also some streams have unframed (invalid?) headers that might
confuse and disrupt the decoding process.

Therefore if ever we see new headers, we accumulate them and once
we get a non-header packet we check them to make sure that:
* We have at least 3 headers
* They are the expected ones (identification, comments and setup)
* They are in order
* Any other "header" is ignored

If those conditions are met, we reset and reconfigure the decoder
Comment 15 Edward Hervey 2018-02-13 07:59:40 UTC
Attachment 368286 [details] pushed as aab5ccc - vorbisdec: Improve "new headers while initialized" handling
Comment 16 Edward Hervey 2018-02-13 08:03:12 UTC
And backported to 1.12 also.
Comment 17 Tristan Miller 2018-06-05 13:48:03 UTC
Thank you!  Works fine for me now.