[FFmpeg-devel] [PATCH] add timeout to udp_read

[Stefano Sabatini]

On date Sunday 2009-12-27 16:36:28 +0100, Reimar D?ffinger encoded:
> On Sun, Dec 27, 2009 at 03:34:33PM +0100, Michael Niedermayer wrote:
> > On Sun, Dec 27, 2009 at 02:51:03PM +0100, Reimar D?ffinger wrote:
> > > On Sun, Dec 27, 2009 at 02:13:53PM +0100, Michael Niedermayer wrote:
> > > > On Sun, Dec 27, 2009 at 01:45:21PM +0100, Reimar D?ffinger wrote:
> > > > > On Sun, Dec 27, 2009 at 01:23:48PM +0100, Michael Niedermayer wrote:
> > > > > > On Sun, Dec 27, 2009 at 01:15:55PM +0100, Reimar D?ffinger wrote:
> > > > > > > On Sun, Dec 27, 2009 at 01:09:51PM +0100, Michael Niedermayer wrote:
> > > > > > > > > I think it would also be a good idea to protect against the clock being
> > > > > > > > > reset by detecting av_gettime() < network_timeout_reference_time and
> > > > > > > > > adjusting network_timeout_reference_time.
> > > > > > > > 
> > > > > > > > why?
> > > > > > > > iam pretty sure ffmpeg will fail if the clock resets in 292
> > > > > > > > million years
> > > > > > > 
> > > > > > > I don't mean wrap-around, I mean if someone (e.g. NTP) happens to adjust
> > > > > > > the clock by e.g. a year, without an additional check our 10 ms timeout
> > > > > > > would just have become a 1-year and 10 ms timeout.
> > > > > > 
> > > > > > then we use the wrong clock
> > > > > 
> > > > > Well, I guess if there weren't the portability issues, av_gettime should
> > > > > probably use clock_gettime(CLOCK_MONOTONIC, ...) instead of
> > > > > gettimeofday.
> > > > > 
> > > > > > > It just seems reasonably simple enough to protect against this so I
> > > > > > > thought it can't hurt...
> > > > > > 
> > > > > > and if NTP slows the clock down like stoping it for a year ...
> > > > > 
> > > > > Which it won't. But apart from that it doesn't catch all issues, but it
> > > > > would catch a somewhat relevant one with only 2 lines of code or so.
> > > > 
> > > > what about using clock() for the timeout check?
> > > 
> > > "On several other implementations, the value returned by clock() also
> > > includes the times of any children whose status has
> > > been  collected via wait(2) (or another wait?type call)."
> > 
> > well, if you insisnt on the hack add it but i still think we should use
> > a clock source that works (or undefined behavior if the user adjusts
> > time behind us hard)
> 
> I think clock_gettime sounds fine for the purpose, if av_gettime is changed to use
> that (with a fallback if it is not available, possibly with a hack that
> ensures av_gettime always will be monotonic) I won't insist.
> Actually I think it is the far better solution, I just thought
> av_gettime used gettimeofday on purpose.

See patch, I still didn't tought about how to fix the case when the
function is missing.

*And* this function also requires librt, would be acceptable to use it
 or building libavformat? On the other hand I see that clock_gettime()
 is the right way to go (or at least gettimeofday is not the correct
 solution).

Regards.
-- 
FFmpeg = Forgiving Fancy Multimedia Picky Energized Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: use-clock-gettime.patch
Type: text/x-diff
Size: 525 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100225/5691bcd4/attachment.patch>