On Fri, Jun 15, 2012 at 10:55 AM, Martin Storsjö <mar...@martin.st> wrote:
> On Wed, 13 Jun 2012, Samuel Pitoiset wrote:
>
>> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
>> index 1320a28..69f27ab 100644
>> --- a/libavformat/allformats.c
>> +++ b/libavformat/allformats.c
>> @@ -256,6 +256,8 @@ void av_register_all(void)
>> REGISTER_PROTOCOL (MD5, md5);
>> REGISTER_PROTOCOL (PIPE, pipe);
>> REGISTER_PROTOCOL (RTMP, rtmp);
>> + REGISTER_PROTOCOL (RTMPT, rtmpt);
>> + REGISTER_PROTOCOL (RTMPHTTP, rtmphttp);
>> REGISTER_PROTOCOL (RTP, rtp);
>> REGISTER_PROTOCOL (SCTP, sctp);
>> REGISTER_PROTOCOL (TCP, tcp);
>
>
> The patch is missing proper dependecies in configure. E.g. if you do
> ./configure --disable-everything --enable-protocol=rtmpt, it should enable
> everything which is needed (rtmphttp, http, tcp), have a look at how this is
> handled in configure. Also, building with librtmp doesn't work at the
> moment, you need to make sure the native rtmpt protocol isn't built if you
> use librtmp.
Indeed, I discovered this issue yesterday when I tried to publish
streams with librtmp.
>
>> diff --git a/libavformat/rtmphttp.c b/libavformat/rtmphttp.c
>> new file mode 100644
>> index 0000000..17d309f
>> --- /dev/null
>> +++ b/libavformat/rtmphttp.c
>> @@ -0,0 +1,220 @@
>> +/*
>> + * RTMP HTTP network protocol
>> + * Copyright (c) 2012 Samuel Pitoiset
>> + *
>> + * This file is part of Libav.
>> + *
>> + * Libav is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * Libav is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with Libav; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> 02110-1301 USA
>> + */
>> +
>> +/**
>> + * @file
>> + * RTMP HTTP protocol
>> + */
>> +
>> +#include "libavutil/avstring.h"
>> +#include "libavutil/intfloat.h"
>> +#include "libavutil/opt.h"
>> +#include "internal.h"
>> +#include "http.h"
>> +
>> +#define RTMPT_DEFAULT_PORT 80
>> +
>> +/* protocol handler state */
>> +typedef enum {
>> + STATE_START, ///< client has not done anything yet
>> + STATE_OPENED, ///< client has opened the connection
>> + STATE_WAITING, ///< client is waiting for a server reply
>> + STATE_READY, ///< client is ready for sending requests
>> +} ClientState;
>> +
>> +/* 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
>> + int out_capacity; ///< current output buffer capacity
>> +} RTMP_HTTPContext;
>> +
>> +static int rtmp_http_send_cmd(URLContext *h, const char *cmd)
>> +{
>> + 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", rt->out_data,
>> + rt->out_size, 0);
>> +
>> + /* send a new request to the server */
>> + if ((ret = ff_http_do_new_request(rt->stream, uri)) < 0)
>> + return ret;
>> +
>> + /* re-init output buffer */
>> + rt->out_size = 0;
>> +
>> + /* read the first byte which contains the polling interval */
>> + if ((ret = ffurl_read(rt->stream, &c, 1)) < 0)
>> + return ret;
>> +
>> + return ret;
>> +}
>> +
>> +static int rtmp_http_write(URLContext *h, const uint8_t *buf, int size)
>> +{
>> + RTMP_HTTPContext *rt = h->priv_data;
>> + void *ptr;
>> +
>> + if (rt->out_size + size > rt->out_capacity) {
>> + rt->out_capacity *= 2;
>
>
> Two flaws: Since out_capacity starts out at 0, you will double the capacity
> to 0 here. Did you really test this?
>
> Also, even if out_capacity would be something else than zero, say
> out_capacity == 10, out_size == 8 and size == 40, you'll only double the
> capacity to 20 but you'll still not be able to fit the full 'size' here. So
> either of these would be better:
>
> out_capacity = 2*(out_capacity + size);
>
> or
>
> out_capacity = 2*out_capacity + size;
>
> or
>
> out_capacity = 2*(out_size + size);
>
> or
>
> out_capacity = 2*out_size + size;
>
> All of them make sure you have space for at least 'size' more bytes, and
> adds a bit of headroom to make sure you won't need to reallocate every
> single time.
>
Already fixed locally.
>
>> + ptr = av_realloc(rt->out_data, rt->out_capacity);
>> + if (!ptr)
>> + return AVERROR(ENOMEM);
>> + rt->out_data = ptr;
>> + }
>> +
>> + memcpy(rt->out_data + rt->out_size, buf, size);
>> + rt->out_size += size;
>> +
>> + return size;
>> +}
>> +
>> +static int rtmp_http_read(URLContext *h, uint8_t *buf, int size)
>> +{
>> + RTMP_HTTPContext *rt = h->priv_data;
>> + int ret, off = 0;
>> +
>> + /* try to read at least 1 byte of data */
>> + do {
>> + ret = ffurl_read(rt->stream, buf + off, size);
>> + if (ret < 0 && ret != AVERROR_EOF)
>> + return ret;
>> +
>> + if (ret == AVERROR_EOF) {
>> + /* 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")) < 0)
>> + return ret;
>> + } else {
>> + if ((ret = rtmp_http_write(h, "", 1)) < 0)
>> + return ret;
>> +
>> + if ((ret = rtmp_http_send_cmd(h, "idle")) < 0)
>> + return ret;
>> + }
>> + } else {
>> + off += ret;
>> + size -= ret;
>> + }
>> + } while (off <= 0);
>> +
>> + return off;
>> +}
>> +
>> +static int rtmp_http_close(URLContext *h)
>> +{
>> + RTMP_HTTPContext *rt = h->priv_data;
>> + uint8_t tmp_buf[2048];
>> + int ret = 0;
>> +
>> + if (rt->state > STATE_OPENED) {
>> + // TODO: Add support for the nonblocking variant.
>> + if ((ret == rtmp_http_write(h, "", 1)) > 0)
>
>
> Still a case of ==. When you have such an issue at one place, be sure to
> doublecheck all of the new code so you don't have more of them.
Fixed.
>
>> + ret = rtmp_http_send_cmd(h, "close");
>> + }
>> +
>> + 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, off = 0;
>> +
>> + 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)
>> + return ret;
>> +
>> + /* 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)
>> + return ret;
>> +
>> + /* read the server reply which contains a unique ID */
>> + for (;;) {
>> + ret = ffurl_read(rt->stream, rt->client_id + off,
>> sizeof(rt->client_id) - off);
>> + if (ret == AVERROR_EOF)
>> + break;
>> + if (ret < 0 || off == sizeof(rt->client_id)) {
>> + rtmp_http_close(h);
>> + return ret;
>
>
> If off == sizeof(client_id), you return a positive number, making the caller
> believe this succeeded.
Fixed.
--
Best regards,
Samuel Pitoiset.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
