Re: [FFmpeg-devel] [PATCH 1/1] avformat/tcp: support timeout for getaddrinfo For fixing stucking when resolving addrinfo

[XinZheng Zhang]

On Tue, Jun 7, 2016 at 5:42 AM, Michael Niedermayer
 wrote:
> On Wed, Jun 01, 2016 at 12:03:32PM +0800, XinZheng Zhang wrote:
>> On Tue, May 31, 2016 at 9:33 PM, Hendrik Leppkes  wrote:
>> >
>> > On Tue, May 31, 2016 at 2:58 PM, Xinzheng Zhang  
>> > wrote:
>> > > ---
>> > >  libavformat/tcp.c | 215 
>> > > ++
>> > >  1 file changed, 215 insertions(+)
>> > >
>> >
>> > This whole patch looks extremely complicated and long for something
>> > hidden behind a tiny option not saying much.
>> > Maybe you should start by elaborating in detail what you are trying to
>> > fix, how you are fixing it, and why you choose this approach and not
>> > any others that may appear simpler.
>> >
>>
>> For some ugly network conditions(Eg.Mobile device in a WiFi network,
>> but offline from internet), getaddrinfo() could block tcp_open() for a
>> long time,
>> neither timeout nor interrupt callback can save us.
>>
>> There are serveral way to fix this issue
>> AFAIK. We can use `getaddrinfo_a` on some linux platform
>> http://man7.org/linux/man-pages/man3/getaddrinfo_a.3.html
>> But, it doesn't work on other platforms.
>
> would the getaddrinfo_a() based code be simple ?
> if so i think thats the way to go
>
> If one really wants one could implement getaddrinfo_a() emulation
> with a thread on other platforms. That should still be cleaner then
> stuffing this code into tcp.c
>
> above is just a suggestion
>
> [...]

I thought that create and destory a thread everty time
for reolving addrinfo is such a waste thing, but as said that
there are no global thread pool or other background facility in ffmpeg,
and on most platform there has not getaddrinfo_a().

Wrap there code to a function like getaddrinfo_a(),
will make these horrible thing to be invisible.
Considering that so I choose to stuff these code into tcp.c

I know that I should implement sth maybe called ff_getaddrinfo_a().
But I do not know how...(Specially in iOS and Android platform).
Any suggestion will be helpful, thanks a lot.

Sorry for the late reply.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] avformat/tcp: support timeout for getaddrinfo For fixing stucking when resolving addrinfo

[Michael Niedermayer]

On Wed, Jun 01, 2016 at 12:03:32PM +0800, XinZheng Zhang wrote:
> On Tue, May 31, 2016 at 9:33 PM, Hendrik Leppkes  wrote:
> >
> > On Tue, May 31, 2016 at 2:58 PM, Xinzheng Zhang  
> > wrote:
> > > ---
> > >  libavformat/tcp.c | 215 
> > > ++
> > >  1 file changed, 215 insertions(+)
> > >
> >
> > This whole patch looks extremely complicated and long for something
> > hidden behind a tiny option not saying much.
> > Maybe you should start by elaborating in detail what you are trying to
> > fix, how you are fixing it, and why you choose this approach and not
> > any others that may appear simpler.
> >
> 
> For some ugly network conditions(Eg.Mobile device in a WiFi network,
> but offline from internet), getaddrinfo() could block tcp_open() for a
> long time,
> neither timeout nor interrupt callback can save us.
> 
> There are serveral way to fix this issue
> AFAIK. We can use `getaddrinfo_a` on some linux platform
> http://man7.org/linux/man-pages/man3/getaddrinfo_a.3.html
> But, it doesn't work on other platforms.

would the getaddrinfo_a() based code be simple ?
if so i think thats the way to go

If one really wants one could implement getaddrinfo_a() emulation
with a thread on other platforms. That should still be cleaner then
stuffing this code into tcp.c

above is just a suggestion

[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato


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

Re: [FFmpeg-devel] [PATCH 1/1] avformat/tcp: support timeout for getaddrinfo For fixing stucking when resolving addrinfo

[XinZheng Zhang]

On Wed, Jun 1, 2016 at 12:03 PM, XinZheng Zhang  wrote:
> On Tue, May 31, 2016 at 9:33 PM, Hendrik Leppkes  wrote:
>>
>> On Tue, May 31, 2016 at 2:58 PM, Xinzheng Zhang  
>> wrote:
>> > ---
>> >  libavformat/tcp.c | 215 
>> > ++
>> >  1 file changed, 215 insertions(+)
>> >
>>
>> This whole patch looks extremely complicated and long for something
>> hidden behind a tiny option not saying much.
>> Maybe you should start by elaborating in detail what you are trying to
>> fix, how you are fixing it, and why you choose this approach and not
>> any others that may appear simpler.
>>
>
> For some ugly network conditions(Eg.Mobile device in a WiFi network,
> but offline from internet), getaddrinfo() could block tcp_open() for a
> long time,
> neither timeout nor interrupt callback can save us.
>
> There are serveral way to fix this issue
> AFAIK. We can use `getaddrinfo_a` on some linux platform
> http://man7.org/linux/man-pages/man3/getaddrinfo_a.3.html
> But, it doesn't work on other platforms.
>
> Besides, we can put this task into a background thread,
> and detach from it when we want to cancel the task,
> or it is timed out. But unfortunately,
> there are no global thread pool or other background facility
> to host such detached tasks in ffmpeg


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

Re: [FFmpeg-devel] [PATCH 1/1] avformat/tcp: support timeout for getaddrinfo For fixing stucking when resolving addrinfo

[XinZheng Zhang]

On Tue, May 31, 2016 at 9:33 PM, Hendrik Leppkes  wrote:
>
> On Tue, May 31, 2016 at 2:58 PM, Xinzheng Zhang  wrote:
> > ---
> >  libavformat/tcp.c | 215 
> > ++
> >  1 file changed, 215 insertions(+)
> >
>
> This whole patch looks extremely complicated and long for something
> hidden behind a tiny option not saying much.
> Maybe you should start by elaborating in detail what you are trying to
> fix, how you are fixing it, and why you choose this approach and not
> any others that may appear simpler.
>

For some ugly network conditions(Eg.Mobile device in a WiFi network,
but offline from internet), getaddrinfo() could block tcp_open() for a
long time,
neither timeout nor interrupt callback can save us.

There are serveral way to fix this issue
AFAIK. We can use `getaddrinfo_a` on some linux platform
http://man7.org/linux/man-pages/man3/getaddrinfo_a.3.html
But, it doesn't work on other platforms.

Besides, we can put this task into a background thread,
and detach from it when we want to cancel the task,
or it is timed out. But unfortunately,
there are no global thread pool or other background facility
to host such detached tasks in ffmpeg
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] avformat/tcp: support timeout for getaddrinfo For fixing stucking when resolving addrinfo

[Hendrik Leppkes]

On Tue, May 31, 2016 at 2:58 PM, Xinzheng Zhang  wrote:
> ---
>  libavformat/tcp.c | 215 
> ++
>  1 file changed, 215 insertions(+)
>

This whole patch looks extremely complicated and long for something
hidden behind a tiny option not saying much.
Maybe you should start by elaborating in detail what you are trying to
fix, how you are fixing it, and why you choose this approach and not
any others that may appear simpler.

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

Re: [FFmpeg-devel] [PATCH 1/1] avformat/tcp: support timeout for getaddrinfo For fixing stucking when resolving addrinfo

[XinZheng Zhang]

Sorry, This a wrong patch from a private branch. I will post the correct
one later.

On Tue, May 31, 2016 at 7:05 PM, Xinzheng Zhang 
wrote:

> From: xinzhengzhang 
>
> ---
>  libavformat/tcp.c | 194
> ++
>  1 file changed, 194 insertions(+)
>
> diff --git a/libavformat/tcp.c b/libavformat/tcp.c
> index 4ac061a..dc3e0bd 100644
> --- a/libavformat/tcp.c
> +++ b/libavformat/tcp.c
> @@ -32,11 +32,15 @@
>  #if HAVE_POLL_H
>  #include 
>  #endif
> +#if HAVE_PTHREADS
> +#include 
> +#endif
>
>  typedef struct TCPContext {
>  const AVClass *class;
>  int fd;
>  int listen;
> +int addrinfo_timeout;
>  int open_timeout;
>  int rw_timeout;
>  int listen_timeout;
> @@ -52,6 +56,7 @@ static const AVOption options[] = {
>  { "listen",  "Listen for incoming connections",
> OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0,   2,
>  .flags = D|E },
>  { "timeout", "set timeout (in microseconds) of socket I/O
> operations", OFFSET(rw_timeout), AV_OPT_TYPE_INT, { .i64 = -1 },
>  -1, INT_MAX, .flags = D|E },
>  { "listen_timeout",  "Connection awaiting timeout (in
> milliseconds)",  OFFSET(listen_timeout), AV_OPT_TYPE_INT, { .i64 = -1
> }, -1, INT_MAX, .flags = D|E },
> +{ "addrinfo_timeout", "set timeout (in microseconds) for
> getaddrinfo()",   OFFSET(addrinfo_timeout), AV_OPT_TYPE_INT, { .i64 = -1
> },   -1, INT_MAX, .flags = D|E },
>  { "send_buffer_size", "Socket send buffer size (in bytes)",
>   OFFSET(send_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1,
> INT_MAX, .flags = D|E },
>  { "recv_buffer_size", "Socket receive buffer size (in bytes)",
>  OFFSET(recv_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1,
> INT_MAX, .flags = D|E },
>  { "ijkapplication",  "AVApplicationContext",
>  OFFSET(app_ctx),AV_OPT_TYPE_INT64, { .i64 = 0 },   INT64_MIN,
> INT64_MAX, .flags = D },
> @@ -65,6 +70,189 @@ static const AVClass tcp_class = {
>  .version= LIBAVUTIL_VERSION_INT,
>  };
>
> +#ifdef HAVE_PTHREADS
> +
> +typedef struct TCPAddrinfoRequest
> +{
> +AVBufferRef *buffer;
> +
> +pthread_mutex_t mutex;
> +pthread_cond_t cond;
> +
> +int64_t timeout;// in microseconds;
> +AVIOInterruptCB interrupt_callback;
> +
> +char*hostname;
> +char*servname;
> +struct addrinfo  hints;
> +struct addrinfo *res;
> +
> +volatile int finished;
> +int  ret;
> +} TCPAddrinfoRequest;
> +
> +static void tcp_getaddrinfo_request_free(TCPAddrinfoRequest *req)
> +{
> +freeaddrinfo(req->res);
> +
> +av_freep(>servname);
> +av_freep(>hostname);
> +pthread_cond_destroy(>cond);
> +pthread_mutex_destroy(>mutex);
> +av_freep();
> +}
> +
> +static void tcp_getaddrinfo_request_free_buffer(void *opaque, uint8_t
> *data)
> +{
> +TCPAddrinfoRequest *req = (TCPAddrinfoRequest *)opaque;
> +tcp_getaddrinfo_request_free(req);
> +}
> +
> +static int tcp_getaddrinfo_request_create(TCPAddrinfoRequest **request,
> +  const char *hostname,
> +  const char *servname,
> +  const struct addrinfo *hints,
> +  int64_t timeout,
> +  const AVIOInterruptCB *int_cb)
> +{
> +TCPAddrinfoRequest *req = (TCPAddrinfoRequest *)
> av_mallocz(sizeof(TCPAddrinfoRequest));
> +if (!req)
> +return AVERROR(ENOMEM);
> +
> +if (pthread_mutex_init(>mutex, NULL)) {
> +av_freep();
> +return AVERROR(ENOMEM);
> +}
> +
> +if (pthread_cond_init(>cond, NULL)) {
> +pthread_mutex_destroy(>mutex);
> +av_freep();
> +return AVERROR(ENOMEM);
> +}
> +
> +req->timeout= timeout;
> +req->interrupt_callback = *int_cb;
> +
> +if (hostname && *hostname) {
> +req->hostname = av_strdup(hostname);
> +if (!req->hostname)
> +goto fail;
> +}
> +
> +if (servname) {
> +req->servname = av_strdup(servname);
> +if (!req->hostname)
> +goto fail;
> +}
> +
> +if (hints) {
> +req->hints.ai_family   = hints->ai_family;
> +req->hints.ai_socktype = hints->ai_socktype;
> +req->hints.ai_protocol = hints->ai_protocol;
> +req->hints.ai_flags= hints->ai_flags;
> +}
> +
> +req->buffer = av_buffer_create(NULL, 0,
> tcp_getaddrinfo_request_free_buffer, req, 0);
> +if (!req->buffer)
> +goto fail;
> +
> +*request = req;
> +return 0;
> +fail:
> +tcp_getaddrinfo_request_free(req);
> +return AVERROR(ENOMEM);
> +}
> +
> +static void *tcp_getaddrinfo_worker(void *arg)
> +{
> +TCPAddrinfoRequest *req = arg;
> +
> +req->ret = getaddrinfo(req->hostname,