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
    • Alexandre Janniaux's avatar
      transcode: encoder: remove encoder_test function · 77cfe38a
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      The encoder test is not used anymore, we'll try to open the encoder at
      the moment we have something to encode and using the output of filters.
      77cfe38a
    • Alexandre Janniaux's avatar
      transcode: remove unused transcode_dequeue_all_pics · 41600043
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      Pictures are now pushed towards the encoder directly and not queued for
      encoding.
      41600043
    • Alexandre Janniaux's avatar
      transcode: fix asynchronous decoder encoding · d8c883e2
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      Some decoders have the decoder_QueueVideo called asynchronously wrt. the
      decode() call, which is allowed and expected by the API. The MediaCodec
      decoder also has a limited number of input buffers and output buffers
      when mediacodec-dr is enabled.
      
      This creates a strong backpressure linking the output buffers to the
      input ones, meaning that no input buffer can be decoded unless the
      already submitted output buffers are processed and released.
      
      By running the decode() call followed by the processing of output
      buffers at the same location, we're linking the output buffers
      processing to the execution of the decode() call: no processing of
      outputs can happen until decode() hasn't returned.
      
      Except that decode() can be called to saturate the input buffers while
      the output was already saturated, leading it to a non deterministic
      deadlock.
      
      By processing the buffer synchronously with the decoder_QueueVideo()
      call, we ensure that the processing of the picture happens regardless of
      the decode() blocking state. It will typically call the filter and
      encode callbacks from the thread of the asynchronous module or from the
      decode callback itself for synchronous decoders, leading to either the
      same behaviour or difference wrt. depth-first vs breadth-first
      processing at the decoder level (do we decode all the pictures we can or
      do we process every new picture before decoding the others).
      
      The refactor is made so that the actual processing function is isolated
      and can be reused in a latter refactor involving a new thread if needed
      to bring those difference in execution.
      
      The previous transcode function is still handling the decoder and
      encoder drain(), waiting for the decoder to queue all the video frames
      and then waiting for the encoder to output every packets.
      
      It fixes a systematic non-deterministic deadlock happening when having
      the MediaCodec decoder in direct rendering mode within the transcode
      pipeline, when implementing #25153.
      d8c883e2
    • Alexandre Janniaux's avatar
      transcode: video: remove unused functions · 9722e11c
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      9722e11c
    • Alexandre Janniaux's avatar
      transcode: encoder: avoid format update after open · b04216b2
      Alexandre Janniaux authored and Felix Paul Kühne's avatar Felix Paul Kühne committed
      When the encoder is opened, it cannot change format anymore, and we
      cannot change it behind its back.
      b04216b2
Loading