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 669590 - [shout2send] support webm streaming
[shout2send] support webm streaming
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal enhancement
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-02-07 18:19 UTC by giles
Modified: 2012-02-15 23:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
work in progress (1.47 KB, patch)
2012-02-07 18:22 UTC, giles
none Details | Review
unconditional webm support (2.93 KB, patch)
2012-02-07 22:15 UTC, giles
none Details | Review
conditional webm support (3.66 KB, patch)
2012-02-07 23:43 UTC, giles
none Details | Review
conditional webm support (3.25 KB, patch)
2012-02-08 00:31 UTC, giles
committed Details | Review

Description giles 2012-02-07 18:19:13 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.
Comment 1 giles 2012-02-07 18:22:37 UTC
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.
Comment 2 giles 2012-02-07 22:15:36 UTC
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.
Comment 3 Michael Smith 2012-02-07 22:25:26 UTC
libshout 2.3 is unreasonably new. Please make it optional.
Comment 4 giles 2012-02-07 23:43:36 UTC
Created attachment 207039 [details] [review]
conditional webm support

Patch enabling webm iff the available libshout supports it.
Comment 5 giles 2012-02-07 23:54:59 UTC
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.
Comment 6 Tim-Philipp Müller 2012-02-07 23:58:49 UTC
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.
Comment 7 Michael Smith 2012-02-08 00:00:53 UTC
C bits look fine. No comment on the autotools.
Comment 8 Tim-Philipp Müller 2012-02-08 00:01:33 UTC
> gstreamer generally requires the latest things for developers anyway

Citation needed ;)
Comment 9 giles 2012-02-08 00:31:12 UTC
Created attachment 207043 [details] [review]
conditional webm support

Thanks for the review! Here's an updated patch, incorporating suggestions so far.
Comment 10 Tim-Philipp Müller 2012-02-08 10:51:54 UTC
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
Comment 11 giles 2012-02-08 16:23:55 UTC
Thanks, Tim!