GNOME Bugzilla – Bug 669590
[shout2send] support webm streaming
Last modified: 2012-02-15 23:47:59 UTC
As of version 2.3.0, libshout supports sending WebM data to Icecast. This bug is for updating gstreamer to use this support. The shout2send gstreamer element uses the shout_send call, which parses the format it's handed and throttles the upload to the playback clock. As such it has to understand the media type it's being sent. The latest version adds API support to send video/webm data; to make use of this, shout2send needs to advertise that as a input cap, and pass SHOUT_FORMAT_WEBM to the library before sending data.
Created attachment 207009 [details] [review] work in progress This is a work-in-progress patch for initial review. It makes the required changes to gstshout2send but doesn't add minimum version checks to configure.
Created attachment 207032 [details] [review] unconditional webm support This patch updates the configure script to require libshout 2.3 or later, and also changes an indent line to conform with style. Please review.
libshout 2.3 is unreasonably new. Please make it optional.
Created attachment 207039 [details] [review] conditional webm support Patch enabling webm iff the available libshout supports it.
I think the unconditional patch is better: gstreamer generally requires the latest things for developers anyway, the code is cleaner, and it encourages adoption. The conditional patch is provided in case others agree with Michael's review. v^_ on IRC pointed out SHOUT_FORMAT_WEBM is a define in a public header, so if we wanted to rely on that we could use it directly in the #ifdef and avoid the extra configure-time check.
I like Vincent's idea of just doing #ifdef SHOUT_FORMAT_WEBM directly, don't see the point of doing this in configure just to set another define to check for. KISS. It would also be good to avoid #ifdef #else within macros (like GST_STATIC_CAPS), that makes some compilers error out. I would suggest to do #ifdef FOO #define WEBM_CAPS "blah" #else #define WEBM_CAPS "" #endif and then just adding WEBM_CAPS within the GST_STATIC_CAPS.
C bits look fine. No comment on the autotools.
> gstreamer generally requires the latest things for developers anyway Citation needed ;)
Created attachment 207043 [details] [review] conditional webm support Thanks for the review! Here's an updated patch, incorporating suggestions so far.
Pushed, thanks for the patch! commit 8b2ca70124d9bb520d8762e323c6bab01dcc667a Author: Ralph Giles <giles@mozilla.com> Date: Tue Feb 7 14:10:44 2012 -0800 shout2send: send video/webm through libshout. This requires SHOUT_FORMAT_WEBM, added in libshout 2.3.0, so video/webm support is contingent on that symbol being defined. Also an indentation change required by the pre-commit hook. https://bugzilla.gnome.org/show_bug.cgi?id=669590
Thanks, Tim!