[FFmpeg-devel] GSoC update
Stephan Holljes
klaxa1337 at googlemail.com
Wed Jun 24 07:36:59 CEST 2015
Thank you for the detailed reply. It has helped me understand the
structures a lot better than before.
Comments inline.
On Tue, Jun 23, 2015 at 5:25 PM, Nicolas George <george at nsup.org> wrote:
> This has better go to the mailing list.
>
> Le quintidi 5 messidor, an CCXXIII, Stephan Holljes a écrit :
>> So far I have split ff_listen_bind() into ff_listen_bind() and
>> ff_accept() and added an url_accept field to URLProtocol (see attached
>> patch).
>
> I do not believe this is the correct approach. ff_listen_bind() is an
> internal helper that works specifically for sockets.
>
>> I read our past exchanges for a number of times and I am somewhat at a
>> loss as to where to allocate my additional Client(-Contexts).
>> No sourcefile I have looked at stores multiple AVIOContexts or
>> AVFormatContexts (or I have not seen it).
>> I also could not find out how ffmpeg would even accept multiple
>> clients. The ff_listen_bind() call is blocking on accept() and it is
>> only called once.
>> Maybe I'm also overlooking some documentation. Maybe I am also looking
>> in the wrong place. The files I have looked at the most have been:
>> url.h network.c/h, avio.c/h aviobuf.c and tcp.c.
>
> You need to understand very well how it happens for plain Unix TCP clients.
>
> First, the server create and prepares the server socket s, with socket(),
> bind() and mostly listen(). There is also a list of client sockets c[i],
> initially empty.
>
> Then, indefinitely and in parallel:
>
> * call accept() on s: this returns a new c[i] added to the list;
>
> * perform read() / write() on each c[i], remove them from the list if they
> are finished.
>
> Here, "in parallel" means either threads, forked processes, or emulated
> parallelism with poll() and non-blocking operations. For lavf, non-blocking
> operations do not work, so you can assume parallelism between multiple
> clients is achieved with threads; although I very much would like that all
> new networking code is input-driven and therefore ready for non-blocking.
>
> Now, as for how to transpose it into lavf terms:
>
> * Creating and preparing the server socket: avio_open2() with various
> options for the address and the "listen" option set.
Am I correct to understand that this has to return immediately,
because if avio_open2() does not return, I have no (server)
AVIOContext to call avio_accept() with?
The HTTPContext will then not have any clients connected to it, which
is different from the current behaviour.
>
> * Accepting a new client: a new function that you have to design, probably
> avio_accept().
>
> There are a few caveats for avio_accept():
>
> * Creating a new AVFormatContext, like accept() creates a new socket. That
> is not really difficult, IMHO. It just means the API will look somewhat
> like that:
>
> int avio_accept(AVIOContext *s, AVIOContext **c, ...);
>
> The c parameter is just like the s parameter to avio_open2(): it allocates
> a new context if necessary and populates it.
>
> * Except, the current API accepts a single client by replacing the server
> context. This must still be possible after the change. Maybe just
> overwriting the server context will work, but it must be considered.
>
> * If you remember, a full TCP accept() requires three handshake packets: SYN
> from the client, ACK-SYN from the server and then ACK from the client. The
> kernel handles this in the background and accept() returns a new client
> only when the full handshake is done. In lavf, we do not have a kernel
> working in the background, so it must be explicit.
>
> We can look at how non-blocking connect() works: first call connect() on
> the socket, it returns EINPROGRESS immediately, the repeatedly call
> getsockopt(SO_ERROR) to check the status of the connection until it stops
> returning EAGAIN.
>
> I believe we can do the same: avio_accept(s, &c) to initiate the accept,
> and then avio_accept_connect(c) or something (feel free to suggest better
> names) to finish the handshake.
>
> For the particular case of the HTTP server, the avio_accept() part would
> call avio_accept() on the underlying socket (or TLS context), and the
> avio_accept_connect() would call avio_accept_connect() on the underlying
> context and then perform the handshake.
>
> In terms of application, the code would probably look something like this
> (error checks and stuff removed):
>
> av_dict_set(&options, "listen", "1");
> avio_open2(&server, "http://:8080", &options);
> while (1) {
> avio_accept(server, &client);
> thread_run {
> avio_accept_connect(client);
> communicate_with_the_client(client);
> avio_close(client);
> };
> }
Would this still be in http.c? If my thinking is correct, this should
be in http_open() then, but only if http_listen() returns without
accepting an initial client.
>
> Hope this helps.
>
> Regards,
>
> --
> Nicolas George
>
A lot of this I still can't wrap my head around. I am still not sure
about a lot of things, for example how and where to use threading to
serve clients in parallel, or why exactly lavf has to do the
TCP-handshake manually and how.
I have spent the whole day doing nothing but analyze how this is
supposed to work, but now I have no energy left. I have attached a new
patch that is by far not complete (it does not even compile), but I
think it explains what I have been thinking more precise than I can
write it down. I also think it will make it easier for you (or anyone
else) to point out where my mistakes in my thought-process are.
Thanks again for taking all this time to provide me with feedback and
information.
Regards,
Stephan
>> From 234199a18e0c3bfede5f04a9ade0a11c71061285 Mon Sep 17 00:00:00 2001
>> From: Stephan Holljes <klaxa1337 at googlemail.com>
>> Date: Tue, 23 Jun 2015 16:41:51 +0200
>> Subject: [PATCH] lavf: Split ff_listen_bind into ff_listen_bind and ff_accept
>> and implement its usage in tcp.
>>
>> Signed-off-by: Stephan Holljes <klaxa1337 at googlemail.com>
>> ---
>> libavformat/network.c | 7 ++++---
>> libavformat/network.h | 12 +++++++++++-
>> libavformat/tcp.c | 10 ++++++++++
>> libavformat/url.h | 1 +
>> 4 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavformat/network.c b/libavformat/network.c
>> index 47ade8c..8d9434f 100644
>> --- a/libavformat/network.c
>> +++ b/libavformat/network.c
>
>> @@ -204,10 +204,11 @@ int ff_listen_bind(int fd, const struct sockaddr *addr,
>> if (ret)
>> return ff_neterrno();
>>
>> - ret = ff_poll_interrupt(&lp, 1, timeout, &h->interrupt_callback);
>> - if (ret < 0)
>> - return ret;
>> + return ff_poll_interrupt(&lp, 1, timeout, &h->interrupt_callback);
>> +}
>
> ff_poll_interrupt() belongs in ff_accept(), not ff_listen_bind(), since it
> is blocking and required for each client.
Is the general idea to split the API at that point the right way to go?
In my current version this is also split.
>
>>
>> +int ff_accept(int fd) {
>> + int ret;
>> ret = accept(fd, NULL, NULL);
>> if (ret < 0)
>> return ff_neterrno();
>> diff --git a/libavformat/network.h b/libavformat/network.h
>> index 86fb656..810319d 100644
>> --- a/libavformat/network.h
>> +++ b/libavformat/network.h
>
>> @@ -247,7 +247,7 @@ int ff_is_multicast_address(struct sockaddr *addr);
>> * @param timeout Polling timeout in milliseconds.
>> * @param h URLContext providing interrupt check
>> * callback and logging context.
>> - * @return A non-blocking file descriptor on success
>> + * @return 0 on success
>> * or an AVERROR on failure.
>> */
>
> This looks strange.
>
>> int ff_listen_bind(int fd, const struct sockaddr *addr,
>> @@ -255,6 +255,16 @@ int ff_listen_bind(int fd, const struct sockaddr *addr,
>> URLContext *h);
>>
>> /**
>> + * Accept a filedescriptor on a server socket.
>> + *
>> + * @param fd Server socket filedescriptor.
>> + *
>> + * @return A non-blocking file descriptor on success
>> + * or an AVERROR on failure.
>> + */
>> +int ff_accept(int fd);
>> +
>> +/**
>> * Connect to a file descriptor and poll for result.
>> *
>> * @param fd First argument of connect(),
>> diff --git a/libavformat/tcp.c b/libavformat/tcp.c
>> index f24cad2..168e5c5 100644
>> --- a/libavformat/tcp.c
>> +++ b/libavformat/tcp.c
>> @@ -130,6 +130,9 @@ static int tcp_open(URLContext *h, const char *uri, int flags)
>> s->listen_timeout, h)) < 0) {
>> goto fail1;
>> }
>> + if ((ret = ff_accept(fd)) < 0) {
>> + goto fail;
>> + }
>> fd = ret;
>> } else {
>> if ((ret = ff_listen_connect(fd, cur_ai->ai_addr, cur_ai->ai_addrlen,
>> @@ -163,6 +166,12 @@ static int tcp_open(URLContext *h, const char *uri, int flags)
>> return ret;
>> }
>>
>> +static int tcp_accept(URLContext *h)
>> +{
>> + TCPContext *s = h->priv_data;
>> + return ff_accept(s->fd);
>> +}
>> +
>> static int tcp_read(URLContext *h, uint8_t *buf, int size)
>> {
>> TCPContext *s = h->priv_data;
>> @@ -223,6 +232,7 @@ static int tcp_get_file_handle(URLContext *h)
>> URLProtocol ff_tcp_protocol = {
>> .name = "tcp",
>> .url_open = tcp_open,
>> + .url_accept = tcp_accept,
>> .url_read = tcp_read,
>> .url_write = tcp_write,
>> .url_close = tcp_close,
>> diff --git a/libavformat/url.h b/libavformat/url.h
>> index 99a3201..5c82117 100644
>> --- a/libavformat/url.h
>> +++ b/libavformat/url.h
>> @@ -58,6 +58,7 @@ typedef struct URLProtocol {
>> * for those nested protocols.
>> */
>
>> int (*url_open2)(URLContext *h, const char *url, int flags, AVDictionary **options);
>> + int (*url_accept)(URLContext *h);
>
> If you do never access this field, there is no use to adding it yet.
>
>>
>> /**
>> * Read data from the protocol.
>> --
>> 2.1.0
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavf-Incomplete-avio_accept.patch
Type: text/x-patch
Size: 7653 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150624/1f4c709d/attachment.bin>
More information about the ffmpeg-devel
mailing list