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 761489 - matroskademux: fails to parse opus ('audio/x-unknown, codec-id=(string)A_OPUS')
matroskademux: fails to parse opus ('audio/x-unknown, codec-id=(string)A_OPUS')
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Windows
: Normal normal
: 1.7.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 761588
 
 
Reported: 2016-02-03 10:31 UTC by Arjen Veenhuizen
Modified: 2016-02-23 08:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test file which fails to demux using matroskademux (1.70 MB, application/octet-stream)
2016-02-03 10:31 UTC, Arjen Veenhuizen
  Details
make up an OpusHead block from caps (3.20 KB, patch)
2016-02-04 10:47 UTC, Vincent Penquerc'h
needs-work Details | Review
make up an OpusHead block from caps (3.49 KB, patch)
2016-02-04 11:07 UTC, Vincent Penquerc'h
none Details | Review
make up an OpusHead block from caps (2.81 KB, patch)
2016-02-04 16:01 UTC, Vincent Penquerc'h
none Details | Review
similar fix for matroskademux (2.54 KB, patch)
2016-02-04 16:01 UTC, Vincent Penquerc'h
none Details | Review
Fix opus negotiation with mono streams (1.99 KB, patch)
2016-02-04 16:05 UTC, Vincent Penquerc'h
none Details | Review
similar fix for matroskademux (2.58 KB, patch)
2016-02-04 16:06 UTC, Vincent Penquerc'h
none Details | Review
make up an OpusHead block from caps (2.86 KB, patch)
2016-02-04 16:07 UTC, Vincent Penquerc'h
none Details | Review
make up an OpusHead block from caps (2.87 KB, patch)
2016-02-05 09:22 UTC, Vincent Penquerc'h
committed Details | Review
similar fix for matroskademux (2.11 KB, patch)
2016-02-05 09:22 UTC, Vincent Penquerc'h
committed Details | Review
Fix opus negotiation with mono streams (2.00 KB, patch)
2016-02-05 12:53 UTC, Vincent Penquerc'h
rejected Details | Review

Description Arjen Veenhuizen 2016-02-03 10:31:53 UTC
Created attachment 320328 [details]
Test file which fails to demux using matroskademux

It looks like I found a case where demuxing opus using matroskamux does not work (using GIT master of today). The attached sample file contains VP8 video and OPUS audio:

ffprobe 2016-02-03_-_09-54-34.mkv
ffprobe version git-2016-01-22-74105fc Copyright (c) 2007-2016 the FFmpeg developers
[...]
Input #0, matroska,webm, from '/storage/2016-02-03_-_09-54-34_-_Medialab-6Plus.mkv':
  Metadata:
    encoder         : GStreamer matroskamux version 1.7.1.1
    creation_time   : 2016-02-03 08:54:35
  Duration: N/A, start: 0.024000, bitrate: N/A
    Stream #0:0(eng): Video: vp8, yuv420p, 1280x720, SAR 1:1 DAR 16:9, 1k tbr, 1k tbn, 1k tbc (default)
    Metadata:
      title           : Video
    Stream #0:1(eng): Audio: opus, 48000 Hz, stereo, fltp (default)
    Metadata:
      title           : Audio

The source of the clip is a stream generated by Google's libwebrtc for Ios received over RTP, depayloaded and muxed using matroskamux.

The file plays just fine with ffplay, but 

playbin uri=file:///storage/2016-02-03_-_09-54-34.mkv shows the following output:

Setting pipeline to PAUSED ...
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: ring-buffer-max-size = 0
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: buffer-size = -1
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: buffer-duration = -1
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: use-buffering = false
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: download = false
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: uri = file:///storage/2016-02-03_-_09-54-34.mkv
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: connection-speed = 0
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: source = "\(GstFileSrc\)\ source"
Pipeline is PREROLLING ...
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstTypeFindElement:typefind.GstPad:src: caps = "NULL"
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstTypeFindElement:typefind.GstPad:src: caps = "NULL"
Missing element: audio/x-unknown decoder
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstMultiQueue:multiqueue0: max-size-buffers = 5
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstMultiQueue:multiqueue0: max-size-time = 0
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstMultiQueue:multiqueue0: max-size-bytes = 2097152
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstMultiQueue:multiqueue0.GstPad:sink_0: caps = "video/x-vp8\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstMultiQueue:multiqueue0.GstPad:src_0: caps = "video/x-vp8\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstVP8Dec:vp8dec0.GstPad:sink: caps = "video/x-vp8\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstVP8Dec:vp8dec0.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstMultiQueue:multiqueue0: max-size-buffers = 5
WARNING: from element /GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: No decoder available for type 'audio/x-unknown, codec-id=(string)A_OPUS'.
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstMultiQueue:multiqueue0: max-size-time = 0
Additional debug info:
gsturidecodebin.c(939): unknown_type_cb (): /GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0
/GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstMultiQueue:multiqueue0: max-size-bytes = 2097152
/GstPlayBin:playbin0/GstInputSelector:inputselector0.GstSelectorPad:sink_0: always-ok = false
/GstPlayBin:playbin0/GstInputSelector:inputselector0.GstSelectorPad:sink_0: active = true
/GstPlayBin:playbin0/GstInputSelector:inputselector0: active-pad = "\(GstSelectorPad\)\ sink_0"
/GstPlayBin:playbin0/GstInputSelector:inputselector0.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink.GstGhostPad:video_sink.GstProxyPad:proxypad5: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstStreamSynchronizer:streamsynchronizer0.GstPad:src_0: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vdbin.GstGhostPad:sink.GstProxyPad:proxypad9: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vdbin/GstVideoConvert:vdconv.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vdbin/GstDeinterlace:deinterlace.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vdbin.GstGhostPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin.GstGhostPad:sink.GstProxyPad:proxypad8: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstQueue:vqueue.GstPad:sink: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin.GstGhostPad:sink: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin.GstGhostPad:sink: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv.GstGhostPad:sink.GstProxyPad:proxypad6: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv.GstGhostPad:sink.GstProxyPad:proxypad6: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv/GstVideoConvert:conv.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv/GstVideoScale:scale.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv.GstGhostPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstXvImageSink:xvimagesink0.GstPad:sink: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv.GstGhostPad:src.GstProxyPad:proxypad7: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv/GstVideoScale:scale.GstPad:sink: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv/GstVideoConvert:conv.GstPad:sink: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstPlaySink:playsink/GstBin:vbin/GstPlaySinkVideoConvert:vconv.GstGhostPad:sink: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)1280\,\ height\=\(int\)720\,\ interlace-mode\=\(string\)progressive\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ chroma-site\=\(string\)mpeg2\,\ colorimetry\=\(string\)bt709\,\ framerate\=\(fraction\)0/1"
/GstPlayBin:playbin0/GstInputSelector:inputselector0.GstSelectorPad:sink_0: tags = "taglist\,\ video-codec\=\(string\)\"VP8\\\ video\"\;"
/GstPlayBin:playbin0/GstInputSelector:inputselector0.GstSelectorPad:sink_0: tags = "taglist\,\ video-codec\=\(string\)\"VP8\\\ video\"\,\ container-format\=\(string\)Matroska\;"
/GstPlayBin:playbin0/GstInputSelector:inputselector0.GstSelectorPad:sink_0: tags = "taglist\,\ video-codec\=\(string\)\"On2\\\ VP8\"\,\ container-format\=\(string\)Matroska\;"
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock

Notice the:

WARNING: from element /GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0: No decoder available for type 'audio/x-unknown, codec-id=(string)A_OPUS'.

Is this an issue in matroskamux or matroskademux? I suspect the latter but maybe something goes haywire while muxing.


Ps. this is a follow-up bug report on https://bugzilla.gnome.org/show_bug.cgi?id=727305
Comment 1 Vincent Penquerc'h 2016-02-03 17:12:39 UTC
audio/x-unknown says it all I think.

Quick check yields:

gst_matroska_demux_audio_caps: Invalid Opus codec data size

I'll be debugging that one. It could be the muxer or encoder got it wrong, or there's new extra data format.
Comment 2 Sebastian Dröge (slomo) 2016-02-03 21:27:27 UTC
Invalid codec data is not nice, but for Opus (with <= 2 channels) we can recover from that easily. The codec data contains nothing that is really needed in that case.
Comment 3 Vincent Penquerc'h 2016-02-04 09:35:52 UTC
There's no extra data at all in that stream, in fact. So we can't tell number of channels. I guess we could assume 2, but that will fail if not.
Comment 4 Vincent Penquerc'h 2016-02-04 09:37:14 UTC
Oh, and it seems it was created by gstreamer so we got another bug there :/
Comment 5 Vincent Penquerc'h 2016-02-04 09:40:26 UTC
Reading the linked bug, I guess matroskamux should make up a header from caps if there's none already.
Comment 6 Sebastian Dröge (slomo) 2016-02-04 10:32:51 UTC
There are helper functions for the in libgstpbutils, yes.

Also matroska stores the number of channels separately from the codec data, it should still be there.
Comment 7 Vincent Penquerc'h 2016-02-04 10:47:38 UTC
Created attachment 320416 [details] [review]
make up an OpusHead block from caps

This patch creates a OpusHead block from caps, if no stream header is present.

This should be all that's needed, except a test case here shows incoming caps have 0 channels. I'm not sure how that's valid in the first place, so there's a bit more debugging to be had.
Comment 8 Vincent Penquerc'h 2016-02-04 10:50:19 UTC
(In reply to Sebastian Dröge (slomo) from comment #6)
> There are helper functions for the in libgstpbutils, yes.
> 
> Also matroska stores the number of channels separately from the codec data,
> it should still be there.

Do you mean you would generate the OpusHead in the demuxer, rather than the muxer ? It's a mandatory block I think, so we have to do it in the muxer anyway, but we could do both as a safety thing.
Comment 9 Sebastian Dröge (slomo) 2016-02-04 10:50:27 UTC
Comment on attachment 320416 [details] [review]
make up an OpusHead block from caps

As said, there's helper API for this available in pbutils. gst_codec_utils_opus_create_header() and gst_codec_utils_opus_parse_caps() is probably useful here.
Comment 10 Vincent Penquerc'h 2016-02-04 11:07:25 UTC
Created attachment 320419 [details] [review]
make up an OpusHead block from caps

Done, I didn't know about these functions :)

That code seems to deal with "0" channels, so it all seems to work fine from RTP.
Comment 11 Sebastian Dröge (slomo) 2016-02-04 12:13:57 UTC
(In reply to Vincent Penquerc'h from comment #8)
> (In reply to Sebastian Dröge (slomo) from comment #6)
> > There are helper functions for the in libgstpbutils, yes.
> > 
> > Also matroska stores the number of channels separately from the codec data,
> > it should still be there.
> 
> Do you mean you would generate the OpusHead in the demuxer, rather than the
> muxer ? It's a mandatory block I think, so we have to do it in the muxer
> anyway, but we could do both as a safety thing.

In the muxer we must handle it, in the demuxer we could to be nice :)
Comment 12 Sebastian Dröge (slomo) 2016-02-04 12:16:11 UTC
Review of attachment 320419 [details] [review]:

::: gst/matroska/matroska-demux.c
@@ +5602,3 @@
     } else {
+      GST_WARNING ("Invalid Opus codec data size (got %" G_GSIZE_FORMAT
+          ", expected 19)", context->codec_priv_size);

It can be bigger than 19 bytes
Comment 13 Vincent Penquerc'h 2016-02-04 16:01:24 UTC
Created attachment 320450 [details] [review]
make up an OpusHead block from caps
Comment 14 Vincent Penquerc'h 2016-02-04 16:01:54 UTC
Created attachment 320451 [details] [review]
similar fix for matroskademux
Comment 15 Vincent Penquerc'h 2016-02-04 16:05:41 UTC
Created attachment 320452 [details] [review]
Fix opus negotiation with mono streams
Comment 16 Vincent Penquerc'h 2016-02-04 16:06:38 UTC
Created attachment 320453 [details] [review]
similar fix for matroskademux
Comment 17 Vincent Penquerc'h 2016-02-04 16:07:00 UTC
Created attachment 320454 [details] [review]
make up an OpusHead block from caps
Comment 18 Vincent Penquerc'h 2016-02-04 16:08:12 UTC
All done. The demux case needed a fix in opusdec's negotiation too. I guess this can still fail in case downstream gives us weird caps, though.
Comment 19 Sebastian Dröge (slomo) 2016-02-04 21:29:16 UTC
Comment on attachment 320452 [details] [review]
Fix opus negotiation with mono streams

This breaks the case where downstream only accepts mono (or stereo), and the stream is stereo (or mono). We supported this conversion before.

This needs to be a bit more clever.
Comment 20 Vincent Penquerc'h 2016-02-04 21:31:59 UTC
I didn't realize (or forgot) this was possible without a converter element. I'll update the patch to do something a bit more clever :)
Comment 21 Sebastian Dröge (slomo) 2016-02-04 21:41:39 UTC
Review of attachment 320453 [details] [review]:

::: gst/matroska/matroska-demux.c
@@ +5625,3 @@
+          buf =
+              gst_codec_utils_opus_create_header (samplerate, channels, 0,
+              streams, coupled, NULL, 0, 0);

The header is not mandatory in the caps, just leave this part out here. It's just Ogg stuff
Comment 22 Sebastian Dröge (slomo) 2016-02-04 21:42:24 UTC
Review of attachment 320454 [details] [review]:

Looks good except for one cosmetic thing

::: gst/matroska/matroska-mux.c
@@ +1955,3 @@
+    } else {
+      // no streamheader, but we need to have one, so we make one up
+      // based on caps

No // comments
Comment 23 Vincent Penquerc'h 2016-02-05 09:16:39 UTC
(In reply to Sebastian Dröge (slomo) from comment #19)
> Comment on attachment 320452 [details] [review] [review]
> Fix opus negotiation with mono streams
> 
> This breaks the case where downstream only accepts mono (or stereo), and the
> stream is stereo (or mono). We supported this conversion before.
> 
> This needs to be a bit more clever.

How did this work, and do you have a particular test case for this ?
If I remove the patch, this still does not work:

gst-launch-1.0 audiotestsrc ! audio/x-raw,channels=1 ! opusenc ! opusdec ! audio/x-raw,channels=2 ! autoaudiosink

I want to understand what is/are the case(s) that used work before I modify/test.
Comment 24 Vincent Penquerc'h 2016-02-05 09:22:36 UTC
Created attachment 320488 [details] [review]
make up an OpusHead block from caps
Comment 25 Vincent Penquerc'h 2016-02-05 09:22:59 UTC
Created attachment 320489 [details] [review]
similar fix for matroskademux
Comment 26 Sebastian Dröge (slomo) 2016-02-05 12:24:04 UTC
(In reply to Vincent Penquerc'h from comment #23)

> gst-launch-1.0 audiotestsrc ! audio/x-raw,channels=1 ! opusenc ! opusdec !
> audio/x-raw,channels=2 ! autoaudiosink
> 
> I want to understand what is/are the case(s) that used work before I
> modify/test.

This should've worked at some point. But probably not too important :) You can simplify your code with caps intersection probably btw.
Comment 27 Vincent Penquerc'h 2016-02-05 12:53:22 UTC
Created attachment 320496 [details] [review]
Fix opus negotiation with mono streams
Comment 28 Vincent Penquerc'h 2016-02-05 12:53:54 UTC
Good point, uses intersection now.
I'll make another bug for the auto channel conversion.
Comment 29 Sebastian Dröge (slomo) 2016-02-16 11:51:08 UTC
Comment on attachment 320488 [details] [review]
make up an OpusHead block from caps

Seems ok but the codec_delay is going to be wrong. Ideally you would parse the GstAudioClippingMeta of the first buffers until you received a buffer without pre-skip. But that can be handled in another bug (please open one).
Comment 30 Sebastian Dröge (slomo) 2016-02-16 11:52:00 UTC
Comment on attachment 320496 [details] [review]
Fix opus negotiation with mono streams

This one is handled in another bug, right? Let's get rid of the noise here then :)
Comment 31 Arjen Veenhuizen 2016-02-17 20:14:08 UTC
It appears that this patch no longer solves the problem against GIT master (of today). It applies just fine, like the patches in the linked bug, but I can no longer play the sample file. The pipeline gets stuck in preroll.
Comment 32 Sebastian Dröge (slomo) 2016-02-18 07:46:55 UTC
There shortly was a broken commit in GIT master yesterday that could've caused this. Try again with today :)
Comment 33 Arjen Veenhuizen 2016-02-18 08:21:21 UTC
That did indeed to the trick :). Works against GIT master of today (	3eeca9c7d20bac6f4da1e1100982836539a2daf7)
Comment 34 Nicolas Dufresne (ndufresne) 2016-02-22 20:35:09 UTC
Sorry, my bad.
Comment 35 Sebastian Dröge (slomo) 2016-02-23 08:49:55 UTC
commit 6861d11c49ea0f30d2432cf4ebf6108bc89897f1
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Thu Feb 4 15:59:04 2016 +0000

    matroska-demux: make up an OpusHead block if possible when missing
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761489

commit 565607107f7d19e360faa379a7c19a68d140b1d9
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Thu Feb 4 10:43:15 2016 +0000

    matroska-mux: make up an OpusHead block if possible when missing
    
    This block is needed in the Matroska file, but data coming from
    RTP may not have one.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761489