Previously, the user input for stsd entries is trusted completely, and
so a maliciously crafted file could choose the length of the stsd
entries arbitrarily and cause qtdemux to try to allocate up to 2GB of
memory (half of a 32 bit max int).
This patch fixes this by sanity checking the stsd input against the
size of the entire stsd atom.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/670>
During trak parsing, we need to check for the existence of stsd_entries,
otherwise, we end up with a NULL pointer to them. It is entirely
possible for the stsd to exist, but for it to have no entries, which the
previous checks did not take into account.
This patch adds a simply check to ensure that all files that do not
contain a stsd entry are deemed corrupt, and adds a test case to prevent
a regression.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/670>
Previously imagefreeze would always operate as non-live element and
output frames as fast as possible according to the configured segment
(via SEEK events) and the negotiated framerate from start to stop or the
other way around.
With the new live mode (enabled via the is-live property) it would only
output frames in PLAYING. Frames would be output according to the
negotiated framerate unless it would be too late, in which case it would
jump ahead and skip over the requirement amount of frames.
This makes it possible to actually use imagefreeze in live pipelines
without having to manually ensure somehow that it would start outputting
at the current running time and without still risking to fall behind
without recovery.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/653>
We never run as a live element, even if upstream is live, and never
output any buffers with latency but immediately generate buffers as
fast as we can according to the negotiated framerate.
Passing the query upstream would proxy whatever mode of operation
upstream has, which has nothing to do with how we produce buffers.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/653>
Move the SVMI stereoscopic atom parsing out to a helper
function to shrink qtdemux_parse_trak a bit.
Add a bounds check that the received atom is large enough
before parsing it.
Add a note to the atom parser that svmi comes from the
MPEG-A spec 23000-11.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/634>
Until now, do_expected_timeout() was shortly dropping the JBUF_LOCK in order
to push RTX event event without causing deadlock. As a side effect, some
CPU hung would happen as the timerqueue would get filled while looping over
the due timers. To mitigate this, we were processing the lost timer first and
placing into a queue the remainign to be processed later.
In the gap caused by an unlock, we could endup receiving one of the seqnum
present in the pending timers. In that case, the timer would not be found and
a new one was created. When we then update the expected timer, the seqnum
would already exist and the updated timer would be lost.
In this patch we remove the unlock from do_expected_timeout() and place all
pending RTX event into a queue (instead of pending timer). Then, as soon as
we have selected a timer to wait (or if there is no timer to wait for) we send
all the upstream RTX events. As we no longer unlock, we no longer need to pop
more then one timer from the queue, and we do so with the lock held, which
blocks any new colliding timers from being created.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/616>
When a GST_EVENT_FLUSH_START reaches the jitterbuffer, there is a chance that
our task is currently blocking waiting for a timer.
There was two problems:
* That wait wasn't checking for flushing situations
* The flushing handling wasn't waking up that conditional (to check whether it
should abort)
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/608>
In some algorithms (like yadif), the Y plane has to be handled different
than the UV plane. Therefore, the planar_y functions are now called for
the Y plane, and the nv12/nv21 functions are handling only the UV/VU
planes respectively.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/444>
Switching the deinterlacing mode on-the-fly from disabled to
auto used to work, but was broken by commit #1f21747c some
years ago.
Force re-negotiation with downstream when the mode or
fields properties are changed, otherwise deinterlace
never switches out of the passthrough mode.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/584>
First of all get rid of the atomic seeking boolean, which was only ever
set and never read. Replace it with a flushing boolean that is used in
the loop function to distinguish no buffer because of flushing and no
buffer because of an error as otherwise we could end up in a
GST_FLOW_ERROR case during flushing.
Also only reset the state of imagefreeze in flush-stop when all
processing is stopped instead of doing it as part of flush-start.
And last, get a reference to the imagefreeze buffer in the loop function
in the very beginning and work from that as otherwise it could in theory
be replaced or set to NULL in the meantime as we release and re-take the
mutex a couple of times during the loop function.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/580>
Backwards timestamps confuse librtmp, even if they're only backwards
relative to the other stream. If the timestamp of a stream is going
backwards related to the other stream, this property allows the muxer to
skip a few buffers until it reaches the timestamp of the other stream.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/572>
Allows us to request pads after writing header for streamable flv's.
For non-streamable it doesn't make sense to request a new pad after
writing the header, because the headers have been written already and we
can't add the new stream. But for streamable, any clients that connect
after the new pad has been added will be able to see both streams.
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/572>
As we override the GLib item with our own structure, we cannot use any
function from GList or GQueue that would try to free the RTPJitterBufferItem.
In this patch, we move away from g_queue_new() which forces using
g_queue_free(). This this function could use g_slice_free() if there is any items
left in the queue. Passing the wrong size to GSLice may cause data corruption
and crash.
A better approach would be to use a proper intrusive linked list
implementation but that's left as an exercise for the next person
running into crashes caused by this.
Be ware that this regression was introduced 6 years ago in the following
commit [0], the call to flush() looked useless, as there was a g_queue_free()
afterward.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
[0] 479c7642fd
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/573>
The calculated threshold for timecode might be varying depending on
"max-size-timecode" and framerate.
For instance, with framerate 29.97 (30000/1001) and
"max-size-timecode=00:02:00;02", every fragment will have identical
number of frames 3598. However, when "max-size-timecode=00:02:00;00",
calculated next keyframe via gst_video_time_code_add_interval()
can be different per fragment, but this is the nature of timecode.
To compensate such timecode drift, we should keep track of expected
timecode of next fragment based on observed timecode.
The problem was this:
Due to the highly irregular arrival of RTX-packet the max-misorder variable
could be pushed very low. (-10).
If you then at some point get a big in the sequence-numbers (62 in the
test) you end up sending RTX-requests for some of those packets, and then
if the sender answers those requests, you are going to get a bunch of
RTX-packets arriving. (-13 and then 5 more packets in the test)
Now, if max-misorder is pushed very low at this point, these RTX-packets
will trigger the handle_big_gap_buffer() logic, and because they arriving
so neatly in order, (as they would, since they have been requested like
that), the gst_rtp_jitter_buffer_reset() will be called, and two things
will happen:
1. priv->next_seqnum will be set to the first RTX packet
2. the 5 RTX-packet will be pushed into the chain() function
However, at this point, these RTX-packets are no longer valid, the
jitterbuffer has already pushed lost-events for these, so they will now
be dropped on the floor, and never make it to the waiting loop-function.
And, since we now have a priv->next_seqnum that will never arrive
in the loop-function, the jitterbuffer is now stalled forever, and will
not push out another buffer.
The proposed fixes:
1. Don't use RTX in calculation of the packet-rate.
2. Don't use RTX in large-gap logic, as they are likely to be dropped.
If the start of the GOP is >= the requested running time, put it into a
new fragment. That is, split-at-running-time would always ensure that a
split happens as early as possible after the given running time.
Previously it was comparing against the current incoming timestamp,
which does not tell us what we actually want to know as it has no direct
relation to the GOP start/end.
This is disabled by default as it unnecessarily creates bigger headers
but it is something that is required by some applications and most
notably the Apple ProRes spec.
Request pads can released at any time, so make sure to hold
the object lock when iterating the element sinkpads list where
that's safe, or to use other safe pad iteration patterns in
other places.
When choosing a best pad, return a reference to the pad to make sure it
stays alive for output in the aggregator srcpad task.
Should fix a spurious valgrind error in the CI flvmux tests and some
other potential problems if the request sink pads are released while
the element is running..
Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/issues/714
Even if timecode trak is officially unsupported in non-mov flavors,
some software still supports it, e.g. Final Cut Pro X:
https://developer.apple.com/library/archive/technotes/tn2174/_index.html
The user might still expect to see the timecode information in the
non-mov file despite it being officially unsupported , because other
software e.g. QuickTime will create a timecode trak even in mp4 files.
Furthermore, software that supports timecode trak in non-mov flavors
will also display the file duration in "timecode units" instead of real
clock time, which is not necessarily the same for 29.97 fps and friends.
This might confuse users, who see a different duration for the same
framerate and amount of frames depending on whether the container is mp4
or mov.
Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/512
gst_buffer_map () results in memcopying when a GstBuffer contains
more than one GstMemory.
This has quite an impact on performance on systems with limited amount
of resources. With this patch the whole GstBuffer will not be mapped at
once, instead each individual GstMemory will be iterated and mapped
separately.
There is a use-case for a server to re-payload opus going through it.
Problem was that the payloader requires channels in the caps, but
this is not something the depayloader can parse out of the stream, meaning
caps-negotiation would fail.
Removing the requirement of channels in the template-caps fixes this.
RTP session starts a new thread for RTCP and names it
"rtpsession-rtcp-thread" which happens to be longer than the maximum 16B
allowed by pthread_setname_np and causes the naming to fail.
See docs for more details.
This commit simply shortens the thread's name so it can actually be set.
Changing the types from boolean to guint due to the ++ operand used on
them, and only call JBUF_SIGNAL_QUEUE after settling down,
or else you end up signaling the waiting code in chain() for every buffer
pushed out.
When switching the splitmuxsrc state back to NULL quickly, it
can encounter deadlocks shutting down the part readers that
are still starting up, or encounter a crash if the splitmuxsrc
cleaned up the parts before the async callback could run.
Taking the state lock to post async-start / async-done messages can
deadlock if the state change function is trying to shut down the
element, so use some finer grained locks for that.
The queued time includes the duration of the last queued frame
(i.e., new keyframe) so the condition check should not be inclusive.
Note that the new fragment will be cut excluding the last frame
and therefore if the condition is inclusive way,
the fragment might have one frame shorter duration for all keyframe
stream such as jpeg or all-inter video streams.
Since the commit 94bb76b6b9, splitmuxsink
will split fragments based on queued time and the threshold of that.
So don't need to store the next timecode for split decision.
Not only the requested keyframe time, the queued size should be
a criterion for the split decision of timecode based mode
(same as max-size-time based split case).
This is a concept that only applies when a buffer arrives in the chain
function, and it has already been scheduled as part of a "multi"-lost
timer.
However, "multi"-lost timers are now a thing of the past, making this
whole concept superflous, and this buffer is now simply counted as "late",
having already been pushed out (albeit as a lost-event).
There is a problem with the code today, where a single timer will
be scheduled for a series of lost packets, and then if the first packet
in that series arrives, it will cause a rescheduling of that timer, going
from a "multi"-timer to a single-timer, causing a lot of the packets
in that timer to be unaccounted for, and creating a situation in where
the jitterbuffer will never again push out another packet.
This patch solves the problem by instead of scheduling those lost packets
as another timer, it instead asks to have that lost-event pushed straight
out.
This very much goes with the intent of the code here: These packets are
so desperately late that no cure exists, and we might as well get the
lost-event out of the way and get on with it.
This change has some interesting knock-on effect being presented in
later commits. It completely removes the concept of "already-lost", so
that is why that test has been disabled in this commit, to be
removed later.
This should result in no worse accuracy than the base parse element, and may
result in better accuracy. In particular, the number of bytes processed at any
given point, as accumulated by baseparse, can be only accurate to
(1 / # of frames) bytes per second, and if we try to seek immediately after
pausing the pipeline to a large offset, this small inaccuracy can propagate to
something noticeable.
The use case that prompted this patch is a 45-minute MPEG-1 layer 3 file, which
has a constant bit rate but no seek tables. Trying to seek the pipeline
immediately after pauisng it, without the ACCURATE flag, to a location 41
minutes in, yields a location that is, even with <https://gitlab.freedesktop.org/gstreamer/gstreamer/merge_requests/374>,
still audibly incorrect. This patch yields a much closer position, no longer
audibly incorrect, and likely within a frame of the most correct position.
By the time sink_event is called, the pad's current caps have
already been updated. To address this, implement sink_event_pre_queue,
and check if the pad can be renegotiated there.
Fixes#707
- Refactored the planar transform method to support all video formats
that are stored planar, independent of the used subsampling
- Added support for Y444
gst_buffer_map () results in memcopying when a GstBuffer contains
more than one GstMemory and when AVC (length-prefixed) alignment is used.
This has quite an impact on performance on systems with limited amount of
resources. With this patch the whole GstBuffer will not be mapped at once,
instead each individual GstMemory will be iterated and mapped separately.
When calling gst_rtp_jitter_buffer_reset you pass in a seqnum.
This is considered the starting-point for a new stream.
However, the old behavior would unref this buffer, basically lying to
the thread that is pushing out buffers saying that it can expect
this buffer, when it would never arrive. The resulting effect being no
more buffer pushed out of the jitterbuffer, and it would buffer
incoming data indefinitely.
By instead inserting the buffer in the gap_packets queue, the _reset()
function will take responsibility for using that as the first buffer
of the new stream.
Fixes#703
In order to concatenate fragments, splitmuxsrc offsets
the start of each fragment PTS to 0 to align it with the
previous file. This means that DTS can go negative for
the first fragment, with really bad results.
Add a fixed offset to outgoing timestamp ranges to
avoid that.
In this mode each field is carried using its own buffer.
Allow deinterlace to negotiate caps with the Interlaced feature and
adjust the algorithm fetching lines.
Fix#620
Output frames used to have their interlace mode set to the same one as
the input. This breaks their field and comp heights when deinterlacing
an alternate stream.
Each FLAC metadata block starts with a flag denoting whether it is the
last metadata block. The existing flacparse code moves any existing
VORBISCOMMENT block to immediately follow the STREAMINFO block without
changing any block's last-metadata-block flag. If no VORBISCOMMENT block
exists, it created one with the last-metadata-block flag set to true.
This results in gstflacdec sometimes giving bad headers to libflac when
trying to play perfectly valid FLAC files depending on the file's
metadata ordering. Depending on the contents of the other metadata
blocks, current versions of libflac may or may not return
FLAC__STREAM_DECODER_ERROR_STATUS_BAD_HEADER when given this broken
metadata. This is most noticeable with files that have a large cover art
image attached where VORBISCOMMENT is the very last metadata block with
no PADDING afterwards.
This patch changes that behavior so that:
1. For FLAC files that already have a VORBISCOMMENT block, the metadata
order is preserved.
2. For FLAC files that do not have a VORBISCOMMENT block, the generated
dummy VORBISCOMMENT is placed immediately after STREAMINFO and
inherits the last-metadata-block flag from STREAMINFO.
https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/484
Sometimes the headers contain useless, wrong or zero values for e.g. the
sample size with these formats. There's only a single valid value for
them so let's set these instead.
We do not have a way to know the format modifiers to use with string
functions provided by the system. G_GUINT64_FORMAT and other string
modifiers only work for glib string formatting functions. We cannot
use them for string functions provided by the stdlib. See:
https://developer.gnome.org/glib/stable/glib-Basic-Types.html#glib-Basic-Types.description
```
../gst/rtpmanager/gstrtpjitterbuffer.c: In function 'gst_jitter_buffer_sink_parse_caps':
../gst/rtpmanager/gstrtpjitterbuffer.c:1523:32: error: unknown conversion type character 'l' in format [-Werror=format=]
|| sscanf (mediaclk, "direct=%" G_GUINT64_FORMAT, &clock_offset) != 1)
^~~~~~~~~~
In file included from /home/nirbheek/cerbero/build/dist/windows_x86/include/glib-2.0/glib/gtypes.h:32,
from /home/nirbheek/cerbero/build/dist/windows_x86/include/glib-2.0/glib/galloca.h:32,
from /home/nirbheek/cerbero/build/dist/windows_x86/include/glib-2.0/glib.h:30,
from /home/nirbheek/cerbero/build/dist/windows_x86/include/gstreamer-1.0/gst/gst.h:27,
from /home/nirbheek/cerbero/build/dist/windows_x86/include/gstreamer-1.0/gst/rtp/gstrtpbuffer.h:27,
from ../gst/rtpmanager/gstrtpjitterbuffer.c:108:
/home/nirbheek/cerbero/build/dist/windows_x86/lib/glib-2.0/include/glibconfig.h:69:28: note: format string is defined here
#define G_GUINT64_FORMAT "llu"
^
../gst/rtpmanager/gstrtpjitterbuffer.c:1523:32: error: too many arguments for format [-Werror=format-extra-args]
|| sscanf (mediaclk, "direct=%" G_GUINT64_FORMAT, &clock_offset) != 1)
^~~~~~~~~~
```
See also: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/379
Previously we saved the buffer_timestamp straight into
mux->cluster_time. Since the cluster time saved into the file does not
have as high precision as GstClockTime depending on the timecodescale
the rounding of relative_timestamp was invalid as mux->cluster_time
which it was calculated relative to was not equal to the cluster time
written to the matroska file.
Example of "mkvinfo -v" of how it looks before and after this change in
an scenario where previously timestamps got out of order because of this
issue.
Notice the timestamp of the SimpleBlock right before and right after the
Cluster now being in order. The consequence of this however is that the
cluster timestamp is not necessarily the same as the timestamp of the
first buffer in the cluster however (in case it's rounded up).
Before
| + SimpleBlock (track number 1, 1 frame(s), timecode 126.922s = 00:02:06.922)
| + Frame with size 432
| + SimpleBlock (track number 2, 1 frame(s), timecode 126.933s = 00:02:06.933)
| + Frame with size 329
| + SimpleBlock (track number 2, 1 frame(s), timecode 126.955s = 00:02:06.955)
| + Frame with size 333
|+ Cluster
| + Cluster timecode: 126.954s
| + Cluster previous size: 97344
| + SimpleBlock (key, track number 1, 1 frame(s), timecode 126.954s = 00:02:06.954)
| + Frame with size 61239
| + SimpleBlock (track number 2, 1 frame(s), timecode 126.975s = 00:02:06.975)
| + Frame with size 338
After
| + SimpleBlock (track number 1, 1 frame(s), timecode 135.456s = 00:02:15.456)
| + Frame with size 2260
| + SimpleBlock (track number 2, 1 frame(s), timecode 135.468s = 00:02:15.468)
| + Frame with size 332
| + SimpleBlock (track number 2, 1 frame(s), timecode 135.490s = 00:02:15.490)
| + Frame with size 335
|+ Cluster
| + Cluster timecode: 135.489s
| + Cluster previous size: 158758
| + SimpleBlock (key, track number 1, 1 frame(s), timecode 135.490s = 00:02:15.490)
| + Frame with size 88070
| + SimpleBlock (track number 2, 1 frame(s), timecode 135.511s = 00:02:15.511)
| + Frame with size 336
Comparing gst_rtspsrc_loop_interleaved and gst_rtspsrc_loop_udp, and investigating on timeout issues, it sounds like a piece of code has been originally copied from udp to the interleaved one. The timeout variable is never used inside the interleaved one. No side effect has been seen in the removed function calls.
The debug message removed is pointless as the timeout used is "src->tcp_timeout" that is fixed.
The presence of the two timeout drove my team in investigating if the reference to the tcp_timeout was correct (it is). Hence we removed the misleading reference to the local timeout variable.
The VP Codec Configuration Box (vpcC) contains vp9 profile and
colorimetry information. Especially the profile information might
be useful for downstream to select capable decoder element.
For live streams, if we keep the stream for a long time, the timestamp
will be larger than max_uint32. In that case, timestamp should be handled
as a rollover timestamp rather than a backward timestamp.
* Organize GstRtpFunnelPad and GstRtpFunnel separately
* Use G_GNUC_UNUSED instead of (void) casts
* Don't call an event "caps"
* Use semicolons after GST_END_TEST (helps gst-indent)
Instead of having chunks with one sample per raw audio sample, have
chunks with a single sample that contains lots of raw audio samples. If
necessary these are still split again later when reading the stream.
With this we are allocating a lot less memory for the parsed sample
tables and can play files that previously triggered our limit of 200MB
for the sample table. For example, one file here would previously
allocate 3.5GB for the sample table and now only allocates 70KB.
Outputting 48000 buffers per second is not a good idea performance-wise.
If a container sample is less than 1024 raw audio frames, combine
multiple samples to get at least 1024 raw audio samples as long as
they're stored contiguous in the file.
For the other direction, if a container sample contains more than 4096
samples there is already code for splitting them up.
Fixes https://bugzilla.gnome.org/show_bug.cgi?id=692750
When the server replies with a range "now-", it is presumed to
be a "live" stream and we should request a similar range.
This was the case prior to my refactoring to make use of
gst_rtsp_range_to_string in 5f1a732bc7,
this commit restores the behaviour for that case.
The proper way of capping on max-streams is to do it in rtpssrcdemux.
This patch uses the newly introduced property on rtpssrcdemux. Previous
behavior would not prevent rtpssrcdemux spawning new pads for every new
ssrc and potentialy causing performance trouble during teardown.
When used for processing bundled media streams within rtpbin the rtpssrcdemux element may
receive bad RTP and RTCP packets, these should not be treated as a fatal error.
The property is useful against atacks when the sender changes SSRC for
every RTP packet. The property with the same name introduced in rtpbin
was not enough, because we still can end up with thousands of pads
allocated in rtpssrcdemux.
gstrtspsrc uses a queue, set_get_param_q, to store set param and get
param requests. The requests are put on the queue by calling
get_parameters() and set_parameter(). A thread which executs in
gst_rtspsrc_thread() then pops requests from the queue and processes
them. The crash occured because the queue became empty and a NULL
request object was then used. The reason that the queue became empty
is that it was popped even when the thread was NOT processing a get
parameter or set parameter command. The fix is to make sure that the
queue is ONLY popped when the command being processed is a set
parameter or get parameter command.
If not configuring the sinks via the "location" property this can be
useful to know for which sink the fragment was actually opened/closed,
especially if finalization of the fragments is happening asynchronously.
When connected to an upstream rtpfunnel element, payload-type,
ssrc and clock-rate will not be present in the received caps.
rtprtxsend can already deal with only the clock rate being
present there, a new property is exposed to allow users to
provide a payload-type -> clock-rate map, this enables the
use of the max-size-time property for bundled streams.
ffmpeg is doing the same and various files in the wild have bogus
information in the sample description if the same information is also
duplicated afterwards in the v1/v2 sound sample desription.
Previously we only did this for non-raw audio due to
https://bugzilla.gnome.org/show_bug.cgi?id=374914
but this specific file is already worked around differently. It still
works after this change.
Also remove ad-hoc GST_READ_DOUBLE_BE re-implementation and move the
switch for legacy audio formats after reading all the sample
descriptions as we want to override the values from there.
By default imagefreeze will still reject new buffers after the first one
and immediately return GST_FLOW_EOS but the new allow-replace property
allows to change this.
Whenever updating the buffer we now also keep track of the configured
caps of the buffer and from the source pad task negotiate correctly
based on the potentially updated caps.
Only the very first time negotiation of a framerate with downstream is
performed, afterwards only the caps themselves apart from the framerate
are updated.
Elements emitting frames through several srcpads should use a
flow combiner to aggregate the chain returns and therefore only return
GST_FLOW_NOT_LINKED to upstream when all the downstream pads have
received GST_FLOW_NOT_LINKED.
In addition to that, in order to handle pads being relinked downstream,
the flow combiner should be reset in response to RECONFIGURE events.
This ensures that a both srcpads process a chain operation before a
GST_FLOW_NOT_LINKED can be propagated upstream (which would usually stop
the pipeline).
Otherwise, in a configuration with two srcpads, only one linked at a
time, after the relink the element could chain data through the now
unlinked pad and the flow combiner would resolve as GST_FLOW_NOT_LINKED
(stopping the pipeline) just because the now linked pad has not been
chained yet to update the flow combiner.
This patch adds handling of RECONFIGURE events to qtdemux. Also, since
this event handling causes the flow combiner to be used from a thread
other than the qtdemux streaming thread, usages of the flow combiner
has been guarded by the object lock.
The key is to make sure the jitterbuffer is set to NULL *before* the
ptdemux.
The race that existed would basically happen when ptdemux had reached
READY, and the jitterbuffer would then push a buffer, triggering a new
pad with a new payloadtype being added and ghosted to the rtpbin itself.
However, the srcpad of the ptdemux would now be inactive, and all the
sticky-event pushed on it would be swallowed, not allowing any to reach
the ghost-pad. Then the buffer in-flight would come to the ghostpad,
and we would assert that a buffer arrived before the necessary
events.
By simply re-ordering the state-changes, we ensure that there will be
no buffer racing into the ptdemux while its state is being changed,
and the problem disappears completely.
Notice also that there is not point in disconnecting the signals on the
ptdemux before this point, since we need the push-thread to settle
down before we can do this in a non-racy way.
Applications might handle locations and generally configuration of the
sink by themselves instead of having splitmuxsink set the location on
the sink. Nonetheless it makes sense to increment the fragment_id that
is passed to the signal so that applications know which fragment is
requested.
Add parsed=true to output caps, as we always output
whole frames, timestamped and all. Means also that
the output can be decoded by avdec_mjpeg wihout
plugging an extra parser (which has no rank).
Add a property that makes it possible for an application to set the
DateUTC header field in matroska files. This is useful for live feeds,
where the DateUTC header can be set to a UTC timestamp, matching the
beginning of the file.
Needs gstreamer!323
Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/481
An application might try to access splitmuxsink from sync message handler
by g_object_{get,set} which takes lock also. In general, we don't
take lock around message handler.
By passing `NULL` to `g_signal_new` instead of a marshaller, GLib will
actually internally optimize the signal (if the marshaller is available
in GLib itself) by also setting the valist marshaller. This makes the
signal emission a bit more performant than the regular marshalling,
which still needs to box into `GValue` and call libffi in case of a
generic marshaller.
Note that for custom marshallers, one would use
`g_signal_set_va_marshaller()` with the valist marshaller instead.
The do_expected_timeout() function may release the JBUF_LOCK, so we need
to check if nothing wanted the timer thread to exit after this call.
The side effect was that we may endup going back into waiting for a timer
which will cause arbitrary delay on tear down (or deadlock when test
clock is used).
Fixes#653
JBUF_WAIT_QUEUE drops the JBUF_LOCK, which means the stop condition
for the chain function may have changed (change_state to NULL). Check
this immediately after the wait so that we don't delay shutting down.
When in-place, running an allocation is not useful since videocrop
is not implicated in the allocation. So only force the allocation
query for the case it was in passthrough. This is needed since the
change in the crop region will likely pull us out of this mode. For the
case we where neither in passthrough or in-place, the allocation query
is already ran by the baseclass, so nothing special is needed.
This fixes performance issues when changing the crop region per frame.
This was reproduced using videocrop2-test.
One of the jitterbuffers functions is to try and make sense of weird
network behavior.
It is quite unhelpful for the jitterbuffer to start dropping packets
itself when what you are trying to achieve is better network resilience.
In the case of a skew, this could often mean the sender has restarted
in some fashion, and then dropping the very first buffer of this "new"
stream could often mean missing valuable information, like in the case
of video and I-frames.
This patch simply reverts back to the old behavior, prior to 8d955fc32b
and includes the simplest test I could write to demonstrate the behavior,
where a single packet arrives "perfectly", then a 50ms gap happens,
and then two more packets arrive in perfect order after that.
# Conflicts:
# tests/check/elements/rtpjitterbuffer.c
The memory leak occurs in the case when the buffer has been
added to the fragment_buffers array of the current pad and
never been sent because of the push failure of the previous
buffers: moof or mdat header or fragmented buffer(s).
This commit adds custom element messages for when gstrtpjitterbuffer
drops an incoming rtp packets due to for example arriving too late.
Applications can listen to these messages on the bus which enables
actions to be taken when packets are dropped due to for example high
network jitter.
Two properties has been added, one to enable posting drop messages and
one to set a minimum time between each message to enable throttling the
posting of messages as high drop rates.
There are in the wild (mp4) streams that basically contain no tracks
but do have a redirect info[0], in which case, we won't be able
to expose any pad (there are no tracks) so we can't post anything but
an error on the bus, as:
- it can't send EOS downstream, it has no pad,
- posting an EOS message will be useless as PAUSED state can't be
reached and there is no sink in the pipeline meaning GstBin will
simply ignore it
The approach here is to to add details to the ERROR message with a
`redirect-location` field which elements like playbin handle and use right
away.
[0]: http://movietrailers.apple.com/movies/paramount/terminator-dark-fate/terminator-dark-fate-trailer-2_480p.mov
When the queue is full (and adding more packets would risk a seqnum
roll-over), the best approach is to just start pushing out packets
from the other side. Just pushing out the packets results in the
timers being left hanging with old seqnums, so it's safer to just
execute them immediately in this case. It does limit the timer space
to the time it takes to receiver about 32k packets, but without
extended sequence number, this is the best RTP can do.
This also results in the test no longer needed to have timeouts or
timers as pushing packets in drives everything.
Fixes#619
This basically add ability to choose between inserting from head, tail
or in-place in order to try and minimize the distance to walk through in
the timer queue. This removes an overhead we had seen on high drop rate.
The timer passed to update_timers may be from the stats timer. At the
moment, we could endup rescheduling (reusing) that timer onto the normal
timer queue, unschedul it as if it was from the normal timer queue or
duplicate it into the stats timer queue again. This was protected before
as the with the fact the stats timer didn't have a valid idx.
As the offset is already applied now, we need to update and reschedule
all timers each time the offset is changed. I'm not sure who expect this
to be retro-actively applied, but there was a unit test for it.
If the jitterbuffer head change, there is no need to systematically
wakeup the timer thread. The timer thread will be waken up on if
an earlier timeout has been pushed. This prevent some more spurious
wakeup when the system is loaded. As a side effect, cranking the clock
may set the clock at an earlier position.
In this patch we now make use of the new RtpTimerQueue instead of the
old GArray. This required a lot of changes all over the place, some of
the important changes are that `timer->timeout` is no longer a PTS but
the actual timeout. This was required to get the RtpTimerQueue sorting
right. The applied offset is saved as `timer->offset`, this allow
retreiving back the PTS when needed.
The clockid updates only happens once per incoming packet. If the
currently schedule timer is before the earliest timer in the queue, we
no longer wakeup the thread. This way, if other timers get setup in the
meantime, this will reduce the number of wakup.
The timer loop code has been mostly rewritten, though the behaviour of
running the lost timers first has been kept (even though there is no
test to show what would be the side effect of doing this differently).
Fixes#608
Implement a single timer queue for all timers. The goal is to always use
ordered queues for storing timers. This way, extracting timers for
execution becomes O(1). This also allow separating the clock wait
scheduling from the timer itself and ensure that we only wake up the
timer thread when strictly needed.
The knew data structure is still O(n) on insertions and reschedule,
but we now use proximity optimization so that normal cases should be
really fast. The GList structure is also embeded intot he RtpTimer
structure to reduce the number of allocations.
This moves the RtpJitterBufferStructure type, alloc, free into
rtpjitterbuffer.c/h implementation. jitterbuffer.c strictly rely on
the fact this structure is compatible with GList, and so it make more
sense to keep encapsulate it. Also, anything that could possibly
reduce the amount of code in the element is a win.
In order to support that move, a function pointer to free the data
was added. This also allow making the free function option when
flushing the jitterbuffer.
This helps understanding which function modify the Timerdata
and which one does not. This is not always obvious from thelper
name considering recalculate_timer() does not.
In file included from ../../../../dist/linux_x86_64/include/gstreamer-1.0/gst/gst.h:55,
from ../../../../dist/linux_x86_64/include/gstreamer-1.0/gst/tag/tag.h:25,
from ../gst/isomp4/qtdemux.c:56:
In function ‘qtdemux_inspect_transformation_matrix’,
inlined from ‘qtdemux_parse_trak’ at ../gst/isomp4/qtdemux.c:10676:5,
inlined from ‘qtdemux_parse_tree’ at ../gst/isomp4/qtdemux.c:14210:5:
../../../../dist/linux_x86_64/include/gstreamer-1.0/gst/gstinfo.h:645:5: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
645 | gst_debug_log ((cat), (level), __FILE__, GST_FUNCTION, __LINE__, \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
646 | (GObject *) (object), __VA_ARGS__); \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../../dist/linux_x86_64/include/gstreamer-1.0/gst/gstinfo.h:1062:35: note: in expansion of macro ‘GST_CAT_LEVEL_LOG’
1062 | #define GST_DEBUG_OBJECT(obj,...) GST_CAT_LEVEL_LOG (GST_CAT_DEFAULT, GST_LEVEL_DEBUG, obj, __VA_ARGS__)
| ^~~~~~~~~~~~~~~~~
../gst/isomp4/qtdemux.c:10294:5: note: in expansion of macro ‘GST_DEBUG_OBJECT’
10294 | GST_DEBUG_OBJECT (qtdemux, "Transformation matrix rotation %s",
| ^~~~~~~~~~~~~~~~
../gst/isomp4/qtdemux.c: In function ‘qtdemux_parse_tree’:
../gst/isomp4/qtdemux.c:10294:64: note: format string is defined here
10294 | GST_DEBUG_OBJECT (qtdemux, "Transformation matrix rotation %s",
| ^~
Add a property which explicitly maps splitmuxsink pads to the
muxer pads they should connect to, overriding the implicit logic
that tries to match pads but yields arbitrary names.
When running in async-finalize mode, request new pads from the muxer
using the same names as old pads, instead of letting the muxer assign
new ones based on the pad template name.
The output segment is only used in ONVIF mode.
The previous behaviour was to output a segment computed from
the Range response sent by the server.
In ONVIF mode, servers will start serving from the appropriate
synchronization point (keyframe), and the Range in response will
start at that position.
This means rtspsrc can now perform truly accurate seeks in that
mode, by clipping the output segment to the values requested in
the seek. The decoder will then discard out of segment buffers
and playback will start without artefacts at the exact requested
position, similar to the behaviour of a demuxer when an accurate
seek is requested.
In push mode (streaming), if the audio size is smaller than segment buffer size, it would be ignored.
This happens because when the plugin receives an EOS signal while a single audio chunk that is less than the segment buffer size is buffered, it does not
flush this chunk. The fix is to flush the data chunk when it receives an EOS signal and has a single (first) chunk buffered.
How to reproduce:
1. Run gst-launch with tcp source
```
gst-launch-1.0 tcpserversrc port=3000 ! wavparse ignore-length=0 ! audioconvert ! filesink location=bug.wav
```
2. Send a wav file with unspecified data chunk length (0). Attached a test file
```
cat test.wav | nc localhost 3000
```
3. Compare the length of the source file and output file
```
ls -l test.wav bug.wav
-rw-rw-r-- 1 amr amr 0 Aug 15 11:07 bug.wav
-rwxrwxr-x 1 amr amr 3564 Aug 15 11:06 test.wav
```
The expected length of the result of the gst-lauch pipeline should be the same as the test file minus the headers (44), which is ```3564 - 44 = 3520``` but the actual output length is ```0```
After the fix:
```
ls -l test.wav fix.wav
-rw-rw-r-- 1 amr amr 3520 Aug 15 11:09 fix.wav
-rwxrwxr-x 1 amr amr 3564 Aug 15 11:06 test.wav
```
If VP8 is not encoded with error resilience enabled then any packet loss
causes very bad artefacts when decoding and waiting for the next
keyframe instead improves user experience considerably.
Various audio formats require an audio lead-in to decode it properly.
Most parsers would take care of it, but when a container like matroska is
involved, the demuxer handles the seeking and without its own lead-in
handling would never even pass the lead-in data to the parser.
This commit provides an initial implementation of that for audio/mpeg,
audio/x-ac3 and audio/x-eac3 by calculating the worst case lead-in time
needed from known samplerate, potential lead-in frames need and the
maximum blocksize possible for the format (as we don't parse that out
exactly in matroskademux) and seeking that much earlier in case of
accurate seeks. This is especially important for NLE use-cases with GES.
If accurate seeking to a position that happens to have a video keyframe,
it'll go back to the previous keyframe than needed, but with typical
video files that's the best we can do anyway without falling back to
scanning the clusters, as typically only keyframes are indexed in
Cueing Data.
If the media doesn't have a CUE, then we bisect for the cluster to seek
to with the same modified time as well in case of accurate seeking,
ensuring sufficient lead-in. This code path is typically hit only with
(suboptimal) audio-only matroska files, e.g. when created with ffmpeg,
which doesn't add a CUE for audio-only mkv muxing.
The send path in rtpsession processes the buffer list along the way,
sharing info and stats between packets in the same list, because it
assumes that all packets in a buffer list are from the same frame.
However, in the receiving path packets can arrive in all sorts of
arrangements:
- different sources,
- different frames (different timestamps),
- different types (multiplexed RTP and RTCP, invalid RTP packets).
so a more general approach should be used to correctly support buffer
lists in the receive path.
It turns out that it's simpler and more robust to process buffers
individually inside the rtpsession element even if they come in a buffer
list, and then reassemble a new buffer list when pushing the buffers
downstream.
This avoids complicating the existing code to make all functions
buffer-list-aware with the risk of introducing regressions,
To support buffer lists in the receive path and reduce the "push
overhead" in the pipeline, a new private field named processed_list is
added to GstRtpSessionPrivate, it is set in the chain_list handler and
used in the process_rtp callback; this is to achieve the following:
- iterate over the incoming buffer list;
- process the packets one by one;
- add the valid ones to a new buffer list;
- push the new buffer list downstream.
The processed_list field is reset before pushing a buffer list to be on
the safe side in case a single buffer was to be pushed by upstream
at some later point.
NOTE:
The proposed modifications do not change the behavior of the send path.
The process_rtp callback is called in rtpsource.c by the push_rtp
callback (via source_push_rtp) only when the source is not internal.
So even though push_rtp is also called in the send path, it won't end up
using process_rtp in this case because the source would be internal in
the send path.
The reasoning from above may suggest a future refactoring: push_rtp
might be split to better differentiate the send and receive path.
In push mode (streaming), if the last audio payload chunk is less than the segment rate buffer size, it would be ignored since the plugin waits until it has at least segment rate bufer size of audio.
The fix is to introduce a flushing flag that indicates that no more audio will be available so that the plugin can recognize this condition and flush the data is has even if it is less
than the desired segment rate buffer size.
This is useful to support the ONVIF case: when is-live is set to
FALSE and onvif-rate-control is no, the client can control the
rate of delivery and arrange for the server to block and still
keep sending when unblocked, without requiring back and forth
PAUSE / PLAY requests. This enables, amongst other things, fast
frame stepping on the client side.
When is-live is FALSE, we don't use a manager at all. This case
was actually already pretty well handled by the current code. The
standard manager, rtpbin, is simply no longer needed in this case.
Applications can instantiate a downloadbuffer after rtspsrc if
needed.
Refactor the code for parsing and generating the Range, taking
advantage of existing API in GstRtspTimeRange.
Only use the TCP protocol in that mode, as per the specification.
Generate an accurate segment when in that mode, and signal to the
depayloader that it should not generate its own segment, through
the "onvif-mode" field in the caps, see
<https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/merge_requests/328>
for more information.
Translate trickmode seek flags to their ONVIF representation
Expose an onvif-rate-control property
Forwarding a single segment event from the pad that first gets
chained is incorrect: when that first event was sent by an element
such as x264enc, with its offset start, we end pushing out of segment
buffers for the other pad(s).
Instead, everytime the active pad changes, forward the appropriate
segment event.
Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/1028
