[FFmpeg-devel] [PATCH 7/9] lavf/http: increase range for listen, handle connection closing accordingly, add http_handshake and move handshake logic there

[Nicolas George]

Le quintidi 15 messidor, an CCXXIII, Stephan Holljes a écrit :
> Signed-off-by: Stephan Holljes <klaxa1337 at 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.

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150703/79811f7a/attachment.asc>