This is a departure from the conventional idea of decoders always outputting
data as fast as possible. Instead, this allows decoders to be throttled in the
same way filter graphs can be.
This comes into play when e.g. a demuxer is feeding into two decoders, but
only one of the two decoders is actually currently needed (e.g. due to
A/V misalignment). In that case, what typically happens is that the unneeded
decoder alse decodes all frames, and then piles them up on the "buffersrc"
filter's downstream link (growing indefinitely).
Another issue this solves manifests when e.g. a single demuxer is feeding many
decoders that all try to feed frames to the same filter graph. In this case,
all decoders run as fast as posssible, leading to lock contention on the
filter graph input queue; resulting in (again) many frames piling up on the
buffersrc (or downstream filters) for the unneeded inputs that are not actually
the bottleneck, while the input that's actually undersatisfied can end up
starved for CPU time, possibly for long enough to exhaust memory limits. The
normal rate limiting fails to apply in this scenario because all decoders share
a single demuxer, and are hence rate-limited only by the demuxer speed; whereas
the demuxer is not choked because from the PoV of the scheduler, the filter
graph is simply not getting enough frames.
In a more general sense, there's a philosophical argument to be made here.
Since a decoder is typically also a decompressor, it produces more data than
it consumes. So, it a sense, it's acting like a type of producer also - in
the same way that a filter graph can produce more input that outputs.
Solve all of these issues by allowing decoders to be output-choked, which
gives the scheduler control over when decoders are allowed to output frames.
This does mean we have to add some sort of internal packet queue, because the
decoder thread may need to continue *accepting* upstream packets from the
demuxer (or else we risk stalling the demuxer), but defer the actual decoding
by placing them inside an internal "overflow" queue.
This effectively simulates a sort of "filter graph"-type semantics but
for the decoder queue.
This overflow logic is fairly self-contained inside `sch_dec_receive`, though
it is quite nontrivial. I have added as much documentation as is hopefully
needed to understand the logic.
Importantly, we cannot simply unlimit the decoder input thread queue because
the demuxer relies on backpressure from the decoder to rate limit itself. (Note
that demuxers may only be active if there is at least one downstream decoder
that is alse active, so we always have at least one decoder providing
backpressure)
Sponsored-by: nxtedition AB
Signed-off-by: Niklas Haas <git@haasn.dev>
When a filter is choked, but upstream threads are trying to write to its input,
this can result in the filter's input queue getting stuck. Normally, the
unchoke_downstream() logic would prevent this from happening, since the
filter would itself get unchoked as a result of upstream decoders receiving
pressure from the demuxer.
However, upcoming changes to this logic will require weakening this upstream
unchoking logic, so preventing the deadlock in a more elegant way helps with
making the code more robust.
Sponsored-by: nxtedition AB
Signed-off-by: Niklas Haas <git@haasn.dev>
schedule_update_locked() is supposed to be a no-op when `sch->terminate`
is 1. However, there is a TOCTOU error here, where a different thread may
currently be executing schedule_update_locked(), having successfully passed
the sch->terminate check but without actually updating the choke status.
This does not matter for the current code, but will matter with the following
commit, where it creates the theoretical possibility of a race where sch_stop()
is trying to choke the demuxers (and unchoke the decoders) while
schedule_update_locked() is simultaneously trying to choke the decoders,
leading to a deadlock if the last decoder is left choked and unable to
propagate EOF downstream.
The cleanest solution is to just take the scheduler lock while updating the
choke status here. This ensures that any other schedule_update_locked() calls
will have completed.
Sponsored-by: nxtedition AB
Signed-off-by: Niklas Haas <git@haasn.dev>
Instead of awkwardly looping over the type, just split this up into
multiple loops. The loss in complexity seems worth the loss in conciseness
to me, and more importantly, this allows us to easily add more waiter types.
Sponsored-by: nxtedition AB
Signed-off-by: Niklas Haas <git@haasn.dev>
This is to reapply 18217bb0f5.
Its commit msg is still meaningful:
"Using the max instead of the min avoids the progress stopping
with gaps in sparse streams (subtitles)."
Also on a very similar issue: currently, a single stream with
no data makes ffmpeg reports N/A for both time and speed.
Fix this by ignoring missing dtses.
Fix regressions since d119ae2fd8.
Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
This logic was previously added to the scheduler. That commit added locking
(and rescheduling) when updating the corresponding `receive_finished` flag,
but missed doing the same for `send_finished`.
Fixes: fd1fd5850d
When the furthest-behind stream is being fed by a demuxer that is also
feeding packets to a choked filter graph, we need to unchoke that filter
graph to prevent the demuxer from getting stuck trying to write packets to
the choked filter graph.
This situation can also apply recursively - if the demuxer is also writing
to a filtergraph that is also reading from a choked demuxer, there is a
similar deadlock.
Solve all such deadlocks by just brute-force recursively unchoking all
nodes that can somehow prevent this demuxer from writing packets. This
should normally not result in any change in behavior, unless audio/video
streams are badly desynchronized, in which case it may result in extra
memory usage from the too-far-ahead stream buffering packets inside the
muxer. (But this is, of course, preferable to a deadlock)
Fixes: https://code.ffmpeg.org/FFmpeg/FFmpeg/issues/20611
For the purpose of merging streams in a stream group, a filtergraph can't be
created once we know it will be used. Therefore, allow filtergraphs to be
removed from the scheduler after being added.
Signed-off-by: James Almer <jamrial@gmail.com>
This allows upstream filters to observe EOF on their corresponding outputs
and terminate early, which is particularly useful for decoders and demuxers
that may then gracefully release their resources.
Prevents "leaking" memory for previously used, but now unused, filter inputs.
THis is currently done by sch_demux_send() (via demux_stream_send_to_dst()),
sch_enc_send() (via enc_send_to_dst()), and sch_dec_send() (via
dec_send_to_dst()), but not by sch_filter_send().
Implement the same queue-closing logic for them. The main benefit here is that
this will allow them to mark downstream inputs as send-done (in addition
to received-done), which is useful for a following commit.
Cut off a choked demuxer's output codec/filter queues, effectively preventing
them from processing packets while the demuxer is choked. Avoids downstream
nodes from piling up extra input that a demuxer shouldn't currently be
sending.
The main benefit of this is to avoid queuing up excess packets that don't want
to be decoded yet, reducing memory consumption for idle inputs by preventing
them from being read earlier than needed.
Currently, while the filter graph is being initially created, the scheduler
continues demuxing frames on the last input that happened to be active before
the filter graph was complete.
This can lead to an excess number of decoded frames "piling" up on this input,
regardless of whether or not it will actually be requested by the configured
filter graph. Suspending the filter graph during this initialization phase
reduces the amount of wasted memory.
This field is just saving (typically) a single pointer indirection; and IMO
makes the logic and graph relations unnecessarily complicated. I am also
considering adding choking logic to decoders and encoders as well, which this
field would get in the way of.
Apart from the unchoking logic in unchoke_for_input(), the only other place
that uses this field is the (cold) function check_acyclic(), which can be
served just as well with a simple function to do the graph traversal there.
The code in the decoder just cares about allocating enough extra hw frames
to cover the size of the queue; but there's no reason we actually *have* to
use this many. We can safely relax the assertion to a <= check.
The queue needs to track each frame/packet's stream index, this is
achieved by maintaining a parallel AVFifo instance for that purpose.
This is simpler than implementing custom AVContainerFifo callbacks.
These per-stream FIFOs hold the packets before every stream is
initialized and the header can be written. Once that happens, current
code will flush each stream's queue one after the other. However, in
case we buffered a lot of data for multiple streams, this may cause the
muxer to overflow max_interleave_delta, resulting in worse interleaving
than necessary.
Change the code to interleave packets from all the queues by DTS.
Do not return the return value of the last enc_send_to_dst()
call, as this would treat the last call differently from the
earlier calls; furthermore, sch_enc_send() explicitly documents
to always return 0 on success.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Since e0da916b8f the ffmpeg utility has
held multiple frames output by the decoder in internal queues without
telling the decoder that it is going to do so. When the decoder has a
fixed-size pool of frames (common in some hardware APIs where the output
frames must be stored as an array texture) this could lead to the pool
being exhausted and the decoder getting stuck. Fix this by telling the
decoder to allocate additional frames according to the queue size.
These functions used to be passed directly to pthread_create(), which
required them to return void*. This is no longer the case, so they can
return a plain int.
Use 8 packets/frames by default rather than 1, which seems to provide
better throughput.
Allow -thread_queue_size to set the muxer queue size manually again.
Should fix valgrind warnings about Conditional jump or move depends on uninitialised value.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: James Almer <jamrial@gmail.com>
See the comment block at the top of fftools/ffmpeg_sched.h for more
details on what this scheduler is for.
This commit adds the scheduling code itself, along with minimal
integration with the rest of the program:
* allocating and freeing the scheduler
* passing it throughout the call stack in order to register the
individual components (demuxers/decoders/filtergraphs/encoders/muxers)
with the scheduler
The scheduler is not actually used as of this commit, so it should not
result in any change in behavior. That will change in future commits.
Niklas Haas
Nicolas Gaullier
James Almer
Timo Rothenpieler
Gyan Doshi
Anton Khirnov
Michael Niedermayer
Andreas Rheinhardt
Mark Thompson