On Fri, Jul 3, 2015 at 4:18 PM, Nicolas George <geo...@nsup.org> wrote:
> Le quintidi 15 messidor, an CCXXIII, Stephan Holljes a écrit :
>> Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
>> ---
>> libavformat/http.c | 45 ++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 32 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavformat/http.c b/libavformat/http.c
>> index d9c3624..95065f5 100644
>> --- a/libavformat/http.c
>> +++ b/libavformat/http.c
>> @@ -129,7 +129,7 @@ static const AVOption options[] = {
>> { "end_offset", "try to limit the request to bytes preceding this
>> offset", OFFSET(end_off), AV_OPT_TYPE_INT64, { .i64 = 0 }, 0, INT64_MAX, D },
>> { "method", "Override the HTTP method or set the expected HTTP method
>> from a client", OFFSET(method), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D
>> | E },
>> { "reconnect", "auto reconnect after disconnect before EOF",
>> OFFSET(reconnect), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, D },
>> - { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 =
>> 0 }, 0, 1, D | E },
>> + { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 =
>> 0 }, 0, 2, D | E },
>> { NULL }
>> };
>>
>> @@ -307,6 +307,8 @@ static void handle_http_errors(URLContext *h, int error)
>> HTTPContext *s = h->priv_data;
>> if (h->is_connected) {
>> switch(error) {
>> + case 0:
>> + break;
>> case AVERROR_HTTP_BAD_REQUEST:
>> ffurl_write(s->hd, bad_request, strlen(bad_request));
>> break;
>> @@ -317,15 +319,32 @@ static void handle_http_errors(URLContext *h, int
>> error)
>> }
>> }
>>
>> +static int http_handshake(URLContext *c) {
>> + int ret, err = 0, new_location;
>> + HTTPContext *ch = c->priv_data;
>> + URLContext *cl = ch->hd;
>> + static const char header[] = "HTTP/1.1 200 OK\r\nContent-Type:
>> application/octet-stream\r\nTransfer-Encoding: chunked\r\n\r\n";
>> + if ((ret = ffurl_handshake(cl)) < 0)
>> + return ret;
>> + if ((err = http_read_header(c, &new_location)) < 0)
>> + goto fail;
>> + if ((ret = ffurl_write(cl, header, strlen(header))) < 0)
>> + goto fail;
>
>> + // Avoid returning a positive value from ffurl_write()
>> + ret = ret > 0 ? 0 : ret;
>> +fail:
>> + handle_http_errors(c, err);
>> + return ret;
>
> This is a matter of taste, but I find it more readable if it avoids going
> over the "fail" label:
>
> + if ((ret = ffurl_write(cl, header, strlen(header))) < 0)
> + goto fail;
> + return 0;
> +fail:
> + handle_http_errors(c, err);
> + return ret;
>
> That way, you also do not need to change handle_http_errors() for error=0.
> (Although av_assert0(error < 0) would do no harm there.)
>
>> +}
>> +
>> static int http_listen(URLContext *h, const char *uri, int flags,
>> AVDictionary **options) {
>> HTTPContext *s = h->priv_data;
>> int ret;
>> - static const char header[] = "HTTP/1.1 200 OK\r\nContent-Type:
>> application/octet-stream\r\nTransfer-Encoding: chunked\r\n\r\n";
>> char hostname[1024], proto[10];
>> char lower_url[100];
>> const char *lower_proto = "tcp";
>> - int port, new_location;
>> + int port;
>> s->chunked_post = 1;
>> av_url_split(proto, sizeof(proto), NULL, 0, hostname, sizeof(hostname),
>> &port,
>> NULL, 0, uri);
>> @@ -333,18 +352,16 @@ static int http_listen(URLContext *h, const char *uri,
>> int flags,
>> lower_proto = "tls";
>> ff_url_join(lower_url, sizeof(lower_url), lower_proto, NULL, hostname,
>> port,
>> NULL);
>> - av_dict_set(options, "listen", "1", 0);
>> + if ((ret = av_dict_set_int(options, "listen", s->listen, 0)) < 0)
>> + goto fail;
>> if ((ret = ffurl_open(&s->hd, lower_url, AVIO_FLAG_READ_WRITE,
>> &h->interrupt_callback, options)) < 0)
>> goto fail;
>> - if ((ret = http_read_header(h, &new_location)) < 0)
>> - goto fail;
>> - if ((ret = ffurl_write(s->hd, header, strlen(header))) < 0)
>> - goto fail;
>> - return 0;
>> -
>> + if (s->listen == 1) {
>> + // single client
>> + ret = http_handshake(h);
>> + }
>> fail:
>> - handle_http_errors(h, ret);
>> av_dict_free(&s->chained_options);
>> return ret;
>> }
>> @@ -1263,7 +1280,7 @@ static int http_shutdown(URLContext *h, int flags)
>>
>
>> /* signal end of chunked encoding if used */
>> if (((flags & AVIO_FLAG_WRITE) && s->chunked_post) ||
>> - ((flags & AVIO_FLAG_READ) && s->chunked_post && s->listen)) {
>> + ((flags & AVIO_FLAG_READ) && s->chunked_post && s->listen == 1)) {
>> ret = ffurl_write(s->hd, footer, sizeof(footer) - 1);
>> ret = ret > 0 ? 0 : ret;
>> s->end_chunked_post = 1;
>> @@ -1282,7 +1299,7 @@ static int http_close(URLContext *h)
>> av_freep(&s->inflate_buffer);
>> #endif /* CONFIG_ZLIB */
>>
>> - if (!s->end_chunked_post)
>> + if ((s->listen != 2 && !s->end_chunked_post))
>
> The two tests on s->listen seem redundant. Maybe it would be better to take
> the time of renaming a few fields in the structure to make the underlying
> logic simpler.
Those tests made sense (and I think were even necessary?) when I wrote
them. I removed them and tested both, multi-client and one-shot
server, and they appear to be closing the connections correctly.
>
> Also, the parentheses in that last line are strange.
>
>> /* Close the write direction by sending the end of chunked
>> encoding. */
>> ret = http_shutdown(h, h->flags);
>>
>> @@ -1365,6 +1382,8 @@ HTTP_CLASS(http);
>> URLProtocol ff_http_protocol = {
>> .name = "http",
>> .url_open2 = http_open,
>> + .url_accept = http_accept,
>> + .url_handshake = http_handshake,
>> .url_read = http_read,
>> .url_write = http_write,
>> .url_seek = http_seek,
>
> Regards,
>
> --
> Nicolas George
Regards,
Stephan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
