Just making a port and adding it to a node does not make it a working
port..
Make a new node function to get a new free port, this will actually call
the implementation spa_node_add_port(), which will add the new port
which we can then pick up and use.
See #4876
Only send out SUSPENDED event when there is a change in the suspended
state. This avoids sending out unsuspend events when we simply uncork.
Implement the fail_on_suspend flag for capture and playback streams.
Instead of suspending those streams, we need to kill them.
The started boolean is insufficient to fully cover the possible internal
states. For this reason, it needs to be replaced by an enum that covers
these states.
Also, due to potential access by both the dataloop and the mainloop,
access to that internal state needs to be synchronized.
Finally, a variable "internal_state" makes for code that is easier to
read, since it emphasizes that this is state that is fully internal
inside the stream (and is not visible to the rtp-sink and rtp-source
modules for example).
The state_changed callbacks fulfill multiple roles, which is both a problem
regarding separation of concerns and regarding code clarity. De facto,
these callbacks cover error reporting, opening connections, and closing
connection, all in one, depending on a state that is arguably an internal
stream detail. The code in these callbacks tie these internal states to
assumptions that opening/closing callbacks is directly tied to specific
state changes in a common way, which is not always true. For example,
stopping the stream may not _actually_ stop it if a background send timer
is still running.
The notion of a "state_changed" callback is also problematic because the
pw_streams that are used in rtp-sink and rtp-source also have a callback
for state changes, causing confusion.
Solve this by replacing state_changed with three new callbacks:
1. report_error : Used for reporting nonrecoverable errors to the caller.
Note that currently, no one does such error reporting, but the feature
does exist, so this callback is introduced to preserve said feature.
2. open_connection : Used for opening a connection. Its optional return
value informs about success or failure.
3. close_connection : Used for opening a connection. Its optional return
value informs about success or failure.
Importantly, these callbacks do not export any internal stream state. This
improves encapsulation, and also makes it possible to invoke these
callbacks in situations that may not neatly map to a state change. One
example could be to close the connection as part of a stream_start call
to close any connection(s) left over from a previous run. (Followup commits
will in fact introduce such measures.)
When a link enters the "ERROR" state, it is scheduled for destruction in
`module-link-factory.c:link_state_changed()`, which queues `destroy_link()`
to be executed on the context's work queue.
However, if the link is destroyed by means of `pw_impl_link_destroy()`
directly after that, then `link_destroy()` unregisters the associated
`pw_global`'s event hook, resulting in `global_destroy()` not being called
when `pw_impl_link_destroy()` proceeds to call `pw_global_destroy()` some
time later. This causes the scheduled async work to not be cancelled. When
it runs later, it will trigger a use-after-free since the `link_data` object
is directly tied to the `pw_impl_link` object.
For example, if the link is destroyed when the client disconnects:
==259313==ERROR: AddressSanitizer: heap-use-after-free on address 0x7ce753028af0 at pc 0x7f475354a565 bp 0x7ffd71501930 sp 0x7ffd71501920
READ of size 8 at 0x7ce753028af0 thread T0
#0 0x7f475354a564 in destroy_link ../src/modules/module-link-factory.c:253
#1 0x7f475575a234 in process_work_queue ../src/pipewire/work-queue.c:67
#2 0x7b47504e7f24 in source_event_func ../spa/plugins/support/loop.c:1011
[...]
0x7ce753028af0 is located 1136 bytes inside of 1208-byte region [0x7ce753028680,0x7ce753028b38)
freed by thread T0 here:
#0 0x7f475631f79d in free /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:51
#1 0x7f4755594a44 in pw_impl_link_destroy ../src/pipewire/impl-link.c:1742
#2 0x7f475569dc11 in do_destroy_link ../src/pipewire/impl-port.c:1386
#3 0x7f47556a428b in pw_impl_port_for_each_link ../src/pipewire/impl-port.c:1673
#4 0x7f475569dc3e in pw_impl_port_unlink ../src/pipewire/impl-port.c:1392
#5 0x7f47556a02d8 in pw_impl_port_destroy ../src/pipewire/impl-port.c:1453
#6 0x7f4755634f79 in pw_impl_node_destroy ../src/pipewire/impl-node.c:2447
#7 0x7b474f722ba8 in client_node_resource_destroy ../src/modules/module-client-node/client-node.c:1253
#8 0x7f47556d7c6c in pw_resource_destroy ../src/pipewire/resource.c:325
#9 0x7f475545f07d in destroy_resource ../src/pipewire/impl-client.c:627
#10 0x7f47554550cd in pw_map_for_each ../src/pipewire/map.h:222
#11 0x7f4755460aa4 in pw_impl_client_destroy ../src/pipewire/impl-client.c:681
#12 0x7b474fb0658b in handle_client_error ../src/modules/module-protocol-native.c:471
[...]
Fix this by cancelling the work queue item in `link_destroy()`, which should
always run, regardless of the ordering of events.
Fixes#4691
Improve the spa_ump_to_midi function so that it can consume multiple UMP
messages and produce multiple midi messages.
Some UMP messages (like program changes) need to be translated into up
to 3 midi messages. Do this byt adding a state to the function and by
making it consume the input bytes, just like the spa_ump_from_midi
function.
Adapt code to this new world. This is a little API break..
This fixes the case when synchronization is established but actually not
valid anymore. In such a case, the code would _first_ write to the ring
buffer (at the wrong position due to the invalid sync), and _then_ detect
the bogus synchronization. Reorder the code blocks to _first_ check the
current sync, then resynchronize if neeeded (or perform initial sync if
no sync is established yet), and _then_ write to the ring buffer.
Until now, the timestamp check was comparing the timestamp delta against
the value of the "quantum" variable. However, the timestamps use clock
samples as units, while the "quantum" variable uses nanoseconds. The
outcome is that this check virtually never returned true. Use the
spa_io_clock duration instead of that quantum nanosecond duration to make
the check actually work.
Also, do not just rely on vast timestamp deltas to detect discontinuities;
instead, check first for the presence of the SPA_IO_CLOCK_FLAG_DISCONT
flag to detect said discontinuities.
Instead of implicitly acting on the current thread if the provided thread
handle is unknown, return an error. This behaviour is not depended on inside
pipewire, and if needed, special casing e.g. `thread == NULL` could be added,
but any random `thread` value shouldn't affect the current thread.
Make a new body.h file with some functions to deal with pod and their
body. Make the iter.h functions use mostly this.
Rework the parser so that it only uses body.h functions. With the separation
of pod+body, we can read and verify the pod once and then use the
verified copy to handle the rest of the body safely.
We do this because iter.h only works in pods in memory that doesn't change
because it is vulnerable to modifications of the data after verifying it.
The new parser is not vulnerable to this and will not cause invalid
memory access when used on shared memory. There is however no need for
atomic operations to read the headers, whever is read is either valid
and useable of invalid and rejected.
See #4822
The parser does not check that POD arrays have the correct size for
their type, so the calling code must do that.
This also enumerates some of the code that cannot handle the size of the
values of an array not being the exact expected size for its type.
There is a lot of it.
Direct timestamp mode was incorrectly using over/underrun detection logic
and fill level tracking logic that is actually meant for the other mode
(referred to from now on as "constant latency mode"). Over/underruns are
tracked implicitly in the direct timestamp mode, and the absolute fill
level is not relevant in that mode, since the latency is not needed to
be constant then.
Also improve log lines and the RTP module documentation to define these
buffer modes clearly and explain their differences and use cases.
Opus and MIDI code get TODOs added, since their direct timestamp mode
implementations still may be incorrect. Fixing those will be done in
a separate commit.
When a stream has some delay, a time t1 + delay has to be read in time
t1 to play it when expected.
Decrease target_buffer by delay to start playback sooner, so sound
is played at correct time when delay is applied.
Signed-off-by: Martin Geier <martin.geier@streamunlimited.com>
We also need to close the SynObj fd we got, just like we close any
DmaBuf or MemFd.
Make sure we get a compiler error when we add more items to the
data type enumeration later.
Fixes#4807
It uses the onnxruntime library to parse the onnx file and construct a
neural network. It uses the label field to setup the plugin and how to
map the various tensors of the model to input, output, control and
notify ports.
Add an example config for how to use the silero VAD ONNX model with the
noise gate.
The return value is always 0, and the `impl` parameter
is not used, so ues the return value to return the boolean
result instead of an out parameter, and get rid of the
unused argument.
Instead of using a new macro with the `PW_` prefix, simply define
`SCHED_RESET_ON_FORK` to be `0` when it is not defined; as the
prefixed variant can be a bit confusing.
