Skip to content

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.

Merge request reports