On Sat, 21 May 2011, Luca Barbato wrote:

> The connect() timeout can take minutes, gets misreported as EIO and
> isn't interruptible.
> ---
>  libavformat/tcp.c |   25 +++++++++++++++++--------
>  1 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/libavformat/tcp.c b/libavformat/tcp.c
> index ced1038..e602a55 100644
> --- a/libavformat/tcp.c
> +++ b/libavformat/tcp.c
> @@ -45,6 +45,7 @@ static int tcp_open(URLContext *h, const char *uri, int 
> flags)
>      char buf[256];
>      int ret;
>      socklen_t optlen;
> +    int timeout = 100;
>      char hostname[1024],proto[1024],path[1024];
>      char portstr[10];
>  
> @@ -57,6 +58,9 @@ static int tcp_open(URLContext *h, const char *uri, int 
> flags)
>      if (p) {
>          if (av_find_info_tag(buf, sizeof(buf), "listen", p))
>              listen_socket = 1;
> +        if (av_find_info_tag(buf, sizeof(buf), "timeout", p)) {
> +            timeout = strtol(buf, NULL, 10);
> +        }
>      }
>      memset(&hints, 0, sizeof(hints));
>      hints.ai_family = AF_UNSPEC;
> @@ -73,6 +77,7 @@ static int tcp_open(URLContext *h, const char *uri, int 
> flags)
>      cur_ai = ai;
>  
>   restart:
> +    ret = AVERROR(EIO);
>      fd = socket(cur_ai->ai_family, cur_ai->ai_socktype, cur_ai->ai_protocol);
>      if (fd < 0)
>          goto fail;
> @@ -84,28 +89,29 @@ static int tcp_open(URLContext *h, const char *uri, int 
> flags)
>          fd1 = accept(fd, NULL, NULL);
>          closesocket(fd);
>          fd = fd1;
> +        ff_socket_nonblock(fd, 1);
>      } else {
>   redo:
> +        ff_socket_nonblock(fd, 1);
>          ret = connect(fd, cur_ai->ai_addr, cur_ai->ai_addrlen);
>      }
>  
> -    ff_socket_nonblock(fd, 1);
> -
>      if (ret < 0) {
>          struct pollfd p = {fd, POLLOUT, 0};
> -        if (ff_neterrno() == AVERROR(EINTR)) {
> +        ret = ff_neterrno();
> +        if (ret == AVERROR(EINTR)) {
>              if (url_interrupt_cb()) {
>                  ret = AVERROR_EXIT;
>                  goto fail1;
>              }
>              goto redo;
>          }
> -        if (ff_neterrno() != AVERROR(EINPROGRESS) &&
> -            ff_neterrno() != AVERROR(EAGAIN))
> +        if (ret != AVERROR(EINPROGRESS) &&
> +            ret != AVERROR(EAGAIN))
>              goto fail;
>  
>          /* wait until we are connected or until abort */
> -        for(;;) {
> +        while(timeout--) {
>              if (url_interrupt_cb()) {
>                  ret = AVERROR_EXIT;
>                  goto fail1;
> @@ -114,7 +120,10 @@ static int tcp_open(URLContext *h, const char *uri, int 
> flags)
>              if (ret > 0)
>                  break;
>          }
> -
> +        if (ret <= 0) {
> +            ret = AVERROR(ETIMEDOUT);
> +            goto fail;
> +        }
>          /* test error */
>          optlen = sizeof(ret);
>          getsockopt (fd, SOL_SOCKET, SO_ERROR, &ret, &optlen);
> @@ -122,6 +131,7 @@ static int tcp_open(URLContext *h, const char *uri, int 
> flags)
>              av_log(h, AV_LOG_ERROR,
>                     "TCP connection to %s:%d failed: %s\n",
>                     hostname, port, strerror(ret));
> +            ret = AVERROR(ret);

Hmm, I guess we should map error codes from winsock errors to AVERRORs 
here, too, as we do internally in ff_neterrno() now. That's mostly 
important for EAGAIN though, and I don't think we should hit that one 
here.

>              goto fail;
>          }
>      }
> @@ -144,7 +154,6 @@ static int tcp_open(URLContext *h, const char *uri, int 
> flags)
>              closesocket(fd);
>          goto restart;
>      }
> -    ret = AVERROR(EIO);
>   fail1:
>      if (fd >= 0)
>          closesocket(fd);
> -- 
> 1.7.5.rc1

Except for that, it looks quite good I think.

// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to