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 632682 - [matroskademux] Handle missing CodecPrivate for Vorbis/Theora
[matroskademux] Handle missing CodecPrivate for Vorbis/Theora
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal blocker
: 0.10.26
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-10-20 11:49 UTC by Philip Jägenstedt
Modified: 2010-10-25 06:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Crafted WebM file that has no CodecPrivate block for Vorbis (1.03 KB, audio/webm)
2010-10-20 11:49 UTC, Philip Jägenstedt
  Details
Crafted Matroska file that has no CodecPrivate block for Vorbis (1.03 KB, audio/x-matroska)
2010-10-20 11:50 UTC, Philip Jägenstedt
  Details
trivial patch (1.34 KB, patch)
2010-10-20 11:53 UTC, Philip Jägenstedt
rejected Details | Review
trivial patch (1.05 KB, patch)
2010-10-20 11:53 UTC, Philip Jägenstedt
committed Details | Review
actual patch (1.04 KB, patch)
2010-10-20 11:54 UTC, Philip Jägenstedt
committed Details | Review

Description Philip Jägenstedt 2010-10-20 11:49:49 UTC
Created attachment 172829 [details]
Crafted WebM file that has no CodecPrivate block for Vorbis

This would otherwise crash when dereferencing a NULL pointer.
Comment 1 Philip Jägenstedt 2010-10-20 11:50:53 UTC
Created attachment 172830 [details]
Crafted Matroska file that has no CodecPrivate block for Vorbis

Matroska version of the same.
Comment 2 Philip Jägenstedt 2010-10-20 11:52:20 UTC
Pipeline to reproduce crash:
gst-launch filesrc location=vorbis-no-codecprivate.mkv ! matroskademux ! fakesink
Comment 3 Philip Jägenstedt 2010-10-20 11:53:16 UTC
Created attachment 172832 [details] [review]
trivial patch
Comment 4 Philip Jägenstedt 2010-10-20 11:53:31 UTC
Created attachment 172833 [details] [review]
trivial patch
Comment 5 Philip Jägenstedt 2010-10-20 11:54:07 UTC
Created attachment 172834 [details] [review]
actual patch

I took the liberty of piggy-packing two trivial fixes in matroskademux, take them if you want them.
Comment 6 Tim-Philipp Müller 2010-10-23 15:16:38 UTC
If we return a GST_FLOW_ERROR, we should post a proper error message on the bus. I've changed the patch to do that.

(I guess ideally we'd just mark that track as broken and skip it and continue playing other non-broken tracks if there are any, but I guess it's not really worth it seeing that this is just plain broken, and we're just trying to prevent crashes though hand-crafted files.)

commit 7fcd7d8cf29c22ff1bbcd1969fa353306a4d49e7
Author: Philip Jägenstedt <philipj@opera.com>
Date:   Wed Oct 20 10:21:48 2010 +0200

    matroskademux: Remove useless clearing of send_xiph_headers for Dirac
    
    This looks like a mistake when copy-pasting the Theora code.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=632682

commit 6cf398cdf30a65481ec160165048c4396cff76f7
Author: Philip Jägenstedt <philipj@opera.com>
Date:   Wed Oct 20 13:28:28 2010 +0200

    matroskademux: don't crash if vorbis/theora codec data is missing
    
    Error out properly in this case instead of crashing.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=632682
Comment 7 Tim-Philipp Müller 2010-10-23 15:18:21 UTC
Comment on attachment 172832 [details] [review]
trivial patch

Newer versions of indent sadly do the '! !' thing instead of '!!'. I think we should stick to what the newer versions do.
Comment 8 Philip Jägenstedt 2010-10-25 06:32:43 UTC
OK, that explains the weird style at least.