On Thu, 9 Aug 2012, Jordi Ortiz wrote:
---
doc/protocols.texi | 5 +
libavcodec/version.h | 2 +-
libavformat/rtmp.h | 1 +
libavformat/rtmpproto.c | 475 ++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 476 insertions(+), 7 deletions(-)
Receiving data with this patch crashes for me. Please test it with
valgrind (you probably want to receive with "avconv -rtmp_listen 1 -i
<url> -f null -" instead of avplay, to avoid valgrind warnings related to
SDL).
diff --git a/doc/protocols.texi b/doc/protocols.texi
index bf67d89..e7f1d77 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -188,6 +188,11 @@ application specified in @var{app}, may be prefixed by
"mp4:". You
can override the value parsed from the URI through the @code{rtmp_playpath}
option, too.
+@item listen
+Act as a server, listening for an incoming connection.
+
+@item timeout
+Maximum time to wait for the incoming connection. Implies listen.
@end table
Additionally, the following parameters can be set via command line options
diff --git a/libavcodec/version.h b/libavcodec/version.h
index a57369a..8310a79 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -27,7 +27,7 @@
*/
#define LIBAVCODEC_VERSION_MAJOR 54
-#define LIBAVCODEC_VERSION_MINOR 25
+#define LIBAVCODEC_VERSION_MINOR 26
#define LIBAVCODEC_VERSION_MICRO 0
#define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
Since the changed/added functionality is in libavformat, bump the
libavformat version, not libavcodec. (Also, bumping libavcodec version
causes rebuilding both libavcodec and libavformat, while bumping
libavformat version only causes a rebuild of libavformat, not libavcodec.)
diff --git a/libavformat/rtmp.h b/libavformat/rtmp.h
index c9aa67e..5972640 100644
--- a/libavformat/rtmp.h
+++ b/libavformat/rtmp.h
@@ -28,6 +28,7 @@
#define RTMPS_DEFAULT_PORT 443
#define RTMP_HANDSHAKE_PACKET_SIZE 1536
+#define RTMP_HANDSHAKE_PACKET_ARRAY_SIZE RTMP_HANDSHAKE_PACKET_SIZE - 8
This is better than before, but I still don't like it much - I'd rather
have you just buffer the full packet. The logic about which bits of the
packet to compare/checksum/whatever is then kept within the function that
checks that "does this packet match the other one", without knowing
exactly which parts it actually checks. Now the handshake comparison logic
is not concentrated to one spot but spread out all over the code. What if
we later choose to only compare and checksum some small parts (e.g. as
part of supporting server side RTMPE handshake)?
In other words - this constant isn't something that is directly related to
permanent protocol constants, it's an implementation detail that can
change.
#define HMAC_IPAD_VAL 0x36
#define HMAC_OPAD_VAL 0x5C
diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index 6ad90b8..7ee938f 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -29,6 +29,7 @@
#include "libavutil/intfloat.h"
#include "libavutil/lfg.h"
#include "libavutil/opt.h"
+#include "libavutil/random_seed.h"
#include "libavutil/sha.h"
#include "avformat.h"
#include "internal.h"
@@ -47,6 +48,7 @@
#define PLAYPATH_MAX_LENGTH 256
#define TCURL_MAX_LENGTH 512
#define FLASHVER_MAX_LENGTH 64
+#define RTMP_PKTDATA_DEFAULT_SIZE 4096
/** RTMP protocol handler state */
typedef enum {
@@ -100,6 +102,8 @@ typedef struct RTMPContext {
TrackedMethod*tracked_methods; ///< tracked methods buffer
int nb_tracked_methods; ///< number of tracked methods
int tracked_methods_size; ///< size of the tracked methods
buffer
+ int listen; ///< listen mode flag
+ int listen_timeout; ///< listen timeout to wait for
new connections
} RTMPContext;
#define PLAYER_KEY_OPEN_PART_LEN 30 ///< length of partial key used for first
client digest signing
@@ -342,6 +346,141 @@ static int gen_connect(URLContext *s, RTMPContext *rt)
return rtmp_send_packet(rt, &pkt, 1);
}
+static int read_connect(URLContext *s, RTMPContext *rt)
+{
+ RTMPPacket pkt = { 0 };
+ uint8_t *p;
+ const uint8_t *cp;
+ int ret;
+ char command[64];
+ int stringlen;
+ double seqnum;
+ GetByteContext gbc;
+
+ if ((ret = ff_rtmp_packet_read(rt->stream, &pkt, rt->chunk_size,
+ rt->prev_pkt[1])) < 0)
+ return ret;
+ cp = pkt.data;
+ bytestream2_init(&gbc, cp, pkt.data_size);
+ if (ff_amf_read_string(&gbc, command, sizeof(command), &stringlen)) {
+ av_log(s, AV_LOG_ERROR, "Unable to read command string\n");
+ return AVERROR_INVALIDDATA;
Here you leak the rtmp packet that you free below with
ff_rtmp_packet_destroy.
+ }
+ if (strcmp(command, "connect")) {
+ av_log(s, AV_LOG_ERROR, "Expecting connect\n");
Here you could return the actual command as well, like "Expecting connect,
got %s".
+ return AVERROR_INVALIDDATA;
+ }
+ ret = ff_amf_read_number(&gbc, &seqnum);
+ if (ret)
+ av_log(s, AV_LOG_WARNING, "SeqNum not found\n");
+ /* Here one could parse an AMF Object with data as flashVers and others.
+ * Not needed by now */
You could however, parse it enough to find the requested app name, and
compare it to the one that the caller specified (just like for playpath,
and perhaps log a warning if it didn't match what we expected).
+ ff_rtmp_packet_destroy(&pkt);
+
+ // Send Window Acknowledgement Size (as defined in speficication)
+ if ((ret = ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL,
+ RTMP_PT_SERVER_BW, 0, 4)) < 0)
+ return ret;
+ p = pkt.data;
+ bytestream_put_be32(&p, rt->server_bw);
+ pkt.data_size = p - pkt.data;
+ ret = ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size,
+ rt->prev_pkt[1]);
+ ff_rtmp_packet_destroy(&pkt);
+ if (ret < 0)
+ return ret;
+ // Send Peer Bandwidth
+ if ((ret = ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL,
+ RTMP_PT_CLIENT_BW, 0, 5)) < 0)
+ return ret;
+ p = pkt.data;
+ bytestream_put_be32(&p, rt->server_bw);
+ bytestream_put_byte(&p, 2); // dynamic
+ pkt.data_size = p - pkt.data;
+ ret = ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size,
+ rt->prev_pkt[1]);
+ ff_rtmp_packet_destroy(&pkt);
+ if (ret < 0)
+ return ret;
+
+ // Ping request
+ if ((ret = ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL,
+ RTMP_PT_PING, 0, 6)) < 0)
+ return ret;
+
+ p = pkt.data;
+ bytestream_put_be16(&p, 0); // 0 -> Stream Begin
+ bytestream_put_be32(&p, 0);
+ ret = ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size,
+ rt->prev_pkt[1]);
+ ff_rtmp_packet_destroy(&pkt);
+ if (ret < 0)
+ return ret;
+
+ // Chunk size
+ if ((ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL,
+ RTMP_PT_CHUNK_SIZE, 0, 4)) < 0)
+ return ret;
+
+ p = pkt.data;
+ bytestream_put_be32(&p, rt->chunk_size);
Once you rebase this on top of the latest master, this will be
out_chunk_size (just to be clear). And since the out chunk size by default
is 128, just as it is, this packet won't actually have any effect. If we
wanted to have a different chunk size, we could change out_chunk_size
before sending this, and then send the new value to the client.
That is, what I'm saying is, this currently don't have much use in itself
(but is still ok to keep as such). We could increase the chunk size here,
but there's not much value in that since we mostly receive data, don't
send.
This is also where the current behaviour for publishing is bad, since when
publishing, we don't choose to explicitly use a larger chunk size, we just
set that if the server sent one. So when sending avconv->avconv/play,
neither party will increase their chunk size, and you'll be spending a few
extra bytes on sending with too small chunk size.
That is, the server could choose to increase the chunk size here. And when
in publishing mode (perhaps always, not just in that case), the client
code could just as well choose to increase the chunk size regardless of
what the server sent. That would make more sense than the current code.
+ ret = ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size,
+ rt->prev_pkt[1]);
+ ff_rtmp_packet_destroy(&pkt);
+ if (ret < 0)
+ return ret;
+
+ // Send result_ NetConnection.Connect.Success to connect
+ if ((ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL,
+ RTMP_PT_INVOKE, 0,
+ RTMP_PKTDATA_DEFAULT_SIZE)) < 0)
+ return ret;
+
+ p = pkt.data;
+ ff_amf_write_string(&p, "_result");
+ ff_amf_write_number(&p, 1);
This should be the transaction id of the connect call, right? That is,
seqnum in this function.
+
+ ff_amf_write_object_start(&p);
+ ff_amf_write_field_name(&p, "fmsVer");
+ ff_amf_write_string(&p, "FMS/3,0,1,123");
+ ff_amf_write_field_name(&p, "capabilities");
+ ff_amf_write_number(&p, 31);
+ ff_amf_write_object_end(&p);
+
+ ff_amf_write_object_start(&p);
+ ff_amf_write_field_name(&p, "level");
+ ff_amf_write_string(&p, "status");
+ ff_amf_write_field_name(&p, "code");
+ ff_amf_write_string(&p, "NetConnection.Connect.Success");
+ ff_amf_write_field_name(&p, "description");
+ ff_amf_write_string(&p, "Connection succeeded.");
+ ff_amf_write_field_name(&p, "objectEncoding");
+ ff_amf_write_number(&p, 0);
+ ff_amf_write_object_end(&p);
+
+ pkt.data_size = p - pkt.data;
+ ret = ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size,
+ rt->prev_pkt[1]);
+ ff_rtmp_packet_destroy(&pkt);
+ if (ret < 0)
+ return ret;
+
+ if ((ret = ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL,
+ RTMP_PT_INVOKE, 0, 28)) < 0)
+ return ret;
+ p = pkt.data;
+ ff_amf_write_string(&p, "onBWDone");
+ ff_amf_write_number(&p, 0);
+ ff_amf_write_null(&p);
+ ff_amf_write_number(&p, 8192);
+ pkt.data_size = p - pkt.data;
+ ret = ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size,
+ rt->prev_pkt[1]);
+ ff_rtmp_packet_destroy(&pkt);
+
+ return ret;
+}
+
/**
* Generate 'releaseStream' call and send it to the server. It should make
* the server release some channel for media streams.
@@ -927,6 +1066,132 @@ static int rtmp_handshake(URLContext *s, RTMPContext *rt)
return 0;
}
+static int rtmp_receive_hs_packet(RTMPContext* rt, uint32_t *first_int,
+ uint32_t *second_int, char *arraydata,
+ int size)
+{
+ ssize_t inoutsize;
+ uint8_t buffer[RTMP_HANDSHAKE_PACKET_SIZE];
+
+ inoutsize = ffurl_read_complete(rt->stream, buffer, sizeof(buffer));
+ if (inoutsize <= 0)
+ return AVERROR(EIO);
+ if (inoutsize != RTMP_HANDSHAKE_PACKET_SIZE) {
+ av_log(rt, AV_LOG_ERROR, "Erroneous Message size %d"
+ " not following standard\n", (int)inoutsize);
+ return AVERROR(EINVAL);
+ }
+
+ *first_int = AV_RB32(buffer);
+ *second_int = AV_RB32(buffer + 4);
+ memcpy(arraydata, buffer + 8,
+ RTMP_HANDSHAKE_PACKET_ARRAY_SIZE);
See how you still have the +8 here. This one would be even clearer if
you'd just use the full RTMP_HANDSHAKE_PACKET_SIZE.
+ return 0;
+}
+
+static int rtmp_send_hs_packet(RTMPContext* rt, uint32_t first_int,
+ uint32_t second_int, char *arraydata, int size)
+{
+ ssize_t inoutsize;
+ uint8_t buffer[RTMP_HANDSHAKE_PACKET_SIZE];
+
+ AV_WB32(buffer, first_int);
+ AV_WB32(buffer + 4, first_int);
+ memcpy(buffer + 8, arraydata, RTMP_HANDSHAKE_PACKET_ARRAY_SIZE);
The logic for what to copy would then be kept here, where'd you do
memcpy(buffer + 8, arraydata + 8, RTMP_HANDSHAKE_PACKET_SIZE - 8). You
could rename arraydata to "client_packet" or so as well. Then the code IMO
becomes clearer - in rtmp_receive_hs_packet you don't care about what to
copy and what not, you just store the full exact packet you received. Here
you then choose which parts to copy and which ones you don't, and that
logic doesn't need to be visible anywhere else in the code. Wouldn't that
be nice? :-)
+ inoutsize = ffurl_write(rt->stream, buffer,
+ RTMP_HANDSHAKE_PACKET_SIZE);
+ if (inoutsize != RTMP_HANDSHAKE_PACKET_SIZE) {
+ av_log(rt, AV_LOG_ERROR, "Unable to write answer\n");
+ return AVERROR(EIO);
+ }
+
+ return 0;
+}
+
+/**
+ * rtmp handshake server side
+ */
+static int rtmp_shandshake(URLContext *s, RTMPContext *rt)
I'd rather have this function named rtmp_server_handshake or so, which IMO
is clearer.
+{
+ uint8_t buffer[RTMP_HANDSHAKE_PACKET_SIZE];
+ uint32_t hs_epoch;
+ uint32_t hs_my_epoch;
+ uint8_t hs_peer_random[RTMP_HANDSHAKE_PACKET_ARRAY_SIZE];
+ uint8_t hs_my_random[RTMP_HANDSHAKE_PACKET_ARRAY_SIZE];
+ uint32_t zeroes;
+ uint32_t temp = 0;
+ int randomidx = 0;
+ ssize_t inoutsize = 0;
+ int ret;
+
+ inoutsize = ffurl_read_complete(rt->stream, buffer, 1); // Receive C0
+ if (inoutsize <= 0) {
+ av_log(s, AV_LOG_ERROR, "Unable to read handshake\n");
+ return AVERROR(EIO);
+ }
+ if (buffer[0] != 3) { /* Check Version */
+ av_log(s, AV_LOG_ERROR, "RTMP protocol version mismatch\n");
+ return AVERROR_PROTOCOL_NOT_FOUND;
+ }
+ if (ffurl_write(rt->stream, buffer, 1) <= 0) { // Send S0
+ av_log(s, AV_LOG_ERROR,
+ "Unable to write answer - RTMP S0\n");
+ return AVERROR(EIO);
+ }
+ /* Receive C1 */
+ ret = rtmp_receive_hs_packet(rt, &hs_epoch, &zeroes, hs_peer_random,
+ RTMP_HANDSHAKE_PACKET_ARRAY_SIZE);
+ if (ret) {
+ av_log(s, AV_LOG_ERROR, "RTMP Handshake C1 Error\n");
+ return ret;
+ }
+ if (zeroes) {
+ av_log(NULL, AV_LOG_WARNING,
+ "Erroneous C1 Message zero != 0\n");
+ }
+ /* Send S1 */
+ /* By now same epoch will be sent */
+ hs_my_epoch = hs_epoch;
+ /* Generate random */
+ for (randomidx = 0; randomidx < (RTMP_HANDSHAKE_PACKET_ARRAY_SIZE);
+ randomidx += 4)
+ AV_WB32(hs_my_random + randomidx, av_get_random_seed());
+
+ ret = rtmp_send_hs_packet(rt, hs_my_epoch, 0, hs_my_random,
+ RTMP_HANDSHAKE_PACKET_ARRAY_SIZE);
+ if (ret) {
+ av_log(s, AV_LOG_ERROR, "RTMP Handshake S1 Error\n");
+ return ret;
+ }
+ /* Send S2 */
+ ret = rtmp_send_hs_packet(rt, hs_epoch, 0, hs_peer_random,
+ RTMP_HANDSHAKE_PACKET_ARRAY_SIZE);
+ if (ret) {
+ av_log(s, AV_LOG_ERROR, "RTMP Handshake S2 Error\n");
+ return ret;
+ }
+ /* Receive C2 */
+ ret = rtmp_receive_hs_packet(rt, &temp, &zeroes, hs_peer_random,
+ RTMP_HANDSHAKE_PACKET_ARRAY_SIZE);
+ if (ret) {
+ av_log(s, AV_LOG_ERROR, "RTMP Handshake C2 Error\n");
+ return ret;
+ }
+ if (temp != hs_my_epoch) {
+ av_log(NULL, AV_LOG_ERROR, "Erroneous C2 Message epoch"
+ " does not match up with C1 epoch\n");
+ return AVERROR_INVALIDDATA;
+ }
+ if (memcmp(hs_peer_random, hs_my_random,
+ RTMP_HANDSHAKE_PACKET_ARRAY_SIZE)) {
+ av_log(NULL, AV_LOG_ERROR, "Erroneous C2 Message random"
+ " does not match up\n");
+ return AVERROR_INVALIDDATA;
+ }
If you choose to remove RTMP_HANDSHAKE_PACKET_ARRAY_SIZE like I suggest,
the logic about "which parts of the handshake do we validate, and how is
that validation done?" is kept encapsulated here, and not spread out all
over the place.
+
+ return 0;
+}
+
static int handle_chunk_size(URLContext *s, RTMPPacket *pkt)
{
RTMPContext *rt = s->priv_data;
@@ -1021,6 +1286,122 @@ static int handle_server_bw(URLContext *s, RTMPPacket
*pkt)
return 0;
}
+static int send_invoke_response(URLContext *s, RTMPPacket *pkt)
+{
+ RTMPContext *rt = s->priv_data;
+ double seqnum;
+ char filename[64];
+ char command[64];
+ char statusmsg[128];
+ int stringlen;
+ char *pchar;
+ const uint8_t *p = pkt->data;
+ uint8_t *pp = NULL;
+ RTMPPacket spkt = { 0 };
+ GetByteContext gbc;
+ int ret;
+
+ bytestream2_init(&gbc, p, pkt->data_size);
+ if (ff_amf_read_string(&gbc, command, sizeof(command),
+ &stringlen)) {
+ av_log(s, AV_LOG_ERROR, "Error in PT_INVOKE\n");
+ return AVERROR_INVALIDDATA;
+ }
+
+ ret = ff_amf_read_number(&gbc, &seqnum);
+ if (ret)
+ return ret;
+ ret = ff_amf_read_null(&gbc);
Weird spacing
+ if (ret)
+ return ret;
+ if (!strcmp(command, "FCPublish") ||
+ !strcmp(command, "publish")) {
+ ret = ff_amf_read_string(&gbc, filename,
+ sizeof(filename), &stringlen);
+ // check with url
+ if (s->filename) {
+ pchar = strrchr(s->filename, '/');
+ pchar++;
+ if (strcmp(pchar, filename))
+ av_log(s, AV_LOG_WARNING, "Unexpected stream %s, expecting"
+ " %s\n", filename, pchar);
+ }
+ }
+
+ if (!strcmp(command, "FCPublish")) {
+ if ((ret = ff_rtmp_packet_create(&spkt, RTMP_SYSTEM_CHANNEL,
+ RTMP_PT_INVOKE, 0,
+ RTMP_PKTDATA_DEFAULT_SIZE)) < 0) {
+ av_log(s, AV_LOG_ERROR, "Unable to create response packet\n");
+ return ret;
+ }
+ pp = spkt.data;
+ ff_amf_write_string(&pp, "onFCPublish");
+ } else if (!strcmp(command, "publish")) {
+ PutByteContext pbc;
+ // Send Stream Begin 1 like Wowza
You can remove "like Wowza", I think most servers do that
+ if ((ret = ff_rtmp_packet_create(&spkt, RTMP_NETWORK_CHANNEL,
+ RTMP_PT_PING, 0, 6)) < 0) {
+ av_log(s, AV_LOG_ERROR, "Unable to create response packet\n");
+ return ret;
+ }
+ pp = spkt.data;
+ bytestream2_init_writer(&pbc, pp, spkt.data_size);
+ bytestream2_put_be16(&pbc, 1); // 1 -> Stream Begin
+ bytestream2_put_be32(&pbc, 0);
No, this is the wrong way around. The first one should be 0, which means
"Stream Begin", the second one should be 1. (I don't remember if this was
the number of the stream that was started, or what its meaning was...)
+ ret = ff_rtmp_packet_write(rt->stream, &spkt, rt->chunk_size,
+ rt->prev_pkt[1]);
+ ff_rtmp_packet_destroy(&spkt);
+ if (ret < 0)
+ return ret;
+
+ // Send onStatus(NetStream.Publish.Start)
+ if ((ret = ff_rtmp_packet_create(&spkt, RTMP_SYSTEM_CHANNEL,
+ RTMP_PT_INVOKE, 0,
+ RTMP_PKTDATA_DEFAULT_SIZE)) < 0) {
+ av_log(s, AV_LOG_ERROR, "Unable to create response packet\n");
+ return ret;
+ }
+ spkt.extra = 1; // Wowza magic
No, this is not wowza magic, or related to wowza at all. The "extra" field
is what wireshark calls "stream id" (and what librtmp calls
m_nInfoField2), and I think it's supposed to be which stream this message
is related to. Without this, stricter clients (like wirecast) don't know
that this status is related to the stream it is awaiting a status on. So
this
Unrelated, I think we should rename the extra field to stream_id.
Also, this is missing a pp = spkt.data;
+ ff_amf_write_string(&pp, "onStatus");
+ ff_amf_write_number(&pp, 0);
+ ff_amf_write_null(&pp);
+
+ ff_amf_write_object_start(&pp);
+ ff_amf_write_field_name(&pp, "level");
+ ff_amf_write_string(&pp, "status");
+ ff_amf_write_field_name(&pp, "code");
+ ff_amf_write_string(&pp, "NetStream.Publish.Start");
+ ff_amf_write_field_name(&pp, "description");
+ snprintf(statusmsg, sizeof(statusmsg),
+ "%s is now published", filename);
+ ff_amf_write_string(&pp, statusmsg);
+ ff_amf_write_field_name(&pp, "details");
+ ff_amf_write_string(&pp, filename);
+ ff_amf_write_field_name(&pp, "clientid");
+ snprintf(statusmsg, sizeof(statusmsg), "%s", LIBAVFORMAT_IDENT);
+ ff_amf_write_string(&pp, statusmsg);
+ ff_amf_write_object_end(&pp);
+
+ } else {
+ if ((ret = ff_rtmp_packet_create(&spkt, RTMP_SYSTEM_CHANNEL,
+ RTMP_PT_INVOKE, 0,
+ RTMP_PKTDATA_DEFAULT_SIZE)) < 0) {
+ av_log(s, AV_LOG_ERROR, "Unable to create response packet\n");
+ return ret;
+ }
+ pp = spkt.data;
+ ff_amf_write_string(&pp, "_result");
+ ff_amf_write_number(&pp, seqnum);
+ ff_amf_write_null(&pp);
If the invoke was createStream, the response needs to contain a number as
well. This number is, as far as I know, the number that you'll set in
spkt.extra and everything else that is related to the stream that the
client created.
That is, in my tests, I added this code right here:
if (!strcmp(command, "createStream"))
ff_amf_write_number(&pp, 1);
+ }
+ spkt.data_size = pp - spkt.data;
+ ret = ff_rtmp_packet_write(rt->stream, &spkt, rt->chunk_size,
+ rt->prev_pkt[1]);
+ ff_rtmp_packet_destroy(&spkt);
+ return ret;
+}
+
static int handle_invoke(URLContext *s, RTMPPacket *pkt)
{
RTMPContext *rt = s->priv_data;
@@ -1132,6 +1513,13 @@ static int handle_invoke(URLContext *s, RTMPPacket *pkt)
} else if (!memcmp(pkt->data, "\002\000\010onBWDone", 11)) {
if ((ret = gen_check_bw(s, rt)) < 0)
return ret;
+ } else if (!memcmp(pkt->data, "\002\000\015releaseStream", 16) ||
+ !memcmp(pkt->data, "\002\000\011FCPublish", 12) ||
+ !memcmp(pkt->data, "\002\000\007publish", 10) ||
+ !memcmp(pkt->data, "\002\000\010_checkbw", 11) ||
+ !memcmp(pkt->data, "\002\000\014createStream", 15)) {
+ if (ret = send_invoke_response(s, pkt) < 0)
+ return ret;
}
invoke_fail:
@@ -1139,6 +1527,57 @@ invoke_fail:
return ret;
}
+static int handle_notify(URLContext *s, RTMPPacket *pkt) {
+ RTMPContext *rt = s->priv_data;
+ const uint8_t *p = NULL;
+ uint8_t *cp = NULL;
+ uint8_t commandbuffer[64];
+ char statusmsg[128];
+ int stringlen;
+ GetByteContext gbc;
+ PutByteContext pbc;
+ uint32_t ts;
+
+ p = pkt->data;
+ bytestream2_init(&gbc, p, pkt->data_size);
+ if (ff_amf_read_string(&gbc, commandbuffer, sizeof(commandbuffer),
+ &stringlen))
+ return AVERROR_INVALIDDATA;
+ if (!strcmp(commandbuffer, "@setDataFrame")) {
+ if (ff_amf_read_string(&gbc, statusmsg,
+ sizeof(statusmsg), &stringlen))
+ return AVERROR_INVALIDDATA;
+ if (strcmp(statusmsg, "onMetaData"))
+ return AVERROR_INVALIDDATA;
I'm not sure if this should be an error, you could just as well just
see it as "this was some other data that we don't care about right now",
and return 0. Right now this code will make it fail hard as soon as the
caller does _anything_ else than what you thought.
+ /* Provide ECMAArray to flv */
+ ts = pkt->timestamp;
+
+ // generate packet header and put data into buffer for FLV demuxer
+ if (rt->flv_size - rt->flv_off < pkt->data_size + 15) {
+ rt->flv_data = cp =
+ av_realloc(rt->flv_data, rt->flv_off + pkt->data_size + 15);
If realloc fails, you will have overwritten the flv_data pointer, which
then is leaked. You need to check the return value, and you need to keep
the old pointer until you know the realloc succeeded.
+ rt->flv_size = rt->flv_off + pkt->data_size + 15;
+ } else {
+ cp = rt->flv_data;
+ }
+
+ bytestream2_init_writer(&pbc, cp + rt->flv_off,
+ rt->flv_size - rt->flv_off);
+ bytestream2_put_byte(&pbc, pkt->type);
+ bytestream2_put_be24(&pbc, pkt->data_size);
+ bytestream2_put_be24(&pbc, ts);
+ bytestream2_put_byte(&pbc, ts >> 24);
+ bytestream2_put_be24(&pbc, 0);
+ bytestream2_put_buffer(&pbc, pkt->data, pkt->data_size);
+ bytestream2_put_be32(&pbc, 0);
Did you ever see the metadata that you received here? You don't update the
flv_off to reflect the newly written data, so it's never actually
returned. But this isn't everything you need to do in order to make that
work. Please test and make sure that the metadata that the client sends
actually gets handled properly by the flv demuxer.
+ ff_rtmp_packet_destroy(pkt);
This packet is freed in get_packet already, this makes it freed twice,
which isn't ok.
+ return 0;
+ } else
+ return AVERROR_INVALIDDATA;
This breaks normal playback - please test that as well.
And in general, I think it's much better to have code like this to be more
tolerant than strict, so we don't fail as soon a peer sends something we
don't understand - in most cases you can just ignore it, and things will
be more robust.
+
+ return 0;
+}
+
/**
* Parse received packet and possibly perform some action depending on
* the packet contents.
@@ -1261,6 +1700,14 @@ static int get_packet(URLContext *s, int for_header)
bytestream_put_be32(&p, 0);
ff_rtmp_packet_destroy(&rpkt);
return 0;
+ } else if (rpkt.type == RTMP_PT_NOTIFY) {
+ ret = handle_notify(s, &rpkt);
+ if (ret) {
+ av_log(s, AV_LOG_ERROR, "Handle notify error\n");
If this happens, the packet will be leaked.
+ return ret;
+ }
+ ff_rtmp_packet_destroy(&rpkt);
+ return 0;
} else if (rpkt.type == RTMP_PT_METADATA) {
// we got raw FLV data, make it available for FLV demuxer
rt->flv_off = 0;
@@ -1331,6 +1778,9 @@ static int rtmp_open(URLContext *s, const char *uri, int
flags)
AVDictionary *opts = NULL;
int ret;
+ if (rt->listen_timeout > 0)
+ rt->listen = 1;
+
rt->is_input = !(flags & AVIO_FLAG_WRITE);
av_url_split(proto, sizeof(proto), NULL, 0, hostname, sizeof(hostname),
&port,
@@ -1358,7 +1808,12 @@ static int rtmp_open(URLContext *s, const char *uri, int
flags)
/* open the tcp connection */
if (port < 0)
port = RTMP_DEFAULT_PORT;
- ff_url_join(buf, sizeof(buf), "tcp", NULL, hostname, port, NULL);
+ if (rt->listen)
+ ff_url_join(buf, sizeof(buf), "tcp", NULL, hostname, port,
+ "?listen&listen_timeout=%d",
+ rt->listen_timeout * 1000);
+ else
+ ff_url_join(buf, sizeof(buf), "tcp", NULL, hostname, port, NULL);
}
You only do this in the plain "rtmp" case. If the caller sets -rtmp_listen
but uses a rtmpe or rtmpt url, things will misbehave. For those
unimplemented cases, make sure we return an error here.
Other than this, I see you managed to do a lot of cleanup I suggested in
the previous round - now this is much nicer, with a lot less code specific
to this case.
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
