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 722311 - matroskaparse: should try to identify data on stream header before going with a blind
matroskaparse: should try to identify data on stream header before going with...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.3.1
Assigned To: Reynaldo H. Verdejo Pinochet
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-01-16 03:20 UTC by Reynaldo H. Verdejo Pinochet
Modified: 2014-01-21 14:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Preliminary solution (1.63 KB, patch)
2014-01-16 03:25 UTC, Reynaldo H. Verdejo Pinochet
needs-work Details | Review
Produce better default caps when none is set (4.48 KB, patch)
2014-01-16 19:29 UTC, Reynaldo H. Verdejo Pinochet
needs-work Details | Review
Produce better default caps when none is set (5.10 KB, patch)
2014-01-21 01:12 UTC, Reynaldo H. Verdejo Pinochet
committed Details | Review

Description Reynaldo H. Verdejo Pinochet 2014-01-16 03:20:11 UTC
While working on another issue, realized filesrc ! matroskaparse ! shout2send
doesn't work for a webm file whereas filesrc ! oggparse ! shout2send does for
ogg/vorbis. Reason for the failure is the quick fallback to
video/x-matroska on the parser while a typefind over the stream header
can easily (albeit costly?) find out the data being pushed by filesrc
is actually video/webm without blindly claiming otherwise.
Comment 1 Reynaldo H. Verdejo Pinochet 2014-01-16 03:25:48 UTC
Created attachment 266425 [details] [review]
Preliminary solution

typefindhelper based solution. Considering it's a
corner-case the cost of iterating over the typefinders
should be justified I guess
Comment 2 Sebastian Dröge (slomo) 2014-01-16 14:48:30 UTC
Comment on attachment 266425 [details] [review]
Preliminary solution

No, matroskaparse already *parses* the actual header of the matroska file and knows if it's webm or something else
Comment 3 Sebastian Dröge (slomo) 2014-01-16 14:49:43 UTC
See gst_matroska_read_common_parse_header(). Just needs to be stored somewhere
Comment 4 Reynaldo H. Verdejo Pinochet 2014-01-16 19:29:47 UTC
Created attachment 266497 [details] [review]
Produce better default caps when none is set

Thanks for your review Sebastian. Hooked up this
to the header/track parsing as you suggested.
Comment 5 Sebastian Dröge (slomo) 2014-01-20 08:55:23 UTC
Review of attachment 266497 [details] [review]:

Looks good in general

::: gst/matroska/matroska-parse.c
@@ +311,3 @@
+  /* reset stream type */
+  parse->common.is_webm = FALSE;
+  parse->common.has_video = FALSE;

These also need to be reset in matroskademux for consistency
Comment 6 Reynaldo H. Verdejo Pinochet 2014-01-21 01:12:58 UTC
Created attachment 266829 [details] [review]
Produce better default caps when none is set

Added the stream type reset to matroskademux as requested.

Thanks for your review.
Comment 7 Sebastian Dröge (slomo) 2014-01-21 09:13:25 UTC
Review of attachment 266829 [details] [review]:

Good to go IMHO, unless you want to implement the 3D support. Why does that have different caps anyway?

::: gst/matroska/matroska-parse.c
@@ +3049,3 @@
+ * type and whether it has video or not.
+ *
+ * FIXME: Do something with video/x-matroska-3d if possible

Yes, see gst-plugins-base/gst/typefind/gsttypefindfunctions.c
Comment 8 Tim-Philipp Müller 2014-01-21 11:09:50 UTC
> Good to go IMHO, unless you want to implement the 3D support. Why does that
> have different caps anyway?

I simply re-used the types on http://matroska.org/technical/specs/notes.html when refining them in 0.11.
Comment 9 Reynaldo H. Verdejo Pinochet 2014-01-21 14:13:56 UTC
Cool. Pushed this one as it was. Will take a look at the 3d
one latter on. Thanks for your time.