[FFmpeg-devel] [PATCH v4] Add ZeroMQ as protocol option

[Marton Balint]

On Fri, 30 Aug 2019, Andriy Gelman wrote:

> On Thu, 29. Aug 21:01, Marton Balint wrote:
>> 
>> 
>> On Thu, 29 Aug 2019, Andriy Gelman wrote:
>> 
>> > Changes in v4:
>> >  - Use polling instead of non-blocking option for socket
>> >    read/write operations.
>> >  - Added pkt_size, timeout_send, timeout_recv options.
>> >    Updated documentation for new options.
>> 
>> No, timeout_send and timeout_recv is not needed. The URL context has a
>> timeout parameter.
>> 
>> Please see how unix.c or tcp.c does the polling: They both use a helper
>> funciton called ff_network_wait_fd_timeout (implemented in network.c) which
>> does polling in a loop with small timeouts in order to be able to check the
>> interrupt callback. After successful polling you can call the actual
>> transfer function.
>> 
>> You should do something similar, copy ff_network_wait_fd_timeout, replace
>> the file descriptor polling with zmq polling and you should be good to go.
>
> Thanks Marton, I've added these changes. I'll send the updated patch shortly. 
>
> When working on this new version, I first tried to use the AVIO_FLAG_NONBLOCK
> flag to decide whether to go into a blocking or non-blocking mode.
>
> My intention was to return AVERROR(EAGAIN) when in the non-blocking mode if
> read/write is not available. But avio exits with "Resource temporarily
> unavailable" when AVIO_FLAG_NONBLOCK flag is set and AVERROR(EAGAIN) is
> returned.  So probably I've misunderstood the meaning of AVIO_FLAG_NONBLOCK.
> The updated patch is not using this flag.

AVIO_FLAG_NONBLOCK is not heavily used or tested. You can still add 
support for it, (if the flag is set then you skip the code where you poll 
and if the transfer function returns EAGAIN then you return that as a 
special case). I am not sure how you can test it actually.

>
> Another point: the documentation in url.h says:
>
> " * In non-blocking mode, return AVERROR(EAGAIN) immediately.
> * In blocking mode, wait for data/EOF/error with a short timeout (0.1s),
> * and return AVERROR(EAGAIN) on timeout."
>
> But the function ff_network_wait_fd_timeout returns AVERROR(ETIMEDOUT) on
> timeout (causing the code to exit) in blocking mode. It seems to me that either
> the documentation or the return value should be updated. Should I submit a
> separate patch on this?

Yeah, the docs seems inaccurate. I guess the idea was that common code in 
avio.c:retry_transfer_wrapper should handle the retries but the common 
code has to differentiate between EAGAIN which should be retried 
immediately and EAGAIN which should be retried after sleeping a bit. Maybe 
we should simply return EINTR for the immediate retry case, because that 
already means an immediate retry in retry_transfer_wrapper.

So I guess the docs should be changed to that (yes, submit a separate 
patch), and existing protocols should be fixed to return EINTR when the 
poll() times out instead of EAGAIN. This way we can reduce the duplicated 
code from the protocols. And of course this also means that the zmq 
protocol can return EINTR on poll timeout and you can call ff_zmq_wait 
instead of ff_zmq_wait_timeout.

Regards,
Marton