Skip to content
Snippets Groups Projects
  1. May 14, 2022
  2. May 13, 2022
  3. May 12, 2022
    • Thomas Guillem's avatar
      tracer: json: use a define for tick -> integer conversion · 7ebb2d48
      Thomas Guillem authored and Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen committed
      This fixes a timebase mismatch between values from tracer entries (NS vs US).
      7ebb2d48
    • Thomas Guillem's avatar
      tracer: use a define for tick -> integer conversion · 20973203
      Thomas Guillem authored and Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen committed
      20973203
    • Thomas Guillem's avatar
      tracer: refector VLC_TRACE from ticks · 1a54b8d2
      Thomas Guillem authored and Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen committed
      1a54b8d2
    • Alexandre Janniaux's avatar
      player: input: release assertions on player state · 84659b15
      Alexandre Janniaux authored and Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen committed
      The player code in src/player/player.c is setting player->started to
      true when starting a media with vlc_player_Start() and setting it to
      false when calling vlc_player_Stop().
      
      However, vlc_player_Stop() will only queue the input for stopping, so
      the input is stopped asynchronously with no control from the player
      after this.
      
      In the situation where we enabled :play-and-pause on the input, for
      instance using vlc_player_SetMediaStoppedAction(), the media will
      transition itself from PLAYING_S to PAUSE_S from the input thread
      mainloop, asynchronously from any vlc_player_Stop() call.
      
      If vlc_player_Stop() is called soon enough for player->started to be set
      to false, but late enough so that the input thread still has the time to
      reach EOF and trigger pause, PAUSE_S will be reported to the player,
      which will handle it as VLC_PLAYER_STATE_PAUSED in the function
      vlc_player_input_HandleState, and it will expected the player and the
      input to be started, which is in this situation, not true.
      
      Figure 1: interleaving leading to the assertion
      |
      |   PLAYER              DESTRUCTOR               CONTROL/INPUT
      |
      |         vlc_player_Stop()      input_Stop()          |
      |      | ------------------> | ----------------> is_stopped = true
      |      |                                               |
      | player->started = false                          MainLoop
      |      |                       ChangeState(PAUSE_S)    |
      |      | <---------------------------------------------|
      | assert(player->started)                              |
      |                                           while (!input_Stopped(input))
      |                                                      |
      |                                               Input was supposed
      |                                               to be stopped at this
      |                                               point.
      
      CONTROL/INPUT here represents both the control state, modified by
      input_Stop() and read at each MainLoop loop, and the input_thread
      running the MainLoop function.
      
      Other solutions have been taken into account to solve the issues:
      
      1/ Provide a separate set of boolean to track the playback state of the
         player separately from what the user requested (Start/Stop).
      
      This is quite overlapping the existing player->global_state variable and
      it has been confusing to implement and read again. It does restrict the
      testing surface of the assertion anyway so it doesn't bring much
      compared to the submitted approach.
      
      2/ Ensure in vlc_player_Stop() that input_Stop() has been called and
         check whether the input has been stopped before signalling PAUSE_S.
      
      By far, it would have been my preferred method to prevent signaling the
      PAUSE_S state only in the case when it has already been stopped, meaning
      that the current assertion could have stayed the same, ie. that we could
      keep the testing surface on the player state too, but it's unfortunately
      not compatible with the current model.
      
      input_Stop require the lock_control in order to modify the state of the
      input asynchronously, and we'd need input_Stop to wait while we would be
      checking the input state and sending the PAUSE_S state change event. In
      addition vlc_player_Stop and the player callback for input state change
      events are run under player lock.
      
      So vlc_player_Stop() would lock the player (from the outside) and then
      the lock_control, whereas the input thread would lock the lock_control
      to check the state and then the player lock to report the event, leading
      to a lock inversion and thus deadlocks.
      
      Figure 2: fixed interleaving
      |
      |   PLAYER                                       CONTROL/INPUT
      |
      |      |                                           MainLoop
      |      |                                               |
      |      |                                       [lock lock_control]
      |      |                                        from input thread
      |      |                                              ...
      |      |
      |      |  vlc_player_Stop()    input_Stop()
      |      | ----------------------------------> [Waiting lock_control]
      |      |
      |      |                                              ...
      |      |                       ChangeState(PAUSE_S)    |
      |      | <---------------------------------------------|
      | assert(player->started)                              |
      |      |                                       [unlock lock_control]
      |      |                                         from input thread
      |      |                                              ...
      |      |
      |      |                                              ...
      |      |                                               |
      |      |                                      [locked lock_control]
      |      |                                       from player thread
      |      |                                               |
      |      | -------------------------------------> is_stopped = true
      | player->started = false                              |
      |                                                      |
      |                                           while (!input_Stopped(input))
      |                                                      |
      |                                              Input is now dead
      
      3/ Reduce the scope of the assertion.
      
      The current submission reduce the guarantees on the player state, which
      where checking that we couldn't call vlc_player_Pause from the player
      with a stopped player, and only check that the state reported by the
      input is still correct. It does check that we didn't reach END_S, or
      VLC_PLAYER_STATE_STOPPING in the player, when pausing though.
      
      This is enough to fix the assertion, and not confusing to read in the
      code. Note that the check on input->started is also removed since
      input_Stop() will also stop the input asynchronously, leaving the
      possibility for the input to unroll to EOF regardless of whether
      input_Stop is called from vlc_player_Stop() or the destructor thread.
      
      A test has been written to replicate this bug quite reliably on my
      machine, but because of the racy nature of this interleaving and the
      lack of infrastructure to test such interleaving directly in tests, it
      has been removed from this patch.
      
      Fixes #26876
      84659b15
    • Rémi Denis-Courmont's avatar
      lua: remove dead code · 0ce6dd21
      Rémi Denis-Courmont authored and Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen committed
      0ce6dd21
    • Marcin Jakubowski's avatar
      vlcsub: improve movie info matching · 4417cfc6
      Marcin Jakubowski authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      improves movie info (show name, season and episode number) matching by
      trying meta data as well as the cleaned up filename and trying out
      additional patterns
      4417cfc6
    • Steve Lhomme's avatar
      va_surface: fix potential double use of a buffer · 35886895
      Steve Lhomme authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      Substracting 1 to refcount first, means it goes back to unused state
      temporarily. It may be picked by another thread as well before calling
      atomic_fetch_sub().
      
      In the end we don't need the -1,+1 it's already in the "used once" state (2)
      after the atomic_compare_exchange() call.
      35886895
    • Steve Lhomme's avatar
      test: video_output: fix the decoder_UpdateVideoOutput() output value check · caf41ce2
      Steve Lhomme authored and Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen committed
      It returns 0 or -1, not a VLC_xxx error message.
      caf41ce2
    • Steve Lhomme's avatar
      nvdec: fix the decoder_UpdateVideoOutput() output value check · a4c1cefd
      Steve Lhomme authored and Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen committed
      It returns 0 or -1, not a VLC_xxx error message.
      a4c1cefd
    • Steve Lhomme's avatar
      hw:d3d11 fix the decoder_UpdateVideoOutput() output value check · 1186ad17
      Steve Lhomme authored and Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen committed
      It returns 0 or -1, not a VLC_xxx error message.
      1186ad17
  4. May 11, 2022
    • Alexandre Janniaux's avatar
      kate: fix array type for add_integer_with_list · 03467da9
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      03467da9
    • Alexandre Janniaux's avatar
      kate: use var_Inherit instead of var_CreateGet · c17e7baa
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      c17e7baa
    • Alexandre Janniaux's avatar
      kate: fix option list size · 52875ea9
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      52875ea9
    • Alexandre Janniaux's avatar
      kate: use ARRAY_SIZE · 25a05ec4
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      25a05ec4
    • Alexandre Janniaux's avatar
      transcode_scenario: add encoder_close entrypoint · 5e202180
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      The endpoint checks that the encoder is correctly closed when reaching
      the end of the test. Such failure typically happened during the refactor
      of encoders function pointers into a separate virtual table.
      5e202180
    • Alexandre Janniaux's avatar
      test: modules: add transcoder test · a685b7ea
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      A unit-test for the transcoder code currently is hard to achieve, as it
      involves decoders, encoders, filters, sout_stream_t object, and general
      transcode code.
      
      This commit introduces an integration test which mocks the extern
      components (filters, decoders, encoders) used by the transcode pipeline
      and starts this pipeline using the usual input item properties.
      
      The main end goals are to:
       - check whether no part of the stream is dropped.
       - check whether push model with video context is supported everywhere.
       - check the different kind of decoder behaviour, in particular those
         for which decode is asynchronous with the decoder_QueueVideo call,
         those for which Open() is asynchronous with the decoder_UpdateVideo*
         call and their synchronous variants.
       - check memory leaks and use-after-free in the pipeline (through asan).
       - check threading within the pipeline (through tsan).
       - check decoder, encoder and filter loading failure.
       - check format adaptation through filters before the encoder.
       - check format conversion (called final_conv_static) before the
         encoder, needed for encoders requesting a conversion.
      
      It's currently written in an asynchronous state way spread across
      multiple feature functions to handle the different stage of the pipeline
      without interaction, like it was done for the video output test, but we
      might want to find a way to rewrite those tests in a more sequential way
      if it were to get more complex even.
      a685b7ea
Loading