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 <geo...@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 <klaxa1...@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 <klaxa1...@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
>>
>
From 280162f00ef407b3c85a332f45825bbdd558b32d Mon Sep 17 00:00:00 2001
From: Stephan Holljes <klaxa1...@googlemail.com>
Date: Wed, 24 Jun 2015 07:27:40 +0200
Subject: [PATCH] lavf: (Incomplete) avio_accept
Signed-off-by: Stephan Holljes <klaxa1...@googlemail.com>
---
libavformat/avio.c | 15 +++++++++++++++
libavformat/aviobuf.c | 12 ++++++++++++
libavformat/http.c | 29 ++++++++++++++++++++++++++++-
libavformat/network.c | 22 +++++++++++++++++-----
libavformat/network.h | 5 +++++
libavformat/tcp.c | 6 ++++++
libavformat/url.h | 6 ++++++
7 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/libavformat/avio.c b/libavformat/avio.c
index bd32944..e67588d 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -211,6 +211,21 @@ int ffurl_connect(URLContext *uc, AVDictionary **options)
return 0;
}
+int ffurl_accept(URLContext *uc)
+{
+ int err = uc->prot->url_accept(uc);
+ if (err)
+ return err;
+ uc->is_connected = 1;
+ /* We must be careful here as ffurl_seek() could be slow,
+ * for example for http */
+ if ((uc->flags & AVIO_FLAG_WRITE) || !strcmp(uc->prot->name, "file"))
+ if (!uc->is_streamed && ffurl_seek(uc, 0, SEEK_SET) < 0)
+ uc->is_streamed = 1;
+ return 0;
+
+}
+
#define URL_SCHEME_CHARS \
"abcdefghijklmnopqrstuvwxyz" \
"ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index ff85081..84e85ff 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -932,6 +932,18 @@ int avio_open2(AVIOContext **s, const char *filename, int flags,
return 0;
}
+/*int avio_accept(AVIOContext *s, AVIOContext **c, char *filename, int flags,
+ const AVIOInterruptCB *int_cb, AVDictionary **options)
+{
+ URLContext *h;
+ int err;
+
+ err = ffurl_open(&h, filename, flags, int_cb, options);
+ if (err < 0)
+ return err;
+ err = ffio_fdopen(s, h);
+} */
+
int ffio_open2_wrapper(struct AVFormatContext *s, AVIOContext **pb, const char *url, int flags,
const AVIOInterruptCB *int_cb, AVDictionary **options)
{
diff --git a/libavformat/http.c b/libavformat/http.c
index 676bfd5..14a639a 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -348,6 +348,27 @@ fail:
return ret;
}
+static int http_accept(URLContext *h, const char *uri) {
+ 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;
+ s->chunked_post = 1;
+ av_url_split(proto, sizeof(proto), NULL, 0, hostname, sizeof(hostname), &port,
+ NULL, 0, uri);
+ if (!strcmp(proto, "https"))
+ lower_proto = "tls";
+ ff_url_join(lower_url, sizeof(lower_url), lower_proto, NULL, hostname, port,
+ NULL);
+ if ((ret = ffurl_accept(&s->hd)) < 0)
+ return ret;
+
+
+}
+
static int http_open(URLContext *h, const char *uri, int flags,
AVDictionary **options)
{
@@ -374,7 +395,12 @@ static int http_open(URLContext *h, const char *uri, int flags,
}
if (s->listen) {
- return http_listen(h, uri, flags, options);
+ int ret;
+ if ((ret = http_listen(h, uri, flags, options)) < 0)
+ return ret;
+ while (1) {
+ http_accept(h);
+ }
}
ret = http_open_cnx(h, options);
if (ret < 0)
@@ -1477,6 +1503,7 @@ 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_read = http_buf_read,
.url_write = http_proxy_write,
.url_close = http_proxy_close,
diff --git a/libavformat/network.c b/libavformat/network.c
index 47ade8c..381aafc 100644
--- a/libavformat/network.c
+++ b/libavformat/network.c
@@ -187,12 +187,11 @@ int ff_socket(int af, int type, int proto)
return fd;
}
-int ff_listen_bind(int fd, const struct sockaddr *addr,
- socklen_t addrlen, int timeout, URLContext *h)
+int ff_listen(int fd, const struct sockaddr *addr,
+ socklen_t addrlen, int timeout, URLContext *h)
{
int ret;
int reuse = 1;
- struct pollfd lp = { fd, POLLIN, 0 };
if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(reuse))) {
av_log(NULL, AV_LOG_WARNING, "setsockopt(SO_REUSEADDR) failed\n");
}
@@ -203,7 +202,13 @@ int ff_listen_bind(int fd, const struct sockaddr *addr,
ret = listen(fd, 1);
if (ret)
return ff_neterrno();
+ return ret;
+}
+int ff_accept(int fd, int timeout, URLContext *h)
+{
+ int ret;
+ struct pollfd lp = { fd, POLLIN, 0 };
ret = ff_poll_interrupt(&lp, 1, timeout, &h->interrupt_callback);
if (ret < 0)
return ret;
@@ -212,14 +217,21 @@ int ff_listen_bind(int fd, const struct sockaddr *addr,
if (ret < 0)
return ff_neterrno();
- closesocket(fd);
-
if (ff_socket_nonblock(ret, 1) < 0)
av_log(NULL, AV_LOG_DEBUG, "ff_socket_nonblock failed\n");
return ret;
}
+int ff_listen_bind(int fd, const struct sockaddr *addr,
+ socklen_t addrlen, int timeout, URLContext *h)
+{
+ int ret;
+ if ((ret = ff_listen(fd, addr, addrlen, timeout, h)) < 0)
+ return ret;
+ return ff_accept(fd, timeout, h);
+}
+
int ff_listen_connect(int fd, const struct sockaddr *addr,
socklen_t addrlen, int timeout, URLContext *h,
int will_try_next)
diff --git a/libavformat/network.h b/libavformat/network.h
index 86fb656..d090e8b 100644
--- a/libavformat/network.h
+++ b/libavformat/network.h
@@ -238,6 +238,11 @@ int ff_is_multicast_address(struct sockaddr *addr);
#define POLLING_TIME 100 /// Time in milliseconds between interrupt check
+int ff_listen(int fd, const struct sockaddr *addr,
+ socklen_t addrlen, int timeout,
+ URLContext *h);
+int ff_accept(int fd, int timeout, URLContext *h);
+
/**
* Bind to a file descriptor and poll for a connection.
*
diff --git a/libavformat/tcp.c b/libavformat/tcp.c
index f24cad2..a3392aa 100644
--- a/libavformat/tcp.c
+++ b/libavformat/tcp.c
@@ -163,6 +163,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, s->listen_timeout, h);
+}
+
static int tcp_read(URLContext *h, uint8_t *buf, int size)
{
TCPContext *s = h->priv_data;
diff --git a/libavformat/url.h b/libavformat/url.h
index 99a3201..714858e 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);
/**
* Read data from the protocol.
@@ -140,6 +141,11 @@ int ffurl_open(URLContext **puc, const char *filename, int flags,
const AVIOInterruptCB *int_cb, AVDictionary **options);
/**
+ * Accept a client socket on the URLContext
+ */
+int ffurl_accept(URLContext *h);
+
+/**
* Read up to size bytes from the resource accessed by h, and store
* the read bytes in buf.
*
--
2.1.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
