GNOME Bugzilla – Bug 733148
audio encoder/decoder base classes needlessly delay caps events
Last modified: 2018-11-03 11:30:37 UTC
+++ This bug was initially created as a clone of Bug #733147 +++ The encoder base classes delay caps events currently. This was added as part of bug #680614. Reason is that a few encoders need to wait until they get other sticky events (tags!) before they can negotiate... as the tags are part of the caps (vorbis streamheaders, etc). However doing this delaying unconditionally for everything in the base class seems wrong and causes problems (Nicolas has details). If anything the subclasses have to delay, as only the subclasses really know how long to delay. Currently we always delay until the first buffer but that might be too late or too early! I would like to change this for 1.6, even if it is an ABI change. To make that possible we will have to fix all subclasses though.
This was fixed for videoencoder now, but for consistency we should do it for audioencoder too. Nicolas? Also this change now breaks vorbiscomments in Theora, VP8 and Daala (and would break them in Vorbis, FLAC, Speex, Opus). commit ce50fc221e8a795d7cfedf471d239d4a9d3bf824 Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Tue Jul 8 16:59:37 2014 -0400 videoencoder: Don't delay set_format This prevent implementing allocation query, as the format need to be known in order to determin the size and number of buffers needed. Note: This may lead to few regressions that will need fixing https://bugzilla.gnome.org/show_bug.cgi?id=732288
Oh, I thought you had done all the others while doing the videodecoder. So I supposed audio decoder and audio encoder are the left over. As we know these will cause regression, it's slightly more work I suppose. I have tested all the video encoder I could and found no side effect for this change (most encoder don't currently negotiate the allocation).
For the reference: commit 368d75fe75d0043e0cf90dffdac71fc3b75c28a1 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Jul 10 12:39:46 2014 +0200 audiodecoder: Handle CAPS events immediately instead of delaying them https://bugzilla.gnome.org/show_bug.cgi?id=733147 commit 11ef208736c5867046156e0bb6786f2eb56e6776 Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Jul 11 21:51:05 2014 +0200 videodecoder: Handle CAPS events immediately instead of delaying them https://bugzilla.gnome.org/show_bug.cgi?id=73314 Only the audio encoder remains to completely revert: commit 7b135e8810ab08401d86ea4ba184ecfeefb8c4ce Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Thu Jul 26 14:28:26 2012 +0200 video{de,en}coder: delay input caps processing until processing data Fixes https://bugzilla.gnome.org/show_bug.cgi?id=680614 commit c91615bd82542c0f53046ec6c6d77b5344133ced Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Thu Jul 26 14:27:38 2012 +0200 audio{de,en}coder: delay input caps processing until processing data Fixes https://bugzilla.gnome.org/show_bug.cgi?id=680614
(In reply to comment #1) > Also this change now breaks vorbiscomments in Theora, VP8 and Daala (and would > break them in Vorbis, FLAC, Speex, Opus). Can you explain a little bit how the "vorbiscomments" are used and why this is broken. I'd like to add a unit test for that.
The problem basically is that e.g. vorbisenc needs events after the CAPS event to produce its srcpad caps. In Vorbis we have these stream-headers in the caps, which contain the Vorbis header packets. These contain, among other things, vorbiscomments. To produce vorbiscomments you need the TAGS events that happen somewhere between the SEGMENT event and the first buffer. Before this change set_format() was only called on vorbisenc when the first buffer arrived. At that time the tags were known already. Now set_format() is called when the CAPS event arrived, which is before the tags are known. So we never write tags into Vorbis streams now. That's relatively easy to fix in the encoder subclasses though. They can just delay the downstream part of the negotiation and not immediately do it in set_format().
(In reply to comment #5) > That's relatively easy to fix in the encoder subclasses though. They can just > delay the downstream part of the negotiation and not immediately do it in > set_format(). That seems like a good solution. It would still be nice if you could share a way, code or speudo code, that let me reproduce the issue. We really need a unit test for this.
-- 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-base/issues/125.