[FFmpeg-devel] [PATCH] libavformat: Don't return errors if select is interrupted

[Ronald S. Bultje]

Hi,

On Thu, Mar 4, 2010 at 5:23 PM, Martin Storsj? <martin at martin.st> wrote:
> Currently, most network protocols wait for data with a select loop, with a
> 100 ms timeout and recheck url_interrupt_cb() for each round in the loop.
> If the select is interrupted (e.g. by a signal), most of the protocols
> return an error, even if a similar interruption in recv is handled by
> retrying. The attached patch makes the protocols simply retry select if it
> returned with EINTR or EAGAIN, instead of returning an error in these
> cases. This makes the interruption behaviour consistent between select and
> recv (or send or similar).
>
> Ronald, does this sound sensible to you?

+        } else if (n < 0) {
[..]
+            return AVERROR(EIO);

That should be applied separately and is OK.

For the rest, does it fix an actual bug? Let's start with EAGAIN:

EAGAIN, according to man 2 select:
     [EAGAIN]           The kernel was (perhaps temporarily) unable to allo-
                        cate the requested number of file descriptors.
This is not the same as man 2 recv:
     [EAGAIN]           The socket is marked non-blocking, and the receive
                        operation would block, or a receive timeout had been
                        set, and the timeout expired before data were
                        received.

Since we're blocking, what happens here for recv() is that no data was
received and the timer expired. For select(), then the return value is
0 (see man 2 recv:)

RETURN VALUES
[..] If the time limit expires, select() returns 0. [..]

Because of that, IMO the EAGAIN part is not OK, the difference is intended.

As for EINTR, I in fact consider it bad behaviour to continue a loop
when interrupted, so I've always frowned upon its use in recv(). My
hypothesis is that people use other functions to manually interrupt,
in combination with url_interrupt_cb(), and then want the function to
return EINTR instead of EIO (default return value), but I'm not sure.
Maybe somebody else knows? Tabled, for now... :-).

Ronald