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

[Marton Balint]

On Sat, 31 Aug 2019, Marton Balint wrote:

>
>
> 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.

Ok, this won't work out of the box, because for EINTR we will not 
check if rw_timeout is elapsed...

>
> 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.

Keep the zmq patch as is then, we can remove the extra waiter loop 
function once the API is cleaned up...

Thanks,
Marton