Remove the QUEUED flags to check if a buffer is in some queue.
Add a new flag to check if a buffer was dequeued by the application.
Check if the application only queues buffers with the DEQUEUED flag set.
The flag was used to see if a buffer was in a queue or not but that
doesn't really matter much and with the DEQUEUED flag we can only move
buffers from dequeued to queued.
When renegotiating stream parameters (e.g. size), the buffers
are cleared should no longer be queued back. Add a flag to detect this,
while logging a warning and erroring out when the user tries to queue
such a buffer.
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.
This is required in order to allow plugins to use GL as mincore
is used in Mesas `_eglPointerIsDereferenceable()`.
One example for a client wanting to do so is the in-development
libcamera GPUISP, see https://patchwork.libcamera.org/cover/24183/
Add sample limit switch -n to pw-cat to stop the recording or playback
after a set number of samples received.
Change-Id: Iaa551db9849acd6acdb6897dbfaa92a21afa1312
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.)
Downstream elements accessing dmabufs from the CPU currently need to map
and unmap buffers on every frame.
While the kernel's page cache avoids most overhead, this still requires a
round-trip through the kernel and possibly non-negligible work in
`dma_buf_mmap()`. Keeping the buffers mapped avoids that without causing
additional syncronization work, as the later should only happen when
`DMA_BUF_IOCTL_SYNC` is triggered in `gst_dmabuf_mem_map()`.
A common scenario where this matters is clients using cameras. The
downstream elements in question may not be aware of dmabufs - e.g.
`videoconvert` - or fail to import the dmabuf and fall back to import
from memory - e.g. `glupload`.
Notes:
- GstShmAllocator implicitly does this already.
- We could also do this in the MemFd case, however I'm less convinced
about the trade-offs.
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..
cd68819feb added code to follow the state
change of the node because at the time it the Start code was async and
it was better to complete it before emitting the state.
9b80855821 however made the Start command
sync again and so we can safely emit the running state in the Start.
The effect is that the Running state change is emitted first and then
the node eventfd is added to the data loop that can then call the process
callback. Having the process callback happen before the RUNNING state
change is unexpected and racy.
The idea of fd2db174c1 was to allow for
property updates after the global was registered but this check was not
removed to actually make this happen.
Fixes#4851
Follow various other elements like glupload and gtk4paintablesink and
print the negotiated caps at a higher priority than debug. A small
quality of life improvement to facilitate debugging.
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.
This allows to use clang-include-cleaner tool without <pipewire/foo.h>
headers being automatically added when <pipewire/pipewire.h> is included
instead.
See https://issues.webrtc.org/issues/422940461
At the moment errors are printed using `pw_log_error()`, however, that does
not display them by default because `client.conf` sets `log.level=0`.
Use `fprintf()` and always print the errors.
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
