GNOME Bugzilla – Bug 755481
speex: handle streamheaders in caps/buffers better
Last modified: 2018-11-03 15:04:38 UTC
Created attachment 311961 [details] [review] tests and fix The tests describe this better then I can :)
It is worth mentioning that with this patch and #754226... gst-launch audiotestsrc is-live=1 samplesperbuffer=320 ! audio/x-raw,format=S16LE,rate=16000 ! speexenc ! flvmux ! flvdemux ! speexdec ! fakesink ...now finally works!
Comment on attachment 311961 [details] [review] tests and fix Looks good but does not apply without the other patch
Created attachment 334055 [details] [review] patch that doesn't depend on other bug Here's a patch for this that doesn't depend on the other bug. > The tests describe this better then I can :) Please could you describe *how* this improves the streambuffer handling exactly, so people don't have to "reverse engineer" it from your patch and the tests? :)
(In reply to Tim-Philipp Müller from comment #3) > Created attachment 334055 [details] [review] [review] > patch that doesn't depend on other bug > > Here's a patch for this that doesn't depend on the other bug. > > > The tests describe this better then I can :) > > Please could you describe *how* this improves the streambuffer handling > exactly, so people don't have to "reverse engineer" it from your patch and > the tests? :) There are 4 tests: test_headers_in_caps: Checks that the decoder can decode the output from speexenc, where the streamheaders are negotiated both through caps and as buffers, and in that case speexdec will use the caps. test_headers_in_buffers: Checks that the decoder can decode the output from speexenc, where the caps is *not* available, so that the streamheaders has to be negotiated through the buffers. test_headers_not_in_buffers: Checks that the decoder can decode the output from speexenc where we remove the first two streamheader-buffers, so that negotiation has to rely purly on caps. test_headers_from_flv: Check that the decoder can use the streamheaders coming from flxdemux, and still decode the buffer. So a thorough checks of speex and its handling of streamheaders, coming from both caps and/or buffers.
Cool, thanks. Do you happen to remember which of them fail without the patch? :) All of them?
(In reply to Tim-Philipp Müller from comment #5) > Cool, thanks. Do you happen to remember which of them fail without the > patch? :) All of them? No, but that should be easy to try out right? I am guessing the first would pass and the rest would fail.
Without the patch the first three tests - test_headers_in_caps - test_headers_in_buffers - test_headers_not_in_buffers pass as well, but this one fails: - test_headers_from_flv So what's happening in the FLV case? How are the streamheaders special/different here?
(In reply to Tim-Philipp Müller from comment #7) > Without the patch the first three tests > > - test_headers_in_caps > - test_headers_in_buffers > - test_headers_not_in_buffers > > pass as well, but this one fails: > That is curious. If you look at logging from *speex*:6, is that anything that stands out, maybe something I am not checking well enough in the test (perhaps GstFlowReturn or something?) I would at least expect https://bugzilla.gnome.org/attachment.cgi?id=334055&action=diff#a/ext/speex/gstspeexdec.c_sec4 (with GST_FLOW_ERROR) to be hit by one of the tests. > - test_headers_from_flv > > So what's happening in the FLV case? How are the streamheaders > special/different here? I don't remember, sorry. Maybe try the launch-line from Comment 2 (gst-launch audiotestsrc is-live=1 samplesperbuffer=320 ! audio/x-raw,format=S16LE,rate=16000 ! speexenc ! flvmux ! flvdemux ! speexdec ! fakesink) and try that with some logging?
The test should probably use: fail_unless_equals_int (GST_FLOW_OK, gst_harness_src_crank_and_push_many (... everywhere where gst_harness_src_crank_and_push_many is used. Maybe that will generate a few more failures?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/225.