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 687483 - [PATCH] problems with unsupported caps; patch introduces audioconvert elements
[PATCH] problems with unsupported caps; patch introduces audioconvert elements
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: GStreamer backend
unspecified
Other Linux
: Normal major
: ---
Assigned To: Maintainer alias for GStreamer component of Totem
Maintainer alias for GStreamer component of Totem
Depends on:
Blocks:
 
 
Reported: 2012-11-02 22:42 UTC by Carlos Rafael Giani
Modified: 2012-11-06 17:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backend: added audio converters (3.29 KB, patch)
2012-11-02 22:42 UTC, Carlos Rafael Giani
needs-work Details | Review
backend: added audio converters (3.32 KB, patch)
2012-11-06 12:33 UTC, Bastien Nocera
committed Details | Review
backend: Plug the audio convert in the right place (1.93 KB, patch)
2012-11-06 13:18 UTC, Bastien Nocera
none Details | Review
backend: Plug the audio convert in the right place (1.93 KB, patch)
2012-11-06 13:22 UTC, Bastien Nocera
committed Details | Review

Description Carlos Rafael Giani 2012-11-02 22:42:11 UTC
Created attachment 227927 [details] [review]
backend: added audio converters

The pitch element supports only F32LE data. In addition, the capsfilter further restricts the range of possible caps. It is possible that the playbin's internal decodebin produces data with incompatible caps. In addition, the case may not support the produced caps either. In any of these cases, Totem will refuse to playback data. One example is an mp3 file, which is decoded with the mad element. mad only produces S32LE samples, which cannot be used by the pitch element. The user then gets a confusing message that a plugin is not installed, even though the mad gstreamer plugin is present.

The solution is to introduce two audioconvert elements, one at the very beginning of the audio bin (before the caps filter), and one right before the autoaudiosink. If conversion is not necessary, audioconvert just passes through buffers without actually touching them.
Comment 1 Tim-Philipp Müller 2012-11-02 23:41:30 UTC
Comment on attachment 227927 [details] [review]
backend: added audio converters

You only need one audioconvert, between pitch/scaletempo and the audio sink, playbin will/should plug another one before internally before the pitch/scaletempo/audiosink bin as well.

Also, the patch needs updating for the fix I just pushed, to use scaletempo for real instead of pitch (supports one more format, but still applies of course).

Would be even nicer to make scaletempo support more formats btw.
Comment 2 Bastien Nocera 2012-11-06 12:33:11 UTC
Created attachment 228238 [details] [review]
backend: added audio converters

This makes the backend more robust in case either playbin's internal
decodebin does not produce supported audio formats, or the audio sink
cannot playback with the produced audio format.

Signed-off-by: Carlos Rafael Giani <dv@pseudoterminal.org>
Comment 3 Bastien Nocera 2012-11-06 12:33:34 UTC
Attachment 228238 [details] pushed as b8f8c66 - backend: added audio converters
Comment 4 Tim-Philipp Müller 2012-11-06 12:44:24 UTC
As I wrote above, the audioconvert needs to be between the scaletempo/pitch and the audiosink.

It does not make sense to put another audioconvert in front of the scaletempo, and does not fix the issue of the audiosink not supporting the formats scaletempo supports.
Comment 5 Tim-Philipp Müller 2012-11-06 12:44:58 UTC
Btw, there's also gst_element_link_many().
Comment 6 Bastien Nocera 2012-11-06 13:18:42 UTC
Created attachment 228242 [details] [review]
backend: Plug the audio convert in the right place

caps_filter ! scaletempo ! convert ! sink
not:
convert ! caps_filter ! scaletempo ! sink
Comment 7 Bastien Nocera 2012-11-06 13:22:29 UTC
Created attachment 228243 [details] [review]
backend: Plug the audio convert in the right place

caps_filter ! scaletempo ! convert ! sink
not:
convert ! caps_filter ! scaletempo ! sink
Comment 8 Tim-Philipp Müller 2012-11-06 13:29:58 UTC
Looks right, thanks!
Comment 9 Bastien Nocera 2012-11-06 17:05:06 UTC
Attachment 228243 [details] pushed as fbed584 - backend: Plug the audio convert in the right place