GNOME Bugzilla – Bug 740195
mpg123: Handle setting caps if the srcpad is not linked yet, making it work with decodebin/playbin
Last modified: 2015-01-11 14:38:56 UTC
Created attachment 290776 [details] [review] mpg123audiodec patch for NULL srccaps In some instances (for example, when using decodebin), the decoder's srcpad won't have caps set by the time gst_mpg123_audio_dec_set_format() is called. In this case, instead of failing, use S16LE as default format. Prior to the git version (that is, 1.4.4 and earlier), it appears that the srcpad caps are always set, even with bins. The problem that is fixed by the patch was observed in the git version only. Nevertheless, it is useful to have such a fallback.
Review of attachment 290776 [details] [review]: ::: ext/mpg123/gstmpg123audiodec.c @@ +511,3 @@ gst_pad_get_allowed_caps (GST_AUDIO_DECODER_SRC_PAD (dec)); + if (allowed_srccaps_unnorm) { + allowed_srccaps = gst_caps_normalize (allowed_srccaps_unnorm); Using gst_caps_normalize() is a really bad idea usually. It creates huge caps, instead use the other caps/structure API :) @@ +613,3 @@ + + GST_DEBUG_OBJECT (dec, "srcpad has no caps set - using default format %s", + format_str); It is rather that downstream is not linked yet and thus you can't ask for the allowed caps. Nothing except for mpg123 itself is setting the caps on the srcpad. What other code is usually doing if get_allowed_caps() returns NULL is to just use the srcpad template caps instead and go through the same code path otherwise. This prevents some code duplication and bigger special casing.
Created attachment 293635 [details] [review] set_format rework which addresses the aforementioned issues
Comment on attachment 293635 [details] [review] set_format rework which addresses the aforementioned issues gstmpg123audiodec.c: In function 'gst_mpg123_audio_dec_set_format': gstmpg123audiodec.c:566:9: error: 'encoding' may be used uninitialized in this function [-Werror=maybe-uninitialized] err = ^
commit 9aaf12aae08e7b1d02ec0b0bc365e87a3af67a83 Author: Tim-Philipp Müller <tim@centricular.com> Date: Sun Jan 11 01:08:08 2015 +0000 mpg123: fix compiler warning and simplify checks in set_caps https://bugzilla.gnome.org/show_bug.cgi?id=740195 commit ece6426810f9f0fe3173c09e6fab1647c655d77d Author: Carlos Rafael Giani <dv@pseudoterminal.org> Date: Sat Jan 3 13:06:45 2015 +0100 mpg123: rework set_format code so mpg123audiodec works with decodebin/playbin The old code was using gst_caps_normalize() and was generally overly complex. Simplify by picking sample rate and number of channels from upstream and the sample format from the allowed caps. If the format caps is a list of strins, just pick the first one. And if the srcpad isn't linked yet, use the default format (S16). https://bugzilla.gnome.org/show_bug.cgi?id=740195
*** Bug 742730 has been marked as a duplicate of this bug. ***