The fact that the recorder uses a separate thread is an internal detail,
so the functions _start(), _stop() and _join() should not be exposed.
Instead, start the thread on _open() and _stop()+_join() on close().
This paves the way to expose the recorder as a packet sink trait.
The video buffer took ownership of the producer frame (so that it could
swap frames quickly).
In order to support multiple sinks plugged to the decoder, the decoded
frame must not be consumed by the display video buffer.
Therefore, move the producer and consumer frames out of the video
buffer, and use FFmpeg AVFrame refcounting to share ownership while
avoiding copies.
This flag forced the decoder to wait for the previous frame to be
consumed by the display.
It was initially implemented as a compilation flag for testing, not
intended to be exposed at runtime. But to remove ifdefs and to allow
users to test this flag easily, it had finally been exposed by commit
ebccb9f6cc.
In practice, it turned out to be useless: it had no practical impact,
and it did not solve or mitigate any performance issues causing frame
skipping.
But that added some complexity to the codebase: it required an
additional condition variable, and made video buffer calls possibly
blocking, which in turn required code to interrupt it on exit.
To prepare support for multiple sinks plugged to the decoder (display
and v4l2 for example), the blocking call used for pacing the decoder
output becomes unacceptable, so just remove this useless "feature".
The shortcut "back on screen on" is a bit special: the control is
requested by the client, but the actual event injection (POWER or BACK)
is determined on the device.
To properly inject DOWN and UP events for BACK, transmit the action as
a control parameter.
If the screen is off:
- on DOWN, inject POWER (DOWN and UP) (wake up the device immediately)
- on UP, do nothing
If the screen is on:
- on DOWN, inject BACK DOWN
- on UP, inject BACK UP
A corner case is when the screen turns off between the DOWN and UP
event. In that case, a BACK UP event will be injected, so it's harmless.
As a consequence of this change, the BACK button is now handled by
Android on mouse released. This is consistent with the keyboard shortcut
(Mod+b) behavior.
PR #2259 <https://github.com/Genymobile/scrcpy/pull/2259>
Refs #2258 <https://github.com/Genymobile/scrcpy/pull/2258>
The screen receives callbacks from the decoder, fed by the stream.
The decoder is run from the stream thread, so waiting for the end of
stream is sufficient to avoid possible use-after-destroy.
When --no-display was passed, screen_destroy() was called while
screen_init() was never called.
In practice, it did not crash because it just freed NULL pointers, but
it was still incorrect.
A skipped frame is detected when the producer offers a frame while the
current pending frame has not been consumed.
However, the producer (in practice the decoder) is not interested in the
fact that a frame has been skipped, only the consumer (the renderer) is.
Therefore, notify frame skip via a consumer callback. This allows to
manage the skipped and rendered frames count at the same place, and
remove fps_counter from decoder.
As soon as the stream is started, the video buffer could notify a new
frame available.
In order to pass this event to the screen without race condition, the
screen must be initialized before the screen is started.
Video buffer is a tool between a frame producer and a frame consumer.
For now, it is used between a decoder and a renderer, but in the future
another instance might be used to swscale decoded frames.
It makes sense to extract default values for bitrate and port range
(which are arbitrary and might be changed in the future).
However, the default values for "max size" and "lock video orientation"
are naturally unlimited/unlocked, and will never be changed. Extracting
these options just added complexity for no benefit, so hardcode them.
After the struct screen is initialized, the window, the renderer and the
texture are necessarily valid, so there is no need to check in
screen_destroy().
There were only two frames simultaneously:
- one used by the decoder;
- one used by the renderer.
When the decoder finished decoding a frame, it swapped it with the
rendering frame.
Adding a third frame provides several benefits:
- the decoder do not have to wait for the renderer to release the
mutex;
- it simplifies the video_buffer API;
- it makes the rendering frame valid until the next call to
video_buffer_take_rendering_frame(), which will be useful for
swscaling on window resize.
The functions SDL_malloc(), SDL_free() and SDL_strdup() were used only
because strdup() was not available everywhere.
Now that it is available, use the native version of these functions.
Small unsigned integers promote to signed int. As a consequence, if v is
a uint8_t, then (v << 24) yields an int, so the left shift is undefined
if the MSB is 1.
Cast to uint32_t to yield an unsigned value.
Reported by USAN (meson x -Db_sanitize=undefined):
runtime error: left shift of 255 by 24 places cannot be represented
in type 'int'
The current process could be waited both by run_file_handler() and
file_handler_stop().
To avoid the race condition, wait the process without closing, then
close with mutex locked.
There were two versions: process_wait() and process_wait_noclose().
Expose a single version with a flag (it was already implemented that way
internally).
The function process_wait() returned a bool (true if the process
terminated successfully) and provided the exit code via an output
parameter exit_code.
But the returned value was always equivalent to exit_code == 0, so just
return the exit code instead.
The size, point and position structs were defined in common.h. Move them
to coords.h so that common.h could be used for generic code to be
included in all source files.
The header libavformat/version.h was included, but not
libavcodec/version.h.
As a consequence, the LIBAVCODEC_VERSION_INT definition depended on the
caller includes.
On Linux, waitpid() both waits for the process to terminate and reaps it
(closes its handle). On Windows, these actions are separated into
WaitForSingleObject() and CloseHandle().
Expose these actions separately, so that it is possible to send a signal
to a process while waiting for its termination without race condition.
This allows to wait for server termination normally, but kill the
process without race condition if it is not terminated after some delay.
Let the server terminate properly once all the sockets are closed.
If it does not terminate (this can happen if the device is asleep), then
kill it.
Note: since the server process termination is detected by a flag set
after waitpid() returns, there is a small chance that the process
terminates (and the PID assigned to a new process) before the flag is
set but before the kill() call. This race condition already existed
before this commit.
Fixes#1992 <https://github.com/Genymobile/scrcpy/issues/1992>
TerminateProcess() is "equivalent" to kill(), while
WaitForSingleObject() is "equivalent" to waitpid(), so the handle must
be closed after WaitForSingleObject().
The (non-daemon) threads were not interrupted on video stream stopped,
leaving the server process alive.
Interrupt them to wake up their blocking call so that they terminate
properly.
Refs #1992 <https://github.com/Genymobile/scrcpy/issues/1992>
Before this change, release.sh built some native stuff, and
Makefile.CrossWindows built the Windows releases.
Instead, use a single release.make to build the whole release. It also
avoids to build the server one more time.
On Windows, scrcpy paused on error before exiting to give the user a
chance to see the user message.
This was a hack and causes issues when using scrcpy from batch scripts.
Disable this pause from the scrcpy binary, and provide a batch wrapper
(scrcpy-console.bat) to pause on error.
Fixes#1875 <https://github.com/Genymobile/scrcpy/issues/1875>
Romain Vimont