C compilers can have GNU extensions to support typeof in C code, but
some C++ compilers like clang are removing the builtin since decltype
can be used in C++ without the constraints from typeof. Decltype is not
100% equivalent for this reason: references will be kept in the returned
type.
The check in m4/typeof.m4 comes from graydon/monotone and dovecot/core
and was slightly modified to namespace the define for C++ code.
It exists in very old gcc and clang.
VLC_WARN_CALL/VLC_ERROR_CALL may not warn though.
Don't define check_delay/check_deadline otherwise and don't force the
vlc_tick_sleep/vlc_tick_wait calls.
Windows can send input events even if there is no display, so the
filter chains may need to be accessed even then. This protects the
creation and destruction of the chains with the existing filter lock,
providing memory safety.
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.
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
If there is no display, then not only we cannot translate the
coordinates to something meaningful, but the filters and the mouse
event callback are not initialised yet, so there is also nothing to
pass the events onto (see what ProcessMouseState() does).
This also prevent the leak of sys happening when the wrong type of
window is used when opening the display, and move the test after the
trivial check for window type.
This doesn't imply anything more on the thread system and just
deallocate memory, so there is no need to add that much constraint on
the function usage.
The CGL context was destroyed too early, so move around the
vout display removal to prevent using the already-gone context.
Forward port from 3.0.x branch.
Cherry-picked from commit 7d1e7f289d.
Signed-off-by: Alexandre Janniaux <ajanni@videolabs.io>
This is anyway not properly supported currently so opting in to it
here does not change anything for the better.
Forward port from 3.0.x branch.
Cherry-picked from commit 4591255d8e.
Signed-off-by: Alexandre Janniaux <ajanni@videolabs.io>
sys->embed was used to store the window created from the display in
pre-3.0 design, but now the display is created after the window and the
window is available in vd->cfg->window.
The sys->embed was storing the same pointer in the normal case, but
ironically, it was undefined in the libvlc embedding case and was
leading to crash.
libtool is notoriously known to have been patched over the time and was
provided as 2.4.7-dirty on archlinux, which makes the integer comparison
test fail since 7-dirty is not an integer.
shell || nicely handles the commands on the other lines without escaping
the end of line, which provides better error message in case of error.
In addition, priority of operators is much more obvious since the []
test syntax provides a visible scope-like feeling.
Only the visualisation aout filters need the clock for the vout (for
now).
Create one slave clock per visualisation aout filter instead of always
creating one for all visualisation aout filters.
The SMB modules cannot do the Bonjour lookup, so resolve the hostname
and forward the first IP, which typically is the preferred value.
This fixes vlc-ios#1319
Alexandre Janniaux
Steve Lhomme
Rémi Denis-Courmont
Thomas Guillem
Marvin Scholz
Francois Cartegnie
Felix Paul Kühne
Dave Nicolson