[FFmpeg-devel] [PATCH 9/9] Add http multi-client example code

2015-07-02 Thread Stephan Holljes
Signed-off-by: Stephan Holljes 
---
 doc/examples/Makefile   |   1 +
 doc/examples/http_multiclient.c | 101 
 2 files changed, 102 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 000..fdecab4
--- /dev/null
+++ b/doc/examples/http_multiclient.c
@@ -0,0 +1,101 @@
+/*
+ * copyright (c) 2015 Stephan Holljes
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * libavformat multi-client network API usage example.
+ *
+ * @example http_multiclient.c
+ * This example will serve a file without decoding or demuxing it over http.
+ * Multiple clients can connect and will receive the same file.
+ */
+
+#include 
+#include 
+
+int main(int argc, char **argv)
+{
+AVDictionary *options = NULL;
+AVIOContext *input = NULL, *client = NULL, *server = NULL;
+const char *in_uri, *out_uri;
+int ret, pid, n;
+uint8_t buf[1024];
+
+if (argc < 3) {
+printf("usage: %s input http://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_uri = argv[1];
+out_uri = argv[2];
+
+av_register_all();
+avformat_network_init();
+
+if ((ret = av_dict_set(&options, "listen", "2", 0)) < 0)
+goto end;
+if ((ret = avio_open2(&server, out_uri, AVIO_FLAG_READ_WRITE, NULL, 
&options)) < 0)
+goto end;
+fprintf(stderr, "Entering main loop.\n");
+for(;;) {
+if ((ret = avio_accept(server, &client)) < 0)
+goto end;
+fprintf(stderr, "Accepted client, forking process.\n");
+pid = fork();
+if (pid == 0) {
+fprintf(stderr, "Opening input file.\n");
+if ((ret = avio_open2(&input, in_uri, AVIO_FLAG_READ, NULL, NULL)) 
< 0)
+goto end;
+fprintf(stderr, "In child.\n");
+avio_handshake(client);
+fprintf(stderr, "Handshake performed.\n");
+for(;;) {
+n = avio_read(input, buf, sizeof(buf));
+fprintf(stderr, "Read %d bytes from input.\n", n);
+if (n <= 0) {
+fprintf(stderr, "n is negative, abort.\n");
+break;
+}
+fprintf(stderr, "Writing %d bytes of data.\n", n);
+avio_write(client, buf, n);
+avio_flush(client);
+}
+fprintf(stderr, "Flushing client\n");
+avio_flush(client);
+fprintf(stderr, "Closing client\n");
+avio_close(client);
+break;
+}
+}
+
+end:
+if (ret)
+fprintf(stderr, "Some errors occured.\n");
+if (server)
+avio_close(server);
+if (input)
+avio_close(input);
+if (ret < 0 && ret != AVERROR_EOF)
+return 1;
+return 0;
+}
-- 
2.1.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 9/9] Add http multi-client example code

2015-07-03 Thread Nicolas George
Le quintidi 15 messidor, an CCXXIII, Stephan Holljes a écrit :
> Signed-off-by: Stephan Holljes 
> ---
>  doc/examples/Makefile   |   1 +
>  doc/examples/http_multiclient.c | 101 
> 
>  2 files changed, 102 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 000..fdecab4
> --- /dev/null
> +++ b/doc/examples/http_multiclient.c
> @@ -0,0 +1,101 @@
> +/*
> + * copyright (c) 2015 Stephan Holljes
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +/**
> + * @file
> + * libavformat multi-client network API usage example.
> + *
> + * @example http_multiclient.c
> + * This example will serve a file without decoding or demuxing it over http.

> + * Multiple clients can connect and will receive the same file.

Did you test several simultaneous clients? It should work, because fork() is
a strong isolation, so it if works for one it works for many, but testing is
always better.

> + */
> +
> +#include 
> +#include 
> +
> +int main(int argc, char **argv)
> +{
> +AVDictionary *options = NULL;
> +AVIOContext *input = NULL, *client = NULL, *server = NULL;
> +const char *in_uri, *out_uri;
> +int ret, pid, n;
> +uint8_t buf[1024];
> +
> +if (argc < 3) {
> +printf("usage: %s input http://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"

I suspect this sentence is outdated.

> +   "\n", argv[0]);
> +return 1;
> +}
> +
> +in_uri = argv[1];
> +out_uri = argv[2];
> +
> +av_register_all();
> +avformat_network_init();
> +
> +if ((ret = av_dict_set(&options, "listen", "2", 0)) < 0)
> +goto end;
> +if ((ret = avio_open2(&server, out_uri, AVIO_FLAG_READ_WRITE, NULL, 
> &options)) < 0)
> +goto end;
> +fprintf(stderr, "Entering main loop.\n");
> +for(;;) {
> +if ((ret = avio_accept(server, &client)) < 0)
> +goto end;
> +fprintf(stderr, "Accepted client, forking process.\n");

> +pid = fork();

Zombie apocalypse ahead!

Well, setting SIGCHLD to SIG_IGN is hardly portable, double-fork is ugly and
proper waiting is painful, so I guess we can leave the zombies for now in a
simple testing program, but a comment stating it would be useful.

> +if (pid == 0) {

Maybe also:

if (pid < 0) {
perror("Fork failed");
err = AVERROR(errno);
goto end;
}

> +fprintf(stderr, "Opening input file.\n");
> +if ((ret = avio_open2(&input, in_uri, AVIO_FLAG_READ, NULL, 
> NULL)) < 0)
> +goto end;
> +fprintf(stderr, "In child.\n");
> +avio_handshake(client);
> +fprintf(stderr, "Handshake performed.\n");

It would feel more natural to perform the handshake before opening the input
file. Note that since this is your code, you are free to reject this kind of
stylistic remark. In this particular case, it will be necessary when
extending the code, because avio_handshake() will be responsible for getting
the request file name from the client.

Also: since this is fork()ing instead of pthread_create()ing, you probably
should close the server context in the client process. And you definitely
must close the client context in the server process.

> +for(;;) {
> +n = avio_read(input, buf, sizeof(buf));
> +  

Re: [FFmpeg-devel] [PATCH 9/9] Add http multi-client example code

2015-07-03 Thread Stephan Holljes
On Fri, Jul 3, 2015 at 12:32 PM, Nicolas George  wrote:
> Le quintidi 15 messidor, an CCXXIII, Stephan Holljes a écrit :
>> Signed-off-by: Stephan Holljes 
>> ---
>>  doc/examples/Makefile   |   1 +
>>  doc/examples/http_multiclient.c | 101 
>> 
>>  2 files changed, 102 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 000..fdecab4
>> --- /dev/null
>> +++ b/doc/examples/http_multiclient.c
>> @@ -0,0 +1,101 @@
>> +/*
>> + * copyright (c) 2015 Stephan Holljes
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
>> USA
>> + */
>> +
>> +/**
>> + * @file
>> + * libavformat multi-client network API usage example.
>> + *
>> + * @example http_multiclient.c
>> + * This example will serve a file without decoding or demuxing it over http.
>
>> + * Multiple clients can connect and will receive the same file.
>
> Did you test several simultaneous clients? It should work, because fork() is
> a strong isolation, so it if works for one it works for many, but testing is
> always better.

Yes, I tested it with multiple simultaneous wget instances and with
siege (a benchmark tool for webservers).

>
>> + */
>> +
>> +#include 
>> +#include 
>> +
>> +int main(int argc, char **argv)
>> +{
>> +AVDictionary *options = NULL;
>> +AVIOContext *input = NULL, *client = NULL, *server = NULL;
>> +const char *in_uri, *out_uri;
>> +int ret, pid, n;
>> +uint8_t buf[1024];
>> +
>> +if (argc < 3) {
>> +printf("usage: %s input http://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"
>
> I suspect this sentence is outdated.
>
>> +   "\n", argv[0]);
>> +return 1;
>> +}
>> +
>> +in_uri = argv[1];
>> +out_uri = argv[2];
>> +
>> +av_register_all();
>> +avformat_network_init();
>> +
>> +if ((ret = av_dict_set(&options, "listen", "2", 0)) < 0)
>> +goto end;
>> +if ((ret = avio_open2(&server, out_uri, AVIO_FLAG_READ_WRITE, NULL, 
>> &options)) < 0)
>> +goto end;
>> +fprintf(stderr, "Entering main loop.\n");
>> +for(;;) {
>> +if ((ret = avio_accept(server, &client)) < 0)
>> +goto end;
>> +fprintf(stderr, "Accepted client, forking process.\n");
>
>> +pid = fork();
>
> Zombie apocalypse ahead!
>
> Well, setting SIGCHLD to SIG_IGN is hardly portable, double-fork is ugly and
> proper waiting is painful, so I guess we can leave the zombies for now in a
> simple testing program, but a comment stating it would be useful.
>
>> +if (pid == 0) {
>
> Maybe also:
>
> if (pid < 0) {
> perror("Fork failed");
> err = AVERROR(errno);
> goto end;
> }
>
>> +fprintf(stderr, "Opening input file.\n");
>> +if ((ret = avio_open2(&input, in_uri, AVIO_FLAG_READ, NULL, 
>> NULL)) < 0)
>> +goto end;
>> +fprintf(stderr, "In child.\n");
>> +avio_handshake(client);
>> +fprintf(stderr, "Handshake performed.\n");
>
> It would feel more natural to perform the handshake before opening the input
> file. Note that since this is your code, you are free to reject this kind of
> stylistic remark. In this particular case, it will be necessary when
> extending the code, because avio_handshake() will be responsible for getting
> the request

Re: [FFmpeg-devel] [PATCH 9/9] Add http multi-client example code

2015-07-06 Thread Nicolas George
Le sextidi 16 messidor, an CCXXIII, Stephan Holljes a écrit :
> Yes, I tested it with multiple simultaneous wget instances and with
> siege (a benchmark tool for webservers).

Good. Although you should also test manually, with socat / nc / telnet.

> I see that I missed closing the client context in the server process,
> but I think the server context is being closed when the client process
> reaches the end label.

Yes, but that is too late: imagine you want to restart the server while some
clients are still alive. It is good practice to close file descriptors as
soon as possible, and with forking servers it is especially important to
follow that practice.

Of course, with example code, following good practices can be dispensed
with, but if it does not make the code more complex there is no reason to.

In fact, I believe you should move all client-related code in a separate
function:

void client(AVIOContext *client)
{
avio_handshake(client);
...
}

if (pid == 0) {
avio_closep(&server);
exit(0);
}

That way, changing it into a thread (less isolation) or a separate program
(fork+exec, more isolation) becomes trivial.

This makes sharing the error reporting code less straightforward, but
sharing the error reporting is always awkward: sometimes you need context,
sometime you do not, some you need more than context, etc. IMHO, with the
simplistic error system in FFmpeg, it is best to put the error message at
the place of the error, even if that means more av_log() and more braces.

Unrelated: when you resend a full patch series, try to remember to annotate
the patches with a few words to say what changed since last version. The
lines between the first "---" and the first "diff --git" where Git puts
ASCII-art histograms are purely informational and can be used for that. You
can use "git send-email --annotate" to get an editor to edit them.

Personally, I always use --annotate because it gives me an extra chance to
re-read the patch series.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel