On 25/04/14 09:48, Martin Storsjö wrote:
> On Fri, 25 Apr 2014, Luca Barbato wrote:
>
>> On 25/04/14 08:12, Martin Storsjö wrote:
>>> On Fri, 25 Apr 2014, Luca Barbato wrote:
>>>
>>>> Makes the two protocols nearly seamless.
>>>> ---
>>>>
>>>> It isn't complete and some options might be mapped in better ways.
>>>>
>>>> libavformat/librtmp.c | 114
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++--
>>>> 1 file changed, 111 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/libavformat/librtmp.c b/libavformat/librtmp.c
>>>> index 7133bd6..1996ba9 100644
>>>> --- a/libavformat/librtmp.c
>>>> +++ b/libavformat/librtmp.c
>>>> @@ -37,7 +37,21 @@ typedef struct LibRTMPContext {
>>>> const AVClass *class;
>>>> RTMP rtmp;
>>>> char *app;
>>>> + char *conn;
>>>> + char *subscribe;
>>>> char *playpath;
>>>> + char *tcurl; ///< url of the target stream
>>>> + char *flashver; ///< version of the flash plugin
>>>> + char *swfhash; ///< SHA256 hash of the
>>>> decompressed SWF file (32 bytes)
>>>> + char *swfurl;
>>>> + char *swfverify;
>>>> + char *pageurl;
>>>> + char *client_buffer_time;
>>>> + int flush_interval;
>>>> + int swfsize;
>>>> + int live;
>>>> + int listen;
>>>> + int listen_timeout;
>>>> } LibRTMPContext;
>>>>
>>>> static void rtmp_log(int level, const char *fmt, va_list args)
>>>> @@ -99,7 +113,40 @@ static int rtmp_open(URLContext *s, const char
>>>> *uri, int flags)
>>>> if (ctx->app || ctx->playpath) {
>>>> int len = strlen(s->filename) + 1;
>>>> if (ctx->app) len += strlen(ctx->app) + sizeof("
>>>> app=");
>>>> - if (ctx->playpath) len += strlen(ctx->playpath) + sizeof("
>>>> playpath=");
>>>> + if (ctx->tcurl) len += strlen(ctx->tcurl) + sizeof("
>>>> tcUrl=");
>>>> + if (ctx->pageurl) len += strlen(ctx->pageurl) + sizeof("
>>>> pageUrl=");
>>>> + if (ctx->swfurl) len += strlen(ctx->swfurl) + sizeof("
>>>> swfUrl=");
>>>> + if (ctx->flashver) len += strlen(ctx->flashver) + sizeof("
>>>> flashver=");
>>>> +
>>>> + if (ctx->conn) {
>>>> + char *sep, *p = ctx->conn;
>>>> + int options = 0;
>>>> +
>>>> + while (p) {
>>>> + options++;
>>>> + p += strspn(p, " ");
>>>> + if (!*p)
>>>> + break;
>>>> + sep = strchr(p, ' ');
>>>> + if (sep)
>>>> + p = sep + 1;
>>>> + else
>>>> + break;
>>>> + }
>>>> + len += options * sizeof(" conn=");
>>>> + len += strlen(ctx->conn);
>>>> + }
>>>> +
>>>> + if (ctx->playpath)
>>>> + len += strlen(ctx->playpath) + sizeof(" playpath=");
>>>> + if (ctx->live)
>>>> + len += sizeof(" live=1");
>>>> + if (ctx->subscribe)
>>>> + len += strlen(ctx->subscribe) + sizeof(" subscribe=");
>>>> +
>>>> + if (ctx->client_buffer_time)
>>>> + len += strlen(ctx->client_buffer_time) + sizeof("
>>>> buffer=");
>>>> +
>>>>
>>>> if (!(filename = av_malloc(len)))
>>>> return AVERROR(ENOMEM);
>>>> @@ -109,10 +156,53 @@ static int rtmp_open(URLContext *s, const char
>>>> *uri, int flags)
>>>> av_strlcat(filename, " app=", len);
>>>> av_strlcat(filename, ctx->app, len);
>>>> }
>>>> + if (ctx->tcurl) {
>>>> + av_strlcat(filename, " tcUrl=", len);
>>>> + av_strlcat(filename, ctx->tcurl, len);
>>>> + }
>>>> + if (ctx->pageurl) {
>>>> + av_strlcat(filename, " pageUrl=", len);
>>>> + av_strlcat(filename, ctx->pageurl, len);
>>>> + }
>>>> + if (ctx->swfurl) {
>>>> + av_strlcat(filename, " swfUrl=", len);
>>>> + av_strlcat(filename, ctx->pageurl, len);
>>>> + }
>>>> + if (ctx->flashver) {
>>>> + av_strlcat(filename, " flashVer=", len);
>>>> + av_strlcat(filename, ctx->flashver, len);
>>>> + }
>>>> + if (ctx->conn) {
>>>> + char *sep, *p = ctx->conn;
>>>> + while (p) {
>>>> + av_strlcat(filename, " conn=", len);
>>>> + p += strspn(p, " ");
>>>> + if (!*p)
>>>> + break;
>>>> + sep = strchr(p, ' ');
>>>> + if (sep)
>>>> + *sep = '\0';
>>>> + av_strlcat(filename, p, len);
>>>> +
>>>> + if (sep)
>>>> + p = sep + 1;
>>>> + }
>>>> + }
>>>
>>> Did you test that librtmp actually can handle multiple conn values
>>> passed this way? I tried to read the source briefly but I'm not sure if
>>> the later options are appended on top of the existing ones, or just
>>> overwrite them.
>>
>> yes and the manpage agrees as well =)
>>
>>>
>>>> +
>>>> if (ctx->playpath) {
>>>> av_strlcat(filename, " playpath=", len);
>>>> av_strlcat(filename, ctx->playpath, len);
>>>> }
>>>> + if (ctx->live)
>>>> + av_strlcat(filename, " live=1", len);
>>>
>>> Can't the live field be -2, -1 or 0, where only the -1 value should set
>>> live=1 here?
>>
>> I mapped -2 -1 to live=1 since live is boolean in librtmp.
>
> Right. This is a change in the defaults for the librtmp wrapping, since
> this would start setting live=1 by default (since the default for
> rtmp_live is -2). This is closer to what we do with the native rtmp
> code, but it's a change in behaviour and should at least be mentioned as
> such in the commit message.
>
>>> Other than that, this looks quite ok, and sure is a useful feature.
>>
>> Are you ok with leaving the other options as unsupported?
>
> Oh, I didn't see those initially. I'd rather not add them to the
> avoptions list unless they're actually supported, otherwise it's just
> confusing.
The problem is that some downstreams expect rtmp to always support the
rtmp options and if a distribution decides to always enable librtmp
everything gets quite annoying for the user.
lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
