Message ID | tencent_4F202F4DBE4F85552AA75638A0960DCD5707@qq.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,1/2] avformat/network: fix timeout inaccurate in wait_fd_timeout | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
> The wait_start was about POLLING_TIME larger which leads to timeout > 100ms late than the option setting. > --- > libavformat/network.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/libavformat/network.c b/libavformat/network.c > index 0f5a575f77..7a9a4be5bb 100644 > --- a/libavformat/network.c > +++ b/libavformat/network.c > @@ -78,7 +78,10 @@ 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) > { > int ret; > - int64_t wait_start = 0; > + int64_t wait_start; > + > + if (timeout > 0) > + wait_start = av_gettime_relative(); I think we intentionally wanted to avoid calling gettime on the fast path. > > while (1) { > if (ff_check_interrupt(int_cb)) > @@ -86,12 +89,8 @@ int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, AVIOInterrupt > ret = ff_network_wait_fd(fd, write); > if (ret != AVERROR(EAGAIN)) > return ret; > - if (timeout > 0) { > - if (!wait_start) > - wait_start = av_gettime_relative(); Why not simply wait_start = av_gettime_relative() - POLLING_TIME? It seems to achieve the same result. Thanks, Marton > - else if (av_gettime_relative() - wait_start > timeout) > - return AVERROR(ETIMEDOUT); > - } > + if (timeout > 0 && (av_gettime_relative() - wait_start > timeout)) > + return AVERROR(ETIMEDOUT); > } > } > > -- > 2.28.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> On Feb 9, 2021, at 6:03 AM, Marton Balint <cus@passwd.hu> wrote: > >> >> The wait_start was about POLLING_TIME larger which leads to timeout >> 100ms late than the option setting. >> --- >> libavformat/network.c | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/libavformat/network.c b/libavformat/network.c >> index 0f5a575f77..7a9a4be5bb 100644 >> --- a/libavformat/network.c >> +++ b/libavformat/network.c >> @@ -78,7 +78,10 @@ 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) >> { >> int ret; >> - int64_t wait_start = 0; >> + int64_t wait_start; >> + >> + if (timeout > 0) >> + wait_start = av_gettime_relative(); > > I think we intentionally wanted to avoid calling gettime on the fast path. > >> >> while (1) { >> if (ff_check_interrupt(int_cb)) >> @@ -86,12 +89,8 @@ int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, AVIOInterrupt >> ret = ff_network_wait_fd(fd, write); >> if (ret != AVERROR(EAGAIN)) >> return ret; >> - if (timeout > 0) { >> - if (!wait_start) >> - wait_start = av_gettime_relative(); > > Why not simply wait_start = av_gettime_relative() - POLLING_TIME? It seems to achieve the same result. Then it depends on the implementation of ff_network_wait_fd. After adding wait_fd as a callback, there is no guarantee that it takes POLLING_TIME. It can be solved by adding a polling_time field to NetworkWaitFdCB: typedef struct NetworkWaitFdCB { int (*wait_fd)(int /*fd*/, int /*write*/, void* /*opaque*/); void *opaque; int polling_time; } NetworkWaitFdCB; This is trying to get rid of calling gettime once. We can go further and sacrifice the accuracy by: int64_t loop_count = (timeout > 0) ? (timeout / polling_time) : timeout; for (int64_t i = 0; loop_count <= 0 || i < loop_count; i++) { ... } In my opinion, either depends on gettime or loop_count, but not both. > > Thanks, > Marton > >> - else if (av_gettime_relative() - wait_start > timeout) >> - return AVERROR(ETIMEDOUT); >> - } >> + if (timeout > 0 && (av_gettime_relative() - wait_start > timeout)) >> + return AVERROR(ETIMEDOUT); >> } >> } >> -- >> 2.28.0 >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavformat/network.c b/libavformat/network.c index 0f5a575f77..7a9a4be5bb 100644 --- a/libavformat/network.c +++ b/libavformat/network.c @@ -78,7 +78,10 @@ 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) { int ret; - int64_t wait_start = 0; + int64_t wait_start; + + if (timeout > 0) + wait_start = av_gettime_relative(); while (1) { if (ff_check_interrupt(int_cb)) @@ -86,12 +89,8 @@ int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, AVIOInterrupt ret = ff_network_wait_fd(fd, write); if (ret != AVERROR(EAGAIN)) return ret; - if (timeout > 0) { - if (!wait_start) - wait_start = av_gettime_relative(); - else if (av_gettime_relative() - wait_start > timeout) - return AVERROR(ETIMEDOUT); - } + if (timeout > 0 && (av_gettime_relative() - wait_start > timeout)) + return AVERROR(ETIMEDOUT); } }