Skip to content
Snippets Groups Projects

libvlc/player: Add a new media stopping event

  1. Sep 26, 2023
    • Alexandre Janniaux's avatar
      test: libvlc_callback: check imem-access behaviour · 30d1237d
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      The previous patches added an event from libvlc_media_player_t which
      notify the application that the current media is stopping, regardless of
      whether the player itself is stopping or that it will process the next
      media.
      
      Those patches solve a design problem on the imem-access and libvlc
      callbacks where the application could not be notified that the input was
      stopped when the read() callback was pending, which happens through
      vlc_interrupt_t in other demux.
      
      In that situation, an application wanting to trigger the stop of the
      media player can only signal the media end-of-stream,
      triggering a pipeline drain, before stopping the player.
      
      ```mermaid
      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)
      ```mermaid
      
      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 the application respond to
      the new event to handle the interruption and termination of what it has
      been doing in the libvlc_media_read_cb function, without triggering the
      end of the media when it would have triggered a drain.
      
      ```mermaid
      sequenceDiagram
      
          box Orange VLC side
          participant destructor as Player input destructor
          participant input as Input
          participant imem as imem access
          end
      
          box Cyan Application side
          participant player as Player
          participant app as Application
          end
      
          par Input thread
          activate imem
          input ->> imem: stream::read()
          imem ->> app: media callback read_cb()
          app ->> app: read_cb() might be waiting for some data
      
          and Application thread
          app ->> player: Stop()
          player ->> destructor: Add current input to destructor
      
          and Player destructor thread
          destructor ->> input: input_Stop()
          destructor ->> app: libvlc_MediaPlayerMediaStopping event
          end
      
          app ->> app: Interrupt read_cb() and return EOF
          deactivate imem
      ```
      
      This commit verifies this behaviour in a reliable way to ensure that the
      problem is indeed solved by this approach, while allowing to document
      the solution on the application side.
      
      To do so, two different tests are made:
      
       - A sanity test which checks that we can stop a libvlc media which is
         not blocking, basically checking that the media_callback API works
         and running it when we want to check against sanitizers.
      
       - A specific test checking that we can block the read() and only
         unblock it when the libvlc_MediaPlayerMediaStopping event is emitted,
         checking that the documented behaviour for the media callbacks is
         working correctly.
      
      ---
      
      The test needs:
      
       - No intermediate cache so that we clearly control the access from the
         demux.
      
       - Better deterministic behaviour, typically to check that we don't call
         AccessReadNonBlocking twice if we don't notify between them and try
         to check that read() is correctly terminating in this case.
      
      ---
      
      On a side note, other solutions were investigated to solve this issue:
      
      1/ Provide an `errno` variable as parameter to read:
      
      By providing an errno parameter, we can provide a specific error code
      when -1 is returned, and in particular use EAGAIN/EWOULDBLOCK to inform
      the imem-access code that we don't have data yet. Then, the imem-access
      can wait for a notification from the application to start reading again,
      while listening to interruption coming from the core.
      
      This solution was submitted but it felt like alienating the libVLC imem
      API and libvlc media API to solve this problem, without considering it
      as an application problem as a whole.
      
      2/ 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.
      
      3/ 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.
      
      4/ 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.
      
      5/ 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).
      30d1237d
    • Alexandre Janniaux's avatar
      lib: media player: signal stopping media event · 3841df7a
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      Signal that the media is stopping to the libvlc client. This is done
      after notifying the media on the VLC side that we're going to stop it,
      so that application can rely on the fact that the objects using
      application resources are already stopping, so that we can trigger
      actions like EOS, closing those resources, etc.
      3841df7a
    • Alexandre Janniaux's avatar
      player: signal on_current_media_stopping event · e651ac80
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      Signal that the media is stopping to the client. This is done after
      input_Stop so that application can rely on the fact that the underlying
      input_thread is already stopping, so that we can trigger actions like
      EOS, closing input resources, etc.
      e651ac80
    • Alexandre Janniaux's avatar
      lib: media_player: split function · 6d40b91c
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      Splitting the function allows return statement, which removes the need
      for break, and simplify the code. It also allows to use a designated
      initializer for the event structure.
      
      The code also move the vlc_assert_unreachable in the new function.
      
      By moving the vlc_assert_unreachable after the switch(), we can get
      warnings when not every cases are handled since there is no default
      case, and we can keep asserting when it fails to reach any return
      statement before.
      6d40b91c
    • Alexandre Janniaux's avatar
      lib: media_player: move vlc_assert_unreachable · 4a35193e
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      By moving the vlc_assert_unreachable after the switch(), we can get
      warnings when not every cases are handled since there is no default
      case, and we can keep asserting when it fails to reach any return
      statement before.
      4a35193e
    • Alexandre Janniaux's avatar
      test: player: add a state_equal() function · 52a231b8
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      The function improves the error reporting when the assertion is
      triggered, and also logs what were the resulting state, while nicely
      handling negative index like python arrays.
      52a231b8
    • Alexandre Janniaux's avatar
      test: player: dump the state vec when asserting · bcba4461
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      Dumping the list of state helps finding the root cause of the issue when
      a test fails, but also enable additional manual tests to check whether
      an unexpected untested test transition has happened or not.
      bcba4461
    • Alexandre Janniaux's avatar
      test: player: add player state_to_string function · 9bb751cb
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      Most of the code is checking the player state, but we had no way to
      signal in the test logs which state was actually there when it fails.
      9bb751cb
    • Alexandre Janniaux's avatar
      libvlc_events: add libvlc_MediaPlayerMediaStopping · 4ac61be1
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      The event forwards the vlc_player_t event `on_stopping_current_media`
      which signals that the media actually playing inside the player is
      stopping.
      
      The event is essential to the `libvlc_media_new_callbacks` API since
      there's no way to stop the \ref libvlc_media_read_cb callback without
      triggering an EOF, and the EOF should be signalled only as soon as the
      media is stopping.
      4ac61be1
    • Alexandre Janniaux's avatar
      vlc_player: add a new on_stopping_current_media event · 2ae6636c
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      Add a new event notifying when the current media is stopping.
      
      The media can be stopping without the player being stopping, ie. when
      the media is finished and the player prepares the next one, but also
      when the player changes media while having the player in the
      VLC_PLAYER_STATE_PLAYING state.
      
      The event will also be fired when the player is simply stopping, in
      which case we expect that VLC_PLAYER_STATE_STOPPING is signaled before
      the input is actually stop, which is coherent with the current
      behaviour.
      2ae6636c
    • Alexandre Janniaux's avatar
      VLCPlayerController: use designated initializers · 5d502b0b
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      Designated initializer will allow to add new fields in the
      vlc_player_cbs structure while automatically initializing the new fields
      to null.
      5d502b0b
    • Alexandre Janniaux's avatar
      vlc_player: fix typo in doc · d286d255
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      d286d255
    • Alexandre Janniaux's avatar
      qt: mlbookmarkmodel: use lambda for player cbs · df4ec732
      Alexandre Janniaux authored and Steve Lhomme's avatar Steve Lhomme committed
      The lambda will allow to use a zero-initializer and only initialize the
      required fields.
      df4ec732
Loading