[PATCH] Make rtmpdump handle full RTMP URLs with "-i"
or --url. The library already can handle those, so it makes debugging easier for users of the library. --- rtmpdump.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/rtmpdump.c b/rtmpdump.c index 34bfdba..7827eae 100644 --- a/rtmpdump.c +++ b/rtmpdump.c @@ -638,6 +638,8 @@ void usage(char *prog) ("\n%s: This program dumps the media content streamed over RTMP.\n\n", prog); RTMP_LogPrintf("--help|-h Prints this help screen.\n"); RTMP_LogPrintf + ("--url|-i url URL with options included (e.g. rtmp://host[:port]/path swfUrl=url tcUrl=url)\n"); + RTMP_LogPrintf ("--rtmp|-r url URL (e.g. rtmp://host[:port]/path)\n"); RTMP_LogPrintf ("--host|-n hostname Overrides the hostname in the rtmp url\n"); @@ -755,6 +757,7 @@ main(int argc, char **argv) uint32_t dStopOffset = 0; RTMP rtmp = { 0 }; + AVal fullUrl = { 0, 0 }; AVal swfUrl = { 0, 0 }; AVal tcUrl = { 0, 0 }; AVal pageUrl = { 0, 0 }; @@ -817,6 +820,7 @@ main(int argc, char **argv) {"protocol", 1, NULL, 'l'}, {"playpath", 1, NULL, 'y'}, {"playlist", 0, NULL, 'Y'}, + {"url", 1, NULL, 'i'}, {"rtmp", 1, NULL, 'r'}, {"swfUrl", 1, NULL, 's'}, {"tcUrl", 1, NULL, 't'}, @@ -851,7 +855,7 @@ main(int argc, char **argv) while ((opt = getopt_long(argc, argv, - "hVveqzr:s:t:p:a:b:f:o:u:C:n:c:l:y:Ym:k:d:A:B:T:w:x:W:X:S:#j:", + "hVveqzr:s:t:i:p:a:b:f:o:u:C:n:c:l:y:Ym:k:d:A:B:T:w:x:W:X:S:#j:", longopts, NULL)) != -1) { switch (opt) @@ -991,6 +995,9 @@ main(int argc, char **argv) } break; } + case 'i': + STR2AVAL(fullUrl, optarg); + break; case 's': STR2AVAL(swfUrl, optarg); break; @@ -1069,32 +1076,32 @@ main(int argc, char **argv) } } - if (!hostname.av_len) + if (!hostname.av_len && !fullUrl.av_len) { RTMP_Log(RTMP_LOGERROR, "You must specify a hostname (--host) or url (-r \"rtmp://host[:port]/playpath\") containing a hostname"); return RD_FAILED; } - if (playpath.av_len == 0) + if (playpath.av_len == 0 && !fullUrl.av_len) { RTMP_Log(RTMP_LOGERROR, "You must specify a playpath (--playpath) or url (-r \"rtmp://host[:port]/playpath\") containing a playpath"); return RD_FAILED; } - if (protocol == RTMP_PROTOCOL_UNDEFINED) + if (protocol == RTMP_PROTOCOL_UNDEFINED && !fullUrl.av_len) { RTMP_Log(RTMP_LOGWARNING, "You haven't specified a protocol (--protocol) or rtmp url (-r), using default protocol RTMP"); protocol = RTMP_PROTOCOL_RTMP; } - if (port == -1) + if (port == -1 && !fullUrl.av_len) { RTMP_Log(RTMP_LOGWARNING, "You haven't specified a port (--port) or rtmp url (-r), using default port 1935"); port = 0; } - if (port == 0) + if (port == 0 && !fullUrl.av_len) { if (protocol & RTMP_FEATURE_SSL) port = 443; @@ -1176,9 +1183,20 @@ main(int argc, char **argv) } } - RTMP_SetupStream(&rtmp, protocol, &hostname, port, &sockshost, &playpath, - &tcUrl, &swfUrl, &pageUrl, &app, &auth, &swfHash, swfSize, - &flashVer, &subscribepath, &usherToken, dSeek, dStopOffset, bLiveStream, timeout); + if (!fullUrl.av_len) + { + RTMP_SetupStream(&rtmp, protocol, &hostname, port, &sockshost, &playpath, + &tcUrl, &swfUrl, &pageUrl, &app, &auth, &swfHash, swfSize, + &flashVer, &subscribepath, &usherToken, dSeek, dStopOffset, bLiveStream, timeout); + } + else + { + if (RTMP_SetupURL(&rtmp, fullUrl.av_val) == FALSE) + { + RTMP_Log(RTMP_LOGERROR, "Couldn't parse URL: %s", fullUrl.av_val); + return RD_FAILED; + } + } /* Try to keep the stream moving if it pauses on us */ if (!bLiveStream && !(protocol & RTMP_FEATURE_HTTP)) -- 1.7.10.2
On Jul 18, 2012 Bastien Nocera wrote
Make rtmpdump handle full RTMP URLs with "-i" or --url. The library already can handle those, so it makes debugging easier for users of the library.
This is redundant code. RtmpDump already supports these options via --protocol --host --port --app --swfUrl --tcUrl Additionally, in many cases tcUrl is assumed based on the app/playpath. I say if you want to see this pass you will bring some more discussion to the table than "Short sentence commit message".
On Wed, 2012-07-18 at 17:14 -0500, Steven Penny wrote:
On Jul 18, 2012 Bastien Nocera wrote
Make rtmpdump handle full RTMP URLs with "-i" or --url. The library already can handle those, so it makes debugging easier for users of the library.
This is redundant code. RtmpDump already supports these options via
--protocol --host --port --app --swfUrl --tcUrl
Additionally, in many cases tcUrl is assumed based on the app/playpath. I say if you want to see this pass you will bring some more discussion to the table than "Short sentence commit message".
The library uses the parsed URL, but the command-line application can't? Seems weird. I'm writing code that generates those sort of URLs, which are understood by applications like Totem, VLC, XBMC, or mplayer (through librtmp). It makes complete sense that the low-level utility that's rtmpdump supports it too.
On Wed, 18 Jul 2012 23:27:44 +0100, Bastien Nocera wrote:
On Wed, 2012-07-18 at 17:14 -0500, Steven Penny wrote:
On Jul 18, 2012 Bastien Nocera wrote
Make rtmpdump handle full RTMP URLs with "-i" or --url. The library already can handle those, so it makes debugging easier for users of the library.
This is redundant code. RtmpDump already supports these options via
--protocol --host --port --app --swfUrl --tcUrl
Additionally, in many cases tcUrl is assumed based on the app/playpath. I say if you want to see this pass you will bring some more discussion to the table than "Short sentence commit message".
The library uses the parsed URL, but the command-line application can't? Seems weird.
I'm writing code that generates those sort of URLs, which are understood by applications like Totem, VLC, XBMC, or mplayer (through librtmp). It makes complete sense that the low-level utility that's rtmpdump supports it too.
is there anyway you could move the 'fullurl' parsing code from librtmp to a common file that rtmpdump could also be linked to, so there wouldnt be code duplication? (if there is code duplication, anyways, i'm not paying attention here) -compn
Bastien Nocera wrote:
On Wed, 2012-07-18 at 17:14 -0500, Steven Penny wrote:
On Jul 18, 2012 Bastien Nocera wrote
Make rtmpdump handle full RTMP URLs with "-i" or --url. The library already can handle those, so it makes debugging easier for users of the library.
This is redundant code. RtmpDump already supports these options via
--protocol --host --port --app --swfUrl --tcUrl
Additionally, in many cases tcUrl is assumed based on the app/playpath. I say if you want to see this pass you will bring some more discussion to the table than "Short sentence commit message".
The library uses the parsed URL, but the command-line application can't? Seems weird.
Just a historical cruft. The command line tools were meant to be migrated to the new parser a long time ago. It would be nice to adapt rtmpgw too if you get a chance.
I'm writing code that generates those sort of URLs, which are understood by applications like Totem, VLC, XBMC, or mplayer (through librtmp). It makes complete sense that the low-level utility that's rtmpdump supports it too.
On Thu, 2012-07-19 at 02:01 -0700, Howard Chu wrote:
Bastien Nocera wrote:
On Wed, 2012-07-18 at 17:14 -0500, Steven Penny wrote:
On Jul 18, 2012 Bastien Nocera wrote
Make rtmpdump handle full RTMP URLs with "-i" or --url. The library already can handle those, so it makes debugging easier for users of the library.
This is redundant code. RtmpDump already supports these options via
--protocol --host --port --app --swfUrl --tcUrl
Additionally, in many cases tcUrl is assumed based on the app/playpath. I say if you want to see this pass you will bring some more discussion to the table than "Short sentence commit message".
The library uses the parsed URL, but the command-line application can't? Seems weird.
Just a historical cruft. The command line tools were meant to be migrated to the new parser a long time ago.
Right, it was mainly to reply to Steve's opinion that those were already supported.
It would be nice to adapt rtmpgw too if you get a chance.
If that'll quicken pushing the patch upstream, certainly.
I'm writing code that generates those sort of URLs, which are understood by applications like Totem, VLC, XBMC, or mplayer (through librtmp). It makes complete sense that the low-level utility that's rtmpdump supports it too.
Bastien Nocera wrote:
or --url. The library already can handle those, so it makes debugging easier for users of the library.
Thanks for the patch. I've been meaning to change rtmpdump to use the new format for quite a long time, but never got around to doing the change. This saves me the trouble. As anyone who's been following this list since the beginning knows, the original rtmpdump option parsing was deprecated and meant to be removed when the new format was introduced.
--- rtmpdump.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/rtmpdump.c b/rtmpdump.c index 34bfdba..7827eae 100644 --- a/rtmpdump.c +++ b/rtmpdump.c @@ -638,6 +638,8 @@ void usage(char *prog) ("\n%s: This program dumps the media content streamed over RTMP.\n\n", prog); RTMP_LogPrintf("--help|-h Prints this help screen.\n"); RTMP_LogPrintf + ("--url|-i url URL with options included (e.g. rtmp://host[:port]/path swfUrl=url tcUrl=url)\n"); + RTMP_LogPrintf ("--rtmp|-r url URL (e.g. rtmp://host[:port]/path)\n"); RTMP_LogPrintf ("--host|-n hostname Overrides the hostname in the rtmp url\n"); @@ -755,6 +757,7 @@ main(int argc, char **argv) uint32_t dStopOffset = 0; RTMP rtmp = { 0 };
+ AVal fullUrl = { 0, 0 }; AVal swfUrl = { 0, 0 }; AVal tcUrl = { 0, 0 }; AVal pageUrl = { 0, 0 }; @@ -817,6 +820,7 @@ main(int argc, char **argv) {"protocol", 1, NULL, 'l'}, {"playpath", 1, NULL, 'y'}, {"playlist", 0, NULL, 'Y'}, + {"url", 1, NULL, 'i'}, {"rtmp", 1, NULL, 'r'}, {"swfUrl", 1, NULL, 's'}, {"tcUrl", 1, NULL, 't'}, @@ -851,7 +855,7 @@ main(int argc, char **argv)
while ((opt = getopt_long(argc, argv, - "hVveqzr:s:t:p:a:b:f:o:u:C:n:c:l:y:Ym:k:d:A:B:T:w:x:W:X:S:#j:", + "hVveqzr:s:t:i:p:a:b:f:o:u:C:n:c:l:y:Ym:k:d:A:B:T:w:x:W:X:S:#j:", longopts, NULL)) != -1) { switch (opt) @@ -991,6 +995,9 @@ main(int argc, char **argv) } break; } + case 'i': + STR2AVAL(fullUrl, optarg); + break; case 's': STR2AVAL(swfUrl, optarg); break; @@ -1069,32 +1076,32 @@ main(int argc, char **argv) } }
- if (!hostname.av_len) + if (!hostname.av_len && !fullUrl.av_len) { RTMP_Log(RTMP_LOGERROR, "You must specify a hostname (--host) or url (-r \"rtmp://host[:port]/playpath\") containing a hostname"); return RD_FAILED; } - if (playpath.av_len == 0) + if (playpath.av_len == 0 && !fullUrl.av_len) { RTMP_Log(RTMP_LOGERROR, "You must specify a playpath (--playpath) or url (-r \"rtmp://host[:port]/playpath\") containing a playpath"); return RD_FAILED; }
- if (protocol == RTMP_PROTOCOL_UNDEFINED) + if (protocol == RTMP_PROTOCOL_UNDEFINED && !fullUrl.av_len) { RTMP_Log(RTMP_LOGWARNING, "You haven't specified a protocol (--protocol) or rtmp url (-r), using default protocol RTMP"); protocol = RTMP_PROTOCOL_RTMP; } - if (port == -1) + if (port == -1 && !fullUrl.av_len) { RTMP_Log(RTMP_LOGWARNING, "You haven't specified a port (--port) or rtmp url (-r), using default port 1935"); port = 0; } - if (port == 0) + if (port == 0 && !fullUrl.av_len) { if (protocol & RTMP_FEATURE_SSL) port = 443; @@ -1176,9 +1183,20 @@ main(int argc, char **argv) } }
- RTMP_SetupStream(&rtmp, protocol, &hostname, port, &sockshost, &playpath, - &tcUrl, &swfUrl, &pageUrl, &app, &auth, &swfHash, swfSize, - &flashVer, &subscribepath, &usherToken, dSeek, dStopOffset, bLiveStream, timeout); + if (!fullUrl.av_len) + { + RTMP_SetupStream(&rtmp, protocol, &hostname, port, &sockshost, &playpath, + &tcUrl, &swfUrl, &pageUrl, &app, &auth, &swfHash, swfSize, + &flashVer, &subscribepath, &usherToken, dSeek, dStopOffset, bLiveStream, timeout); + } + else + { + if (RTMP_SetupURL(&rtmp, fullUrl.av_val) == FALSE) + { + RTMP_Log(RTMP_LOGERROR, "Couldn't parse URL: %s", fullUrl.av_val); + return RD_FAILED; + } + }
/* Try to keep the stream moving if it pauses on us */ if (!bLiveStream && !(protocol & RTMP_FEATURE_HTTP))
On Thu, 19 Jul 2012 01:54:11 -0700, Howard Chu wrote:
Bastien Nocera wrote:
or --url. The library already can handle those, so it makes debugging easier for users of the library.
Thanks for the patch. I've been meaning to change rtmpdump to use the new format for quite a long time, but never got around to doing the change. This saves me the trouble.
As anyone who's been following this list since the beginning knows, the original rtmpdump option parsing was deprecated and meant to be removed when the new format was introduced.
did you want someone to commit this ? -compn
compn wrote:
On Thu, 19 Jul 2012 01:54:11 -0700, Howard Chu wrote:
Bastien Nocera wrote:
or --url. The library already can handle those, so it makes debugging easier for users of the library.
Thanks for the patch. I've been meaning to change rtmpdump to use the new format for quite a long time, but never got around to doing the change. This saves me the trouble.
As anyone who's been following this list since the beginning knows, the original rtmpdump option parsing was deprecated and meant to be removed when the new format was introduced.
did you want someone to commit this ?
-compn
Go ahead, thanks.
participants (4)
-
Bastien Nocera -
compn -
Howard Chu -
Steven Penny