[FFmpeg-devel] GSoC update

Nicolas George george at nsup.org
Sun Jun 28 22:06:01 CEST 2015


Le decadi 10 messidor, an CCXXIII, Stephan Holljes a écrit :
> Hi,
> attached patches are the current state of work.
> It's probably still a bit rough around the edges, but I think I made
> some progress.
> The sample code in doc/examples does not write any data as of yet, but
> the HTTP handshake works.

A few quick remarks, without entering into the fine details since the patch
series will likely evolve still quite a bit.

> From b43aeaa27f6ca7df476aa194b2f78aa1b49516d0 Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:06:16 +0200
> Subject: [PATCH 01/10] lavf/network: split ff_listen_bind into ff_listen and
>  ff_accept
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/network.c | 27 +++++++++++++++++++++------
>  libavformat/network.h |  4 ++++
>  2 files changed, 25 insertions(+), 6 deletions(-)

This one looks good to me, except a small doxy would be nice for the two new
functions.

> From 39faa1ea315bb51452446e291fd5d93d7eb3a988 Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:12:37 +0200
> Subject: [PATCH 02/10] lavf/avio: add ffurl_accept and ffurl_handshake
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/avio.c | 15 +++++++++++++++
>  libavformat/url.h  | 12 ++++++++++++
>  2 files changed, 27 insertions(+)

ffurl_accept() requires two arguments, but I suspect ffurl_handshake() only
requires one, the client. A doxy would be nice for ffurl_handshake().

Also, the client argument of ffurl_accept() and url_accept() should probably
a pointer to pointer, like ffurl_alloc(), so that it can allocate the
context itself.

> From 8b473ba9acffecf98f8252eeccb413bcfbbf38c5 Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:13:36 +0200
> Subject: [PATCH 03/10] lavf/avio: add avio_accept and avio_handshake
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/avio.h    |  2 ++
>  libavformat/aviobuf.c | 21 +++++++++++++++++++++
>  2 files changed, 23 insertions(+)

That part looks mostly good.

> From 8ba3d1ef528cdd9209764b0f696b8df81ea46870 Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:16:02 +0200
> Subject: [PATCH 04/10] lavf/http: add http_accept and http_handshake
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/http.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)

I believe this patch can not come before the one for TCP.

> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 676bfd5..7219f08 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -382,6 +382,38 @@ static int http_open(URLContext *h, const char *uri, int flags,
>      return ret;
>  }
>  
> +static int http_accept(URLContext *s, URLContext *c)
> +{
> +    int ret;
> +    HTTPContext *sh = s->priv_data;
> +    HTTPContext *ch = c->priv_data;
> +    URLContext *sl = sh->hd;
> +    URLContext *cl;

> +    if ((ret = ffurl_alloc(&cl, sl->filename, AVIO_FLAG_READ_WRITE, &sl->interrupt_callback)) < 0)
> +        goto fail;
> +    if ((ret = ffurl_accept(sl, cl)) < 0)
> +        goto fail;
> +    ch->hd = cl;
> +fail:
> +    return ret;
> +}

This looks mostly correct, but I suspect it would be more convenient to make
url_accept() responsible for allocating the client context. Otherwise, the
application is responsible for allocating a client context with the correct
protocol and settings, this is fragile.

> +
> +static int http_handshake(URLContext *s, URLContext *c) {
> +    int ret, err, new_location;
> +    HTTPContext *sh = s->priv_data;
> +    HTTPContext *ch = c->priv_data;
> +    URLContext *sl = sh->hd;
> +    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 ((err = http_read_header(c, &new_location)) < 0)
> +        goto fail;
> +    if ((ret = ffurl_write(cl, header, strlen(header))) < 0)
> +        goto fail;
> +fail:
> +    handle_http_errors(c, err);
> +    return ret;
> +}

As you can see, the s argument is never used.

Also, it should probably call ffurl_handshake() on the underlying socket:
for TCP it is a nop, but for TLS, for example, it is blocking.

> +
>  static int http_getc(HTTPContext *s)
>  {
>      int len;
> @@ -1346,6 +1378,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,
> @@ -1364,6 +1398,8 @@ HTTP_CLASS(https);
>  URLProtocol ff_https_protocol = {
>      .name                = "https",
>      .url_open2           = http_open,
> +    .url_accept          = http_accept,
> +    .url_handshake       = http_handshake,
>      .url_read            = http_read,
>      .url_write           = http_write,
>      .url_seek            = http_seek,
> @@ -1477,6 +1513,8 @@ static int http_proxy_write(URLContext *h, const uint8_t *buf, int size)
>  URLProtocol ff_httpproxy_protocol = {
>      .name                = "httpproxy",
>      .url_open            = http_proxy_open,
> +    .url_accept          = http_accept,
> +    .url_handshake       = http_handshake,
>      .url_read            = http_buf_read,
>      .url_write           = http_proxy_write,
>      .url_close           = http_proxy_close,
> -- 
> 2.1.0
> 

> From cfd27b21cf9fae39d881608a3ba379e6fb75848c Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:17:12 +0200
> Subject: [PATCH 05/10] lavf/tcp: make tcp_open return with a listening socket
>  without calling accept()
> 
> ---
>  libavformat/tcp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

This would break existing code. You suggested using a different value for
listen, what happened to that idea?

> From dd197651d205b2dece97798e933974ecef3a2b7f Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:20:17 +0200
> Subject: [PATCH 06/10] lavf/tcp: add tcp_accept
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/tcp.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

No extra remark for that patch, but the suggestion of having url_accept()
responsible for allocating the client context applies here too.

> From 5ab3661637c1ba571bc7f7bf365e3f3c8bc4ae89 Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:25:35 +0200
> Subject: [PATCH 07/10] lavf/http: remove connection logic from http_listen()
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/http.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)

No remark for this for now.

> From b835a248bba5004ad8f8a598992fa959881b2376 Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:26:43 +0200
> Subject: [PATCH 08/10] lavf/http: ignore 0 in handle_http_errors()
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/http.c | 2 ++
>  1 file changed, 2 insertions(+)

It should probably be merged with the patch that makes it necessary.

> From 127b0ef456d203bc295ef017737019c0f8329515 Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:39:13 +0200
> Subject: [PATCH 09/10] lavf/http: only shut down the connection when it's a
>  client.
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  libavformat/http.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Ditto.

> From df4b466693b8d8627cca59a17d9e7ab5fd5e843e Mon Sep 17 00:00:00 2001
> From: Stephan Holljes <klaxa1337 at googlemail.com>
> Date: Sun, 28 Jun 2015 06:39:56 +0200
> Subject: [PATCH 10/10] doc/examples: WIP: add http_multiclient example code.
> 
> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
> ---
>  doc/examples/Makefile           |  1 +
>  doc/examples/http_multiclient.c | 60 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>  create mode 100644 doc/examples/http_multiclient.c
> 
> diff --git a/doc/examples/Makefile b/doc/examples/Makefile
> index 9699f11..8c9501b 100644
> --- a/doc/examples/Makefile
> +++ b/doc/examples/Makefile
> @@ -18,6 +18,7 @@ EXAMPLES=       avio_list_dir                      \
>                  extract_mvs                        \
>                  filtering_video                    \
>                  filtering_audio                    \
> +                http_multiclient                   \
>                  metadata                           \
>                  muxing                             \
>                  remuxing                           \
> diff --git a/doc/examples/http_multiclient.c b/doc/examples/http_multiclient.c
> new file mode 100644
> index 0000000..215a8bb
> --- /dev/null
> +++ b/doc/examples/http_multiclient.c
> @@ -0,0 +1,60 @@
> +#include <libavformat/avformat.h>
> +
> +int main(int argc, char **argv)
> +{

> +    AVOutputFormat *ofmt = NULL;
> +    AVFormatContext *ifmt_ctx = NULL, *ofmt_ctx = NULL;

You could probably dispense with the muxers and demuxers and work directly
with bytes. That would make the code much simpler.

> +    AVDictionary *options = NULL;
> +    AVPacket pkt;
> +    AVIOContext *c = NULL;
> +    const char *in_filename, *out_uri;
> +    int ret;
> +
> +    if (argc < 3) {
> +        printf("usage: %s input http[s]://hostname[:port]\n"
> +               "API example program to serve http to multiple clients.\n"
> +               "The output format is guessed according to the input file extension.\n"
> +               "\n", argv[0]);
> +        return 1;
> +    }
> +
> +    in_filename = argv[1];
> +    out_uri = argv[2];
> +
> +    av_register_all();
> +    avformat_network_init();
> +
> +    if ((ret = avformat_open_input(&ifmt_ctx, in_filename, 0, 0)) < 0) {
> +        fprintf(stderr, "Could not open input file '%s'", in_filename);
> +        goto end;
> +    }
> +
> +    if ((ret = avformat_find_stream_info(ifmt_ctx, 0)) < 0) {
> +        fprintf(stderr, "Failed to retrieve input stream information");
> +        goto end;
> +    }
> +
> +    avformat_alloc_output_context2(&ofmt_ctx, NULL, ifmt_ctx->iformat->name, out_uri);
> +    if (!ofmt_ctx) {
> +        fprintf(stderr, "Could not create output context\n");
> +        ret = AVERROR_UNKNOWN;
> +        goto end;
> +    }
> +
> +    ofmt = ofmt_ctx->oformat;
> +    av_dict_set(&options, "listen", "1", 0);
> +    ret = avio_open2(&ofmt_ctx->pb, out_uri, AVIO_FLAG_READ_WRITE, NULL, &options);

> +    avio_accept(ofmt_ctx->pb, &c);
> +    avio_handshake(ofmt_ctx->pb, c);
> +    avio_close(c);

To test the multi-client API, you need to accept several clients. You can
probably just put that in a while(1) loop, but it would be better to fork a
thread for each client.

> +
> +end:
> +    avformat_close_input(&ifmt_ctx);
> +
> +    if (ofmt_ctx)
> +        avio_closep(&ofmt_ctx->pb);
> +    avformat_free_context(ofmt_ctx);
> +    if (ret < 0 && ret != AVERROR_EOF)
> +        return 1;
> +    return 0;
> +}

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/20150628/9cc0b36d/attachment.asc>


More information about the ffmpeg-devel mailing list