Re: Pecl request for new extension
| From: | Patrick Allaert | Date: | Fri, 30 Mar 2012 21:36:27 +0000 |
| Subject: | Re: Pecl request for new extension | ||
| References: | 1 | Groups: | php.pecl.dev |
| Request: | Send a blank email to pecl-dev+get-9272@lists.php.net to get a copy of this message | ||
2012/3/30 Joshua Thijssen <jthijssen@noxlogic.nl>:
> Hi all,
>
> For your consideration, I'd like to request adoption of a new extension
> into pecl: shout.
>
> This is pretty much a simple (yet useful) wrapper around libshout3
> library, which allows you to stream mp3 and/or ogg/vorbis data to
> icecast or shoutcast streaming servers. The library takes care of data
> transfers, synchronization and allows you to set stream and metadata
> (like the currently streamed song for instance).
>
> This is my first (real) extension and I've blogged about how to write
> the extension in details:
>
>
> http://www.adayinthelifeof.nl/2012/03/24/phpshout-a-shoutcast-streamer-in-php-part-1/
>
> http://www.adayinthelifeof.nl/2012/03/24/phpshout-a-shoutcast-streamer-in-php-part-2/
> ]{óXt0í7Dèû
> Ȗhttp://www.adayinthelifeof.nl/2012/03/24/phpshout-a-shoutcast-streamer-in-php-part-3/
>
> http://www.adayinthelifeof.nl/2012/03/24/phpshout-a-shoutcast-streamer-in-php-part-4/
>
> The libshout3 library is LGPLv2, which I confirmed on #efnet php.pecl is
> not ideal, but permittable. There are 2 .c files in libshout that do not
> have correct license headers, but this is currently issued as a task
> (https://trac.xiph.org/ticket/1867). All other source files looks
> correct, so is the COPYRIGHT and README file which are all refering to
> the LGPLv2.
>
> Furthermore, the libshout uses libogg and libvorbis, both are BSD-style
> licensed and should be compatible.
>
> The extension code can be found on github:
> https://github.com/jaytaph/phpshout
>
>
>
> Regards,
> Joshua
Hi Joshua,
I think you are missing a few stuff regarding thread safety (not that
I care too much, but still...).
In you function declaration (php_shout_get_handler_string(),
php_shout_get_handler_long(), php_shout_get_handler_ulong(), ...),
none of those are using the TSRMLS_DC macro.
example:
static void
php_shout_get_handler_string(INTERNAL_FUNCTION_PARAMETERS, const char
*(*func)(shout_t *))
should become:
static void
php_shout_get_handler_string(INTERNAL_FUNCTION_PARAMETERS, const char
*(*func)(shout_t *) TSRMLS_DC)
which will give the additional void ***tsrm_ls parameter to you
function when building with ZTS.
When calling such method, you have to use an additional macro:
php_shout_get_handler_string(INTERNAL_FUNCTION_PARAM_PASSTHRU,
shout_get_host);
should be:
php_shout_get_handler_string(INTERNAL_FUNCTION_PARAM_PASSTHRU,
shout_get_host TSRMLS_CC);
so that you actually pass the tsrm_ls parameter.
Since all those macros evaluate to nothing in non ZTS mode, you are
probably fine in that condition, however expect to have compiling
failure if ZTS is activated.
An interesting read on that subject is the blog entry of Sara Golemon
at: http://blog.golemon.com/2006/06/what-heck-is-tsrmlscc-anyway.html
Cheers,
Patrick