[FFmpeg-devel] [PATCH 4/5] lavfi: drop the requirement that request_frame returns a frame.

[Paul B Mahol]

On 8/26/15, Paul B Mahol <onemda at gmail.com> wrote:
> On 8/26/15, Nicolas George <george at nsup.org> wrote:
>> Le nonidi 9 fructidor, an CCXXIII, Paul B Mahol a ecrit :
>>> I'm interested.
>>
>> Glad to read it. So here it is:
>>
>> Foremost, I want to remove the recursiveness. Currently, when a frame is
>> requested on buffersink, it recursively calls request_frame() on all
>> filters
>> in the chain until the source, and the source recursively calls
>> filter_frame() until the sink. For a filter chain with N filters, that
>> makes
>> 2N nested function calls, this is bad for stack usage, and very annoying
>> for
>> debugging.
>>
>> First, I must make sure that request_frame() always calls a new
>> ff_filter_link_close() function before returning EOF. Unlike
>> avfilter_link_set_closed(), I intend to have it take a timestamp too,
>> that
>> would allow to fix the long-standing issue of the duration of the last
>> frame.
>>
>> Then I will change ff_request_frame() to just set a flag on the link. The
>> public API functions (mostly buffersink) will call a new function to
>> repeatedly scan the graph for links with the flag and run the
>> request_frame() callback when it is set. No more recursiveness for
>> request_frame().
>>
>> Then I intend to add a FIFO directly in the link structure.
>> ff_filter_frame() can just add the frame to FIFO, the next filter_frame()
>> callback will be called by the loop that scans the graph, just like the
>> request_frame() callback. No more recursiveness for filter_frame().
>>
>> (It leaves recursiveness for get_*_buffer(), but it is less of a
>> problem.)
>>
>> These need to come first and in that order, and after that, new
>> improvements
>> become possible:
>>
>> Keep the list of filters that need a callback called in a more efficient
>> data structure, to avoid scanning the whole graph each round.
>>
>> Merge the request_frame() and filter_frame() callbacks into a single
>> per-filter callback, maybe activate() or something. That avoids
>> separating
>> EOF handling from normal filtering, and overall makes writing filters
>> simpler. Of course, existing filters do not need to be changed, the
>> framework can still call the legacy callbacks automatically; it would
>> probably be able to satisfy the "needs_fifo" flag present on some filter.
>>
>> Allow filters to access the link FIFO directly. Instead of having one
>> frame
>> forcefully presented to it, the callback can access all the frames queued
>> in
>> all the inputs. This would supersedes FFBufQueue without requiring update
>> for the corresponding filters.
>>
>> Keep statistics about the amount of frames and memory in the link FIFOs.
>> With that, we can have a limit on the total memory used, and therefore
>> avoid
>> crashing the host for unschedulable graphs, but still have enough room
>> for
>> bursts of frames in a single link.
>>
>> Note that the last two points together should have the side effect of
>> fixing
>> the "Buffer queue overflow, dropping" that some valid filter graphs have.
>>
>> Make the filters run in threads. If the input and output properties of
>> links
>> are well separated, and the functions that manipulate them are
>> thread-safe,
>> the running the callbacks in a thread pool becomes very easy.
>>
>> In March I had started working on the same project, but I did not start
>> by
>> dropping the requirement addressed in this very patch, and it made things
>> a
>> lot more complex.
>>
>> If you have any remark or suggestion, it is of course welcome.
>
> I'm just waiting for more patches.
>
> Another missing feature that could be considered important is seeking
> support,
> or more specifically to be able to request any frame from stream if it is
> possible by underlying protocol.

One more thing, the query_formats is very limited for filters that pick one
pixel format for input and another one for output depending on picked format
for input. Returning EAGAIN almost always works but there are special
situations when it does not work. There should be query_formats2 or similar
which would address such limitations.