Draft: imem-access: implement errno reporting
WIP: The patch is still missing an addition on the libvlc_media_t object
to notify the access that new data is available. This is
intentional to gather some feedback before. It was also planned to
separate the callback object (libvlc_imem_t) from the media itself
and provide the notification function on this object instead of the
media, but it also impacts the way we design the libvlc API for 4.0.
Errno reporting will be used to forward EAGAIN/EWOULDBLOCK to the access and trigger a wait from the imem-access. The imem-access will be able to wait from a notification of the application while still being able to be interrupted by the input thread through the vlc_interrupt_t mechanism.
It solves a design problem on the imem-access and libvlc callbacks where the application could not be notified by the vlc_interrupt_t when the read() callback is pending. In that situation, an application wanting to trigger the stop of the media player will first need to signal the media end-of-stream, triggering a pipeline drain, before stopping the player.
sequenceDiagram
box Orange VLC side
participant input as Input thread
participant imem as imem access
end
box Cyan Application size
participant app as Application
end
input ->> imem: stream::read()
activate imem
imem ->> app: media callback read()
app --x imem: return "EOS" (no data to send)
imem --x input: Trigger pipeline drain (EOS)
deactivate imem
app ->> input: stop (change media)
This decoder and output drain adds latency to the change of a media. The duration depends on condition but I've been able to reproduce a steady 400ms latency addition when changing media (meaning that it will take 400ms more to start reading the new media), up to a gigantic 1.6s in a randomized manner (1.1s being taken by the drain of the audio decoder!).
With the new system, it's now possible to let a chance from VLC to interrupt just like any other access as long as the application can notify when no data is available instead of waiting, and provides a way for the application to easily trigger the end of the media without triggering a drain.
sequenceDiagram
box Orange VLC side
participant input as Input thread
participant imem as imem access
end
box Cyan SK side
participant player as Player
participant app as Application
end
input ->> imem: stream::read()
activate imem
imem ->> app: media callback read()
app --x imem: "set" errno to EAGAIN and return -1
app ->> player: Stop()
player ->> input: input_Stop()
input ->> imem: vlc_interrupt_kill()
deactivate imem
app ->> app: media release
app ->> imem: release common resource
imem --x app:
player --x app:
imem ->> app: media callback read()
app --x imem:
Other solutions were investigated to solve this issue:
1/ Provide the same solution but use errno
directly:
This solution would be the simplest but there's no way for the application to realize that errno returning EAGAIN, ie. mapping the read callback directly to read() on a non-blocking socket for instance, might create silent regression where the access would not try to fetch more data, since the application won't know it must notify the access.
2/ Provide a poll_cb() callback in the imem interface
The solution would require another callback, instead of another parameter. The new callback would be called every time before the read() callback, and would notify whether the read() is ready or not. This adds a new way for the application to exit, differentiating the return code for read() from the return code from poll(), but the access still cannot be interrupted and the additional cost on function call is higher.
3/ Provide a waker
assignment (like rust futures)
Provide a new callback to specify an object that can be used to notify the access that new data is available. This is much like poll() but it has the benefit that the application can immediately notify that no data is available, and then provide the object to notify the arrival of data asynchronously without exposing a public function on libvlc_media_t. This is still quite complex and still has a heavy function call cost, but it also fullfill every points from the suggestion here. However, the rust async model is not really the idiomatic model in C compared to POSIX errno.
4/ Solve the problem at the decoder and output level
One of the main symptoms that can be observed when changing media is the increase of the time to close a media, and in particular, to drain the decoder. This is also a non-interruptible task, because a decoder drain is synchronous. Admittedly, the interruption problem is less problematic if the drain itself can be handled smoothly afterwards, and I actually think both problems need to be solved eventually. But the decoder behaviour on drain depends on the decoder implementation, whereas the imem-access suggests a solution on the application side to correctly express the required intent, and it is also quite suitable to express an injection workflow (data being written from the application to the VLC pipeline) as well as a pull workflow (data being read from the application by VLC).
A big benefit from the submitted method is that it brings the access imem-access closer in behaviour to other access that can be interrupted, and thus diminish the surprise when the problem arise.