On Thu, Jun 25, 2015 at 11:12 AM, Nicolas George <geo...@nsup.org> wrote: > Le sextidi 6 messidor, an CCXXIII, Stephan Holljes a écrit : >> > * 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? > > Yes. This is more or less equivalent to the socket(), bind() and listen() > system calls, which are all instantaneous. > >> The HTTPContext will then not have any clients connected to it, which >> is different from the current behaviour. > > Exactly. > >> > 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. > > The code snippet above is an example for an application that will wish to > use the API. > >> 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, > > The threading to serve clients in parallel is for the applications. From > your point of view, you only need to make sure that the contexts returned > for separate clients are completely separate structures that do not share > any data; this is the natural thing to do anyways. > >> or why exactly lavf has to do the >> TCP-handshake manually and how. > > Not the TCP handshake, this was for explaining. The TCP handshake is handled > by the kernel: this is the reason the socket API can do with a single > accept() call. But the HTTP handshake needs to be done by the library. > >> Is the general idea to split the API at that point the right way to go? >> In my current version this is also split. > > Not exactly at that point, but yes, you probably need to split that > function. > >> 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) > > You need to have two URLContext structures: one for the server, one for the > new client. > >> +{ >> + 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; > > I do not understand the point of this code. > >> + 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); > > All this is wrong: you do not recompute the address and lower-protocol on a > per-client basis. > >> + 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); >> + } > > Not here! The whole point of the multi-client API is to separate accept() > from open()! > >> } >> 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); >> +} > > This looks correct at first glance. > >> + >> 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) > > Again: you need two context structures. > >> +{ >> + 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. >> * > > Regards, > > -- > Nicolas George > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Thanks, I understand the datastructures and their interaction a lot better now. I discussed it with a friend yesterday too and there a lot of the things started to make more sense. I'm currently working on the implementation, when questions arise I will ask again. Regards, Stephan _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
