Message ID | 20180102161915.19575-1-nfxjfg@googlemail.com |
---|---|
State | Accepted |
Commit | 8a108bdea06fac43af9f44b6d2538f357451167a |
Headers | show |
On Tue, 2 Jan 2018 17:19:14 +0100 wm4 <nfxjfg@googlemail.com> wrote: > It makes no sense to return an error after the first reconnect, and then > somehow resume the next time it's called. Usually this will lead to > demuxer errors. Make reconnecting block instead, until it has either > successfully reconnected, or given up. > > Also make the wait reasonably interruptible. Since there is no mechanism > for this in the API, polling is the best we can do. This behaves roughly > the same as other interruptible network functions in libavformat. > > (The original code would work if it returned AVERROR(EAGAIN) or so, > which would make retry_transfer_wrapper() repeat the read call. But I > think having an explicit loop for this is better anyway.) > > I also snuck in a fix for reconnect_at_eof. It has to check for > AVERROR_EOF, not 0. > --- > libavformat/http.c | 19 ++++++++++--------- > libavformat/network.c | 18 ++++++++++++++++++ > libavformat/network.h | 9 +++++++++ > 3 files changed, 37 insertions(+), 9 deletions(-) > > diff --git a/libavformat/http.c b/libavformat/http.c > index 8f7e56de54..5eff87f8bb 100644 > --- a/libavformat/http.c > +++ b/libavformat/http.c > @@ -117,7 +117,6 @@ typedef struct HTTPContext { > int reconnect; > int reconnect_at_eof; > int reconnect_streamed; > - int reconnect_delay; > int reconnect_delay_max; > int listen; > char *resource; > @@ -1433,6 +1432,7 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size) > HTTPContext *s = h->priv_data; > int err, new_location, read_ret; > int64_t seek_ret; > + int reconnect_delay = 0; > > if (!s->hd) > return AVERROR_EOF; > @@ -1448,25 +1448,26 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size) > return http_buf_read_compressed(h, buf, size); > #endif /* CONFIG_ZLIB */ > read_ret = http_buf_read(h, buf, size); > - if ( (read_ret < 0 && s->reconnect && (!h->is_streamed || s->reconnect_streamed) && s->filesize > 0 && s->off < s->filesize) > - || (read_ret == 0 && s->reconnect_at_eof && (!h->is_streamed || s->reconnect_streamed))) { > + while ((read_ret < 0 && s->reconnect && (!h->is_streamed || s->reconnect_streamed) && s->filesize > 0 && s->off < s->filesize) > + || (read_ret == AVERROR_EOF && s->reconnect_at_eof && (!h->is_streamed || s->reconnect_streamed))) { > uint64_t target = h->is_streamed ? 0 : s->off; > > - if (s->reconnect_delay > s->reconnect_delay_max) > + if (reconnect_delay > s->reconnect_delay_max) > return AVERROR(EIO); > > av_log(h, AV_LOG_INFO, "Will reconnect at %"PRIu64" error=%s.\n", s->off, av_err2str(read_ret)); > - av_usleep(1000U*1000*s->reconnect_delay); > - s->reconnect_delay = 1 + 2*s->reconnect_delay; > + err = ff_network_sleep_interruptible(1000U*1000*reconnect_delay, &h->interrupt_callback); > + if (err != AVERROR(ETIMEDOUT)) > + return err; > + reconnect_delay = 1 + 2*reconnect_delay; > seek_ret = http_seek_internal(h, target, SEEK_SET, 1); > - if (seek_ret != target) { > + if (seek_ret >= 0 && seek_ret != target) { > av_log(h, AV_LOG_ERROR, "Failed to reconnect at %"PRIu64".\n", target); > return read_ret; > } > > read_ret = http_buf_read(h, buf, size); > - } else > - s->reconnect_delay = 0; > + } > > return read_ret; > } > diff --git a/libavformat/network.c b/libavformat/network.c > index 6c3d9def3b..e9eb4b443a 100644 > --- a/libavformat/network.c > +++ b/libavformat/network.c > @@ -103,6 +103,24 @@ int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, AVIOInterrupt > } > } > > +int ff_network_sleep_interruptible(int64_t timeout, AVIOInterruptCB *int_cb) > +{ > + int64_t wait_start = av_gettime_relative(); > + > + while (1) { > + int64_t time_left; > + > + if (ff_check_interrupt(int_cb)) > + return AVERROR_EXIT; > + > + time_left = timeout - (av_gettime_relative() - wait_start); > + if (time_left <= 0) > + return AVERROR(ETIMEDOUT); > + > + av_usleep(FFMIN(time_left, POLLING_TIME * 1000)); > + } > +} > + > void ff_network_close(void) > { > #if HAVE_WINSOCK2_H > diff --git a/libavformat/network.h b/libavformat/network.h > index b78e3ad6ed..a663115541 100644 > --- a/libavformat/network.h > +++ b/libavformat/network.h > @@ -96,6 +96,15 @@ int ff_network_wait_fd(int fd, int write); > */ > int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, AVIOInterruptCB *int_cb); > > +/** > + * Waits for up to 'timeout' microseconds. If the usert's int_cb is set and > + * triggered, return before that. > + * @timeout Timeout in microseconds. Maybe have lower actual precision. > + * @param int_cb Interrupt callback, is checked regularly. > + * @return AVERROR(ETIMEDOUT) if timeout expirted, AVERROR_EXIT if interrupted by int_cb > + */ > +int ff_network_sleep_interruptible(int64_t timeout, AVIOInterruptCB *int_cb); > + > int ff_inet_aton (const char * str, struct in_addr * add); > > #if !HAVE_STRUCT_SOCKADDR_STORAGE Ping.
On Tue, 2 Jan 2018 17:19:14 +0100 wm4 <nfxjfg@googlemail.com> wrote: > It makes no sense to return an error after the first reconnect, and then > somehow resume the next time it's called. Usually this will lead to > demuxer errors. Make reconnecting block instead, until it has either > successfully reconnected, or given up. > > Also make the wait reasonably interruptible. Since there is no mechanism > for this in the API, polling is the best we can do. This behaves roughly > the same as other interruptible network functions in libavformat. > > (The original code would work if it returned AVERROR(EAGAIN) or so, > which would make retry_transfer_wrapper() repeat the read call. But I > think having an explicit loop for this is better anyway.) > > I also snuck in a fix for reconnect_at_eof. It has to check for > AVERROR_EOF, not 0. > --- Both patches approved by BBB on IRC. Pushed.
diff --git a/libavformat/http.c b/libavformat/http.c index 8f7e56de54..5eff87f8bb 100644 --- a/libavformat/http.c +++ b/libavformat/http.c @@ -117,7 +117,6 @@ typedef struct HTTPContext { int reconnect; int reconnect_at_eof; int reconnect_streamed; - int reconnect_delay; int reconnect_delay_max; int listen; char *resource; @@ -1433,6 +1432,7 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size) HTTPContext *s = h->priv_data; int err, new_location, read_ret; int64_t seek_ret; + int reconnect_delay = 0; if (!s->hd) return AVERROR_EOF; @@ -1448,25 +1448,26 @@ static int http_read_stream(URLContext *h, uint8_t *buf, int size) return http_buf_read_compressed(h, buf, size); #endif /* CONFIG_ZLIB */ read_ret = http_buf_read(h, buf, size); - if ( (read_ret < 0 && s->reconnect && (!h->is_streamed || s->reconnect_streamed) && s->filesize > 0 && s->off < s->filesize) - || (read_ret == 0 && s->reconnect_at_eof && (!h->is_streamed || s->reconnect_streamed))) { + while ((read_ret < 0 && s->reconnect && (!h->is_streamed || s->reconnect_streamed) && s->filesize > 0 && s->off < s->filesize) + || (read_ret == AVERROR_EOF && s->reconnect_at_eof && (!h->is_streamed || s->reconnect_streamed))) { uint64_t target = h->is_streamed ? 0 : s->off; - if (s->reconnect_delay > s->reconnect_delay_max) + if (reconnect_delay > s->reconnect_delay_max) return AVERROR(EIO); av_log(h, AV_LOG_INFO, "Will reconnect at %"PRIu64" error=%s.\n", s->off, av_err2str(read_ret)); - av_usleep(1000U*1000*s->reconnect_delay); - s->reconnect_delay = 1 + 2*s->reconnect_delay; + err = ff_network_sleep_interruptible(1000U*1000*reconnect_delay, &h->interrupt_callback); + if (err != AVERROR(ETIMEDOUT)) + return err; + reconnect_delay = 1 + 2*reconnect_delay; seek_ret = http_seek_internal(h, target, SEEK_SET, 1); - if (seek_ret != target) { + if (seek_ret >= 0 && seek_ret != target) { av_log(h, AV_LOG_ERROR, "Failed to reconnect at %"PRIu64".\n", target); return read_ret; } read_ret = http_buf_read(h, buf, size); - } else - s->reconnect_delay = 0; + } return read_ret; } diff --git a/libavformat/network.c b/libavformat/network.c index 6c3d9def3b..e9eb4b443a 100644 --- a/libavformat/network.c +++ b/libavformat/network.c @@ -103,6 +103,24 @@ int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, AVIOInterrupt } } +int ff_network_sleep_interruptible(int64_t timeout, AVIOInterruptCB *int_cb) +{ + int64_t wait_start = av_gettime_relative(); + + while (1) { + int64_t time_left; + + if (ff_check_interrupt(int_cb)) + return AVERROR_EXIT; + + time_left = timeout - (av_gettime_relative() - wait_start); + if (time_left <= 0) + return AVERROR(ETIMEDOUT); + + av_usleep(FFMIN(time_left, POLLING_TIME * 1000)); + } +} + void ff_network_close(void) { #if HAVE_WINSOCK2_H diff --git a/libavformat/network.h b/libavformat/network.h index b78e3ad6ed..a663115541 100644 --- a/libavformat/network.h +++ b/libavformat/network.h @@ -96,6 +96,15 @@ int ff_network_wait_fd(int fd, int write); */ int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, AVIOInterruptCB *int_cb); +/** + * Waits for up to 'timeout' microseconds. If the usert's int_cb is set and + * triggered, return before that. + * @timeout Timeout in microseconds. Maybe have lower actual precision. + * @param int_cb Interrupt callback, is checked regularly. + * @return AVERROR(ETIMEDOUT) if timeout expirted, AVERROR_EXIT if interrupted by int_cb + */ +int ff_network_sleep_interruptible(int64_t timeout, AVIOInterruptCB *int_cb); + int ff_inet_aton (const char * str, struct in_addr * add); #if !HAVE_STRUCT_SOCKADDR_STORAGE