On Tue, 5 Jun 2012, Samuel Pitoiset wrote:
+/* protocol handler context */
+typedef struct RTMP_HTTPContext {
+ URLContext *stream; ///< HTTP stream
+ char host[256]; ///< hostname of the server
+ int port; ///< port to connect (default is 80)
+ char client_id[64]; ///< client ID used for all requests
except the first one
+ int seq; ///< sequence ID used for all requests
+ ClientState state; ///< current state
+ uint8_t *out_data; ///< output buffer
+ int out_size; ///< current output buffer size
+} RTMP_HTTPContext;
+
+static int rtmp_http_send_cmd(URLContext *h, const char *cmd,
+ const uint8_t *buf, int size)
+{
+ RTMP_HTTPContext *rt = h->priv_data;
+ char uri[2048];
+ uint8_t c;
+ int ret;
+
+ ff_url_join(uri, sizeof(uri), "http", NULL, rt->host, rt->port,
+ "/%s/%s/%d", cmd, rt->client_id, rt->seq++);
+
+ av_opt_set_bin(rt->stream->priv_data, "post_data", buf, size, 0);
+
+ /* send a new request to the server */
+ if ((ret = ff_http_do_new_request(rt->stream, uri)) < 0)
+ return ret;
+
+ /* free output buffer */
+ av_freep(&rt->out_data);
+ rt->out_size = 0;
This isn't good design. If the function takes any buffer as parameter, one
wouldn't expect it to explicitly modify the out_data/out_size parameters.
That is, either don't change them here, or don't take any buffer/size
parameters at all and just use out_data/out_size explicitly as the data to
send, too.
+ /* read the first byte which contains the polling interval */
+ if ((ret = ffurl_read(rt->stream, &c, 1)) < 0)
+ return ret;
+
+ /* client is now waiting for a server reply */
+ rt->state = STATE_WAITING;
+
+ return ret;
+}
+
+static int rtmp_http_close(URLContext *h)
+{
+ RTMP_HTTPContext *rt = h->priv_data;
+ int ret = 0;
+
+ if (rt->state > STATE_OPENED)
+ ret = rtmp_http_send_cmd(h, "close", "", 1);
+
+ av_freep(&rt->out_data);
+ ffurl_close(rt->stream);
+
+ return ret;
+}
+
+static int rtmp_http_open(URLContext *h, const char *uri, int flags)
+{
+ RTMP_HTTPContext *rt = h->priv_data;
+ char headers[1024], url[1024];
+ int ret;
+
+ av_url_split(NULL, 0, NULL, 0, rt->host, sizeof(rt->host), &rt->port,
+ NULL, 0, uri);
+
+ rt->state = STATE_START;
+ if (rt->port < 0)
+ rt->port = RTMPT_DEFAULT_PORT;
+
+ /* This is the first request that is sent to the server in order to
+ * register a client on the server and start a new session. The server
+ * replies with a unique id (usually a number) that is used by the client
+ * for all future requests.
+ * Note: the reply doesn't contain a value for the polling interval.
+ * A successful connect resets the consecutive index that is used
+ * in the URLs. */
+ ff_url_join(url, sizeof(url), "http", NULL, rt->host, rt->port, "/open/1");
+
+ /* alloc the http context */
+ if ((ret = ffurl_alloc(&rt->stream, url, AVIO_FLAG_READ_WRITE, NULL)) < 0)
+ goto fail;
+
+ /* set options */
+ snprintf(headers, sizeof(headers),
+ "Cache-Control: no-cache\r\n"
+ "Content-type: application/x-fcs\r\n");
+ av_opt_set(rt->stream->priv_data, "headers", headers, 0);
+ av_opt_set(rt->stream->priv_data, "multiple_requests", "1", 0);
+ av_opt_set_bin(rt->stream->priv_data, "post_data", "", 1, 0);
+
+ /* open the http context */
+ if ((ret = ffurl_connect(rt->stream, NULL)) < 0)
+ goto fail;
+
+ /* read the server reply which contains a unique ID */
+ for (;;) {
+ ret = ffurl_read(rt->stream, rt->client_id, sizeof(rt->client_id));
+ if (ret == AVERROR_EOF)
+ break;
+ if (ret < 0)
+ goto fail;
+ rt->client_id[ret - 1] = '\0';
+ }
This isn't right. ffurl_read could easily return one single char at a time
here, then this wouldn't work. (To test this, try modifying tcp_read in
tcp.c, set size to 1 here - this should work with all calling code.)
Also, the blind [ret - 1] = '\0' isn't too nice IMO (although librtmp does
things similiarly). Ideally I'd check the received buffer from the end and
trim out all whitespace characters, so if the buffer would happen to
contain \r\n it would work just as well.
+ /* client has now opened the http tunneling connection */
+ rt->state = STATE_OPENED;
+
+ return 0;
+
+fail:
+ rtmp_http_close(h);
+ return ret;
+}
+
+static int rtmp_http_read(URLContext *h, uint8_t *buf, int size)
+{
+ RTMP_HTTPContext *rt = h->priv_data;
+ int orig_size = size;
+ int ret, off = 0;
+
+ while (size > 0) {
+ ret = ffurl_read(rt->stream, buf + off, size);
+ if (ret < 0 && ret != AVERROR_EOF)
+ return ret;
This will make things much slower and add more latency than required.
You're only returning once you have the full 'size' amount of data. If the
stream is an audio-only 8 kHz stream, and size is 32 KB (as it ofte is
with AVIOContext), it might require 8 seconds worth of data to fill the 32
KB buffer. So the caller will not get a single byte of data until you have
filled the buffer, giving you a minimum latency of 8 seconds, and really
choppy behaviour.
If the caller wants more data, the caller will call read again, otherwise
it is ok to return as soon as you have at least 1 byte of data to return.
+
+ if (ret == AVERROR_EOF) {
+ /* client is now ready to send new requests */
+ rt->state = STATE_READY;
+
+ /* When the client has reached end of file for the last request,
+ * we have to send a new request if we have buffered data.
+ * Otherwise, we have to send an idle POST. */
+ if (rt->out_size > 0) {
+ if ((ret = rtmp_http_send_cmd(h, "send", rt->out_data,
+ rt->out_size)) < 0)
+ return ret;
+ } else {
+ if ((ret == rtmp_http_send_cmd(h, "idle", "", 1)) < 0)
+ return ret;
+ }
+ } else {
+ off += ret;
+ size -= ret;
+ }
+ }
+
+ return orig_size;
+}
+
+static int rtmp_http_write(URLContext *h, const uint8_t *buf, int size)
+{
+ RTMP_HTTPContext *rt = h->priv_data;
+ void *ptr;
+
+ ptr = av_realloc(rt->out_data, rt->out_size + size);
+ if (!ptr)
+ return AVERROR(ENOMEM);
+ rt->out_data = ptr;
+
You could save some reallocation work here by reusing the same buffer -
when clearing it, you could just set the size back to 0 but keep the old
buffer, and keep track of the allocation size in a separate variable. The
downside of this is that if the buffer ever grows excessively big, it
won't be shrunk.
+ memcpy(rt->out_data + rt->out_size, buf, size);
+ rt->out_size += size;
+
+ return size;
+}
+
+static int rtmp_http_shutdown(URLContext *h, int flags)
+{
+ RTMP_HTTPContext *rt = h->priv_data;
+ int ret;
+
+ if (rt->state == STATE_WAITING) {
+ /* Do not send a new request until the client is waiting
+ * for a server reply of the latest request. */
+ return 0;
+ }
+
+ if ((ret = rtmp_http_send_cmd(h, "send", rt->out_data, rt->out_size)) < 0)
+ return ret;
Given the way the code is written right now, is there any time except at
startup when this actually will be used? I think not - once you enter
STATE_WAITING, you'll only switch out of it in rtmp_http_read once you hit
EOF, and in both cases there, you will switch back into STATE_WAITING
(unless an error occurs).
This isn't (necessarily) an issue in itself, it's just a think which might
(or might not) allow you to simplify things further.
+
+ return ret;
+}
+
+URLProtocol ff_rtmphttp_protocol = {
+ .name = "rtmphttp",
+ .url_open = rtmp_http_open,
+ .url_read = rtmp_http_read,
+ .url_write = rtmp_http_write,
+ .url_close = rtmp_http_close,
+ .url_shutdown = rtmp_http_shutdown,
+ .priv_data_size = sizeof(RTMP_HTTPContext),
+ .flags = URL_PROTOCOL_FLAG_NETWORK,
+};
diff --git a/libavformat/rtmppkt.c b/libavformat/rtmppkt.c
index ed8e6b2..dd719e3 100644
--- a/libavformat/rtmppkt.c
+++ b/libavformat/rtmppkt.c
@@ -163,8 +163,8 @@ int ff_rtmp_packet_read(URLContext *h, RTMPPacket *p,
return size;
}
-int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
- int chunk_size, RTMPPacket *prev_pkt)
+int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt, int chunk_size,
+ RTMPPacket *prev_pkt, int http_tunneling)
{
uint8_t pkt_hdr[16], *p = pkt_hdr;
int mode = RTMP_PS_TWELVEBYTES;
@@ -237,6 +237,14 @@ int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
size++;
}
}
+
+ if (http_tunneling) {
+ /* When rtmpt is used we have to signal the URLContext that we are done
+ * writing the stream in order to send the packet in one chunk. */
+ if ((ret = ffurl_shutdown(h, AVIO_FLAG_WRITE)) < 0)
+ return ret;
+ }
+
return size;
}
I think we can make do without this change, with a bit of reasoning: In
most (all?) cases, the shutdown call will actually not be able to flush
the data, since we're still waiting for the end of the current http
request, so it will actually not be sent at the next read() call. So as
long as we do read calls regularly, we really won't need to flush it
explicitly like this almost at all. This avoids having to change this
function and all the callers of it, shrinking the patch quite a bit.
Also, a bit more of reasoning to why I think this should be ok: This code
will mostly be blocking waiting for data to read. At startup during the
handshake, the client sends a few packets (which are buffered and not sent
yet), then the client reads a reply and waits until it receives a packet
indicating the desired state change. Even if the data is buffered for
quite some time, we will read all data from the server until the client
receives the right packets, so it will flush the data at some point.
So in principle, I think you could actually skip every single
ffurl_shutdown() call here, and things would work just as well as before.
There's one exception to this reasoning, though, which will require larger
fixes within rtmpproto.c (actually this could be seen as a bug in the
current code). When writing data to the server (as in, publishing a live
stream), only rtmp_write is called, never rtmp_read. Given the way this
function is implemented right now, we never ever read from the handle
again after the initial setup.
First, the issue with the code for plain rtmp - if the server has any
message to send to us, we won't read it and act upon it. If the server
would require us to e.g. reply to some ping message regularly, we wouldn't
know about it.
For rtmpt, an initial (but insufficient) solution would be the following:
Buffer the initial byte of the response. That is, in rtmp_http_send_cmd,
after you've read the polling interval byte, you read another byte. If
this succeeds, you store it and return it immediately at the next
rtmp_http_read call. If it fails with EOF, you switch state back to the
right one.
This allows you to continuously send data, as long as the server just
replies with a 1-byte response (the polling byte) each time.
But if the server response contains more data than this, we will never be
able to send any more data until we've read the rest of the response -
since we're still in the state that we've got more data to read from the
current request until we can send the next one, and we can't send any more
data until someone actually consumes the pending input data.
I don't have a good solution off-hand for this problem right now, I'll try
to think about how to solve it cleanly, given the current urlprotocol
architecture (in other words - the minimal changes to the urlprotocol
system needed for solving this properly).
+URLProtocol ff_rtmpt_protocol = {
+ .name = "rtmpt",
+ .url_open = rtmp_open,
+ .url_read = rtmp_read,
+ .url_write = rtmp_write,
+ .url_close = rtmp_close,
+ .priv_data_size = sizeof(RTMPContext),
+ .flags = URL_PROTOCOL_FLAG_NETWORK,
+ .priv_data_class = &rtmp_class,
+};
IIRC (Anton please correct me if I misremember) you have to have a
separate, unique class for each protocol (or was that only for
demuxers/muxers?), so you need to create a rtmpt_class for this one.
// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
