Sometime cases were observed, where the time returned from
pw_stream_get_time_n() had incorrect values.
The root cause was not fully tracked down. Roughly the case was as
follows: A gstreamer pipeline with two gstpipewiresrc elements. One
audio and one libcamera besed video src, both fed into a mp4 encoding
mux. The cock was provided by the audio gstpipewiresrc. Now between
starting the pipewire video stream and receiving the first camera frame
pw_stream_get_time_n() returned strange values (It seemed like it was
using the audio rate for something internally). This wrong value was
used to initialze pclock->start_time and therefore all following calls
to gst_pipewire_clock_get_internal_time() returned completely wrong
values.
Add a warning to hopefully catch these cases. I don't have a proper use
case to test the clock interpolation done in
gst_pipewire_clock_get_internal_time() so I can't drop that (which would
remove the bogus call to pw_stream_get_time_n() alltogether), although
it feels like the right thing to do.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
When a gstreamer pipeline transits to playing state, it sets the
base_time of all elements to the internal time of the current clock. At
that point, the pipewire stream has not yet started and the
gstpipewireclock returns last_time which is initialized to 0 in
gstpipewirestream. This leads to a incorrect base_time in the gstreamer
element and various synchronization issues.
The use-case for last_time is not really clear to me. My basic guesswork
is: If a stream is no longer streaming the internal clock should pause
at that time and return last_time. So this patch keeps this behaviour
in place and only ensures that a valid time is returned when the stream
is not yet started and last_time is not in the future.
To keep the time scaling logic in place, a start time is recorded when
the stream was started, to properly match stream time and clock
monotonic.
A gstreamer pipeline that can be used to replicate the issue is:
GST_DEBUG="pipeline:5,GST_CLOCK:6,pipewiresrc:6" gst-launch-1.0 \
pipewiresrc name=video target-object=<some video target> ! \
video/x-raw,format=UYVY ! fakesink \
pipewiresrc name=audio target-object=<some audio target> ! \
audio/x-raw ! f akesink 2>&1 | \
grep PTS
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
Changes in v2:
- Drop incorrect logic in case s == NULL
- Keep clock scaling in place
It can not generically assumed that the gstreamer clock (and therefore
the base_time) is based on CLOCK_MONOTONIC.
It was tried to use the logic provided by
GstBaseSrc::gst_base_src_do_sync() in commit 004206db37
("gst/pipewiresrc: Let GstBaseSrc handle pseudo-live calculations").
This has the downside, that a potential jitter on the first buffer is
included in the calculated time offset. In gstreamer pipelines with
multiple pipewiresrc elements and big jitter on the first buffer the
streams will stay out of sync.
Improve that by checking if the gstreamer clock is provided by pipewire
and therefore known to be CLOCK_MONOTONIC or if it is provided by
gstreamer and we need to manually calculate the base_time.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Make a new library.filter-path for the filter-graph that will filter and
restrict the dlopen filenames (used for the LADSPA plugin only).
By default this is false and so filter-chain can load from absolute
paths without extra checks.
Enable the extra checks for the pulse LADSPA modules and the
audioconvert filter graphs because these allow loading LADSPA plugins
into other processes.
Fixes#5222
There could have been a write error or allocation error while building
the json file that we can detect in spa_json_builder_close().
Error out instead of silently using a truncated JSON.
Use spa_autofree for the memory to make cleanup easier.
Make the module valid_args a structure that includes the argument key,
description and some flags. Use this to enforce mandatory properties
in a more central place.
We should be able to generate the module usage from this as wel later to
have things a bit more structured.
Make pw_net_get_ip also accept NULL ip to just get the port and ip
version. Make rtsp-client use pw_net_get_ip.
Make sure we initialize the iovec before logging in all cases.
AVB_MRP_VECTOR_GET_NUM_VALUES can be 13 bits and is stored in a
unit16_t. event_len and param_len are however calculated from this and
then truncated to 8 bits (uint8_t) which causes the bounds check to
silently pass and cause an OOB read.
Change the type to uint16_t to avoid overflows.
Move the source offs, stride, data and size calculations out of the
destination loop. We only need to clamp the size to copy to the maxsize
of the destination buffer.
parse_sdp_a_rtpmap used c += strlen(c) + 1 to skip past the MIME type to the
rate/channels part, but if the a=rtpmap: line had no / separator, strcspn
returned the full string length and the +1 advanced past the null terminator.
Fix this by checking if / was actually found, returning -EINVAL if not.
JACK2 only sends -1 as the frames, meaning we should take the value from
the negotiated period as the frames to process.
We however send the actual number of frames and use the sync value to
decide how many frames to process. We need to be careful because a value
of 0 will cause a division by 0 so treat <= 0 frames the negotiated period
size as well.
Check that the params don't include more than MAX_CHANNELS of audio or
else we overflow the position array.
Adapt to the compiled value of SPA_AUDIO_MAX_CHANNELS but allow at least
128 channels.
It's a terrible idea, doesn't work so well (locks up the data-loop when
read is blocked) and a security mightmare. If you really need to pipe
samples through some program, do that somewhere else, like from the
command line with pw-cat and pw-record.
Normally, when loading a plugin feature, often a library.name property
is given as well. If the feature to load is not explicitly listed in
context.spa-libs, the library.name is used a fallback library.
Add an option to ignore this library.name and only use the
context.spa-libs entries. This makes it possible to only load explicitly
listed features in the config file and makes it possible to lock down
what plugins can be loaded.
Set the option to true by default for now, which keeps the existing
behaviour of using the fallback library. Add some more entries to the
context.spa-libs in case the option is switched off to make things
work.
Set the option to false for the minimal.conf.
Add a special 'blocked' spa-libs value that returns EPERM when trying to
load the factory.
Only allow loading the LADSPA filter.graph nodes for the LADSPA sink and
source. The most problematic part is the pipe filter, that allows it to
spawn arbirary programs as part of the filter.graph.
You can add a filter-graph to any stream with stream_props.
Use spa_steal_ptr to transfer props ownership when we can.
This fixes a problem in the upload stream where the props would be freed
twice when buffer allocation failed, once with properties_free and
then with stream_free.
