The lost-event was using a different time-domain (dts) than the outgoing
buffers (pts). Given certain network-conditions these two would become
sufficiently different and the lost-event contained timestamp/duration
that was really wrong. As an example GstAudioDecoder could produce
a stream that jumps back and forth in time after receiving a lost-event.
The previous behavior calculated the pts (based on the rtptime) inside the
rtp_jitter_buffer_insert function, but now this functionality has been
refactored into a new function rtp_jitter_buffer_calculate_pts that is
called much earlier in the _chain function to make pts available to
various calculations that wrongly used dts previously
(like the lost-event).
There are however two calculations where using dts is the right thing to
do: calculating the receive-jitter and the rtx-round-trip-time, where the
arrival time of the buffer from the network is the right metric
(and is what dts in fact is today).
The patch also adds two tests regarding B-frames or the
“rtptime-going-backwards”-scenario, as there were some concerns that this
patch might break this behavior (which the tests shows it does not).
The new timeout is always going to be (timeout + delay), however, the
old behavior compared the current timeout to just (timeout), basically
being (delay) off.
This would happen if rtx-delay == rtx-retry-timeout, with the result that
a second rtx attempt for any buffers would be scheduled immediately instead
of after rtx-delay ms.
Simply calculate (new_timeout = timeout + delay) and then use that instead.
https://bugzilla.gnome.org/show_bug.cgi?id=773905
It's been broken for years, and it's unlikely it will ever
be fixed for collectpads/videomixer now that there's compositor
which works fine. So let's disable it, since all it does
is that it creates noise that distracts from other failures.
Also see the corresponding adder bug as it failed in the same way:
https://bugzilla.gnome.org/show_bug.cgi?id=708891
It seems that the forked processes all attempt to handle the listening
socket from the server, and only one has to shutdown the socket to break
the server completely.
Create a new server inside each test to avoid this.
https://bugzilla.gnome.org/show_bug.cgi?id=772656
The tests accumulate buffers in GstCheck's buffers list, and the list is
not (consistently) reset between tests. Do that and remove the now
conflicting unrefs for outbuffers.
https://bugzilla.gnome.org/show_bug.cgi?id=772644
The basic idea is this:
1. For *larger* rtx-rtt, weigh a new measurement as before
2. For *smaller* rtx-rtt, be a bit more conservative and weigh a bit less
3. For very large measurements, consider them "outliers"
and count them a lot less
The idea being that reducing the rtx-rtt is much more harmful then
increasing it, since we don't want to be underestimating the rtt of the
network, and when using this number to estimate the latency you need for
you jitterbuffer, you would rather want it to be a bit larger then a bit
smaller, potentially losing rtx-packets. The "outlier-detector" is there
to prevent a single skewed measurement to affect the outcome too much.
On wireless networks, these are surprisingly common.
https://bugzilla.gnome.org/show_bug.cgi?id=769768
Assuming equidistant packet spacing when that's not true leads to more
loss than necessary in the case of reordering and jitter. Typically this
is true for video where one frame often consists of multiple packets
with the same rtp timestamp. In this case it's better to assume that the
missing packets have the same timestamp as the last received packet, so
that the scheduled lost timer does not time out too early causing the
packets to be considered lost even though they may arrive in time.
https://bugzilla.gnome.org/show_bug.cgi?id=769768
There is no need to schedule another EXPECTED timer if we're already
past the retry period. Under normal operation this won't happen, but if
there are more timers than the jitterbuffer is able to process in
real-time, scheduling more timers will just make the situation worse.
Instead, consider this packet as lost and move on. This scenario can
occur with high loss rate, low rtt and high configured latency.
https://bugzilla.gnome.org/show_bug.cgi?id=769768
This patch fixes an issue with the estimated gap duration when there is
a gap immediately after a lost timer has been processed. Previously
there was a discrepancy beteen the gap in seqnum and gap in dts which
would cause wrong calculated duration. The issue would only be seen with
retranmission enabled since when it's disabled lost timers are only
created when a packet is received and the actual gap length and last dts
is known.
https://bugzilla.gnome.org/show_bug.cgi?id=769768
Stats should also be collected for unsuccessful packets.
rtx-rtt is very important for determining the necessary configured
latency on the jitterbuffer. It's especially important to be able to
increase the latency when retransmitted packets arrive too late and are
considered lost. This patch includes these late packets in the
calculation of the various rtx stats, making them more correct and
useful.
Also in the case where the original packet arrives after a NACK is sent,
the received RTX packet should update the stats since it provides useful
information about RTT.
The RTT is only updated if and only if all requested retranmissions are
received. That way the RTT is guaranteed to make sense. If not we don't
know which request the packet is a response to and the RTT may be bogus.
A consequence of this patch is that RTT is not updated for a request
when one of the RTX packets for that seqnum is lost, but that since
measured RTT will be more accurate.
The implementation store the RTX information from the timed out timers
and use this when the retransmitted packet arrives. For performance
these timers are stored separately from the "normal" timers in order to
not impact performance (see attached performance test).
https://bugzilla.gnome.org/show_bug.cgi?id=769768
Under certain conditions gst_rtp_buffer_get_payload() returns a copy of
the payload. In this case the payload modifications will not affect the
rtp buffer. So instead of modifying the payload buffer directly we
should modify the buffer that actually gets pushed on the adapter.
The functionality of all the tests was kept exactly the same. Some tests
were renamed:
test_push_forward_seq -> test_rtxsend_rtxreceive
test_drop_one_sender -> test_rtxsend_rtxreceive_with_packet_loss
test_drop_multiple_sender -> test_multi_rtxsend_rtxreceive_with_packet_loss
test_rtxreceive_data_reconstruction was testing that retransmitted
buffer produced by rtxsend was correctly transformed to the original
buffer by rtxreceive. Now we are checking for this in all the tests
where both rtxsend & rtxreceive are involved. That's why the test was
removed.
Need to set max-misorder-time and max-dropout-time to 0 so the
jitterbuffer does not base them on packet rate calculations.
If it does, out gap is big enough to be considered a new stream and
we wait for a few consecutive packets just to be sure
https://bugzilla.gnome.org/show_bug.cgi?id=751311
When parsing NAL unit type in codec_data, check the 6bits of
NAL_unit_type only and do not require the array_completeness bit to be
0, since the default and mandatory value of array_completeness is 1 for
hvc1.
https://bugzilla.gnome.org/show_bug.cgi?id=768653
Handle sprop-vps, sprop-sps and sprop-pps in caps instead of
sprop-parameter-sets.
rtph265pay works with byte-stream and hvc1 formats but not hev1 yet. It
handles profile-id, tier-flag and level-id in caps query.
https://bugzilla.gnome.org/show_bug.cgi?id=753760
This is to handle cases where upstream handles the fragmented streaming in TIME
segments and sends us data with gaps within fragments. This would happen when dealing
with trick-modes.
When upstream (push-based, TIME SEGMENT) wishes to send discontinuous samples,
it must obey the following rules:
* The buffer containing the [moof] must have a valid GST_BUFFER_OFFSET
* The buffers containing the first sample after a gap:
* MUST start at the beginning of a sample,
* MUST have the DISCONT flag set,
* MUST have a valid GST_BUFFER_OFFSET relative to the beginning of the fragment.
https://bugzilla.gnome.org/show_bug.cgi?id=767354
Some endpoints (like Tandberg E20) can send BYE packet containing our
internal SSRC. I this case we would detect SSRC collision and get rid
of the source at some point. But because we are still sending packets
with that SSRC the source will be recreated immediately.
This brand new internal source will not have some variables incorrectly
set in its state. For example 'seqnum-base` and `clock-rate` values will be
-1.
The fix is not to act on BYE RTCP if it contains internal or unknown
SSRC.
https://bugzilla.gnome.org/show_bug.cgi?id=762219
Keeping the lock while emitting the stats signal introduces potential
deadlock in those situations when the signal callback wants the access
to rtpsession's properties which also requre the lock.
https://bugzilla.gnome.org/show_bug.cgi?id=762216
Avoid using soup_server_run_async and old get_port() APIs,
replace with me soup_server_listen and get the port through the
URIs list returned from the server.
When a packet arrives that has already been considered lost as part of a
large gap the "lost timer" for this will be cancelled. If the remaining
packets of this large gap never arrives, there will be missing entries
in the queue and the loop function will keep waiting for these packets
to arrive and never push another packet, effectively stalling the
pipeline.
The proposed fix conciders parts of a large gap definitely lost (since
they are calculated from latency) and ignores the late arrivals.
In practice the issue is rare since large gaps are scheduled immediately,
and for the stall to happen the late arrival needs to be processed
before this times out.
https://bugzilla.gnome.org/show_bug.cgi?id=765933
In other words, gst_pad_get_current_caps should never return NULL
in a pad-added callback from the demuxer.
Added tests for the two special cases with AAC and H.264 where this
would happen every time.
https://bugzilla.gnome.org/show_bug.cgi?id=763780
Making the event itself writable is not enough, it won't make
the actual taglist in the event writable as well. Instead, just
make a copy of the taglist and then create a new tag event from
that if required, replacing the old one. Before we would
inadvertently modify taglists upstream elements might still
be holding on to. Add unit test for this as well.
https://bugzilla.gnome.org/show_bug.cgi?id=762793
Use fail_unless and friends instead of g_assert
Factor seq-num checking out to separate function
Check more return-values from push and crank and others
https://bugzilla.gnome.org/show_bug.cgi?id=762254
This way we can use -1 as special value, which is nicer than MAXUINT.
This is backwards compatible even with the GValue API, as shown by
a unit test.
https://bugzilla.gnome.org/show_bug.cgi?id=757892
By not doing this, the muxer is not effectively a rtpmuxer, rather a
funnel, since it should be a single stream that exists the muxer.
If not specified, take the first ssrc seen on a sinkpad, allowing upstream
to decide ssrc in "passthrough" with only one sinkpad.
Also, let downstream ssrc overrule internal configured one
We hence has the following order for determining the ssrc used by
rtpmux:
0. Suggestion from GstRTPCollision event
1. Downstream caps
2. ssrc-Property
3. (First) upstream caps containing ssrc
4. Randomly generated
https://bugzilla.gnome.org/show_bug.cgi?id=752694
Avoid using default accept-caps handler that will query downstream
and is more expensive. Just check if the caps is compatible with
the template and check if the channels are the same.
The time of the first RTCP packet is semi-random, so
sometimes it was produced before enough packets from
the second SSRC were received. First drop queued RTCP
packets, then advance the clock enough to ensure
that at least one new RTCP packet is produced.
https://bugzilla.gnome.org/show_bug.cgi?id=750731
The accept-caps query just does a shallow check at the current
element while at this test we want it to also look at downstream.
So use caps query there.
https://bugzilla.gnome.org/show_bug.cgi?id=753623
1) Tests that using dynamic PT instead of the default ones work
2) If we ever decide to change the codec here we don't need to
worry about change the PT for the default one of the new codec
in the test
https://bugzilla.gnome.org/show_bug.cgi?id=746445
The RTP PT for alaw is 8.
Less than 50 packets are received in the length of this test so it
would never drop a buffer or would drop only the last buffer and
it would fail sometimes when the received wouldn't receive the
retransmission packet in time.
https://bugzilla.gnome.org/show_bug.cgi?id=746445
Some of the subtitle chunks will have embedded
NUL-terminators (last three), some don't (first three),
some will have markup, some won't, some will be valid
UTF-8 (all but last), some won't (last stanza).
https://bugzilla.gnome.org/show_bug.cgi?id=752421
Replace static constants with macros to make gcc happy
CC elements/elements_rtpjitterbuffer-rtpjitterbuffer.o
elements/rtpjitterbuffer.c:387:1: error: initializer element is not constant
static const GstClockTime PCMU_BUF_DURATION = PCMU_BUF_MS * GST_MSECOND;
^
elements/rtpjitterbuffer.c:388:1: error: initializer element is not constant
static const guint PCMU_BUF_SIZE = 64000 * PCMU_BUF_MS / 1000;
^
elements/rtpjitterbuffer.c:390:5: error: initializer element is not constant
PCMU_BUF_CLOCK_RATE * PCMU_BUF_MS / 1000;
The amount of time that is completely expired and not worth waiting for,
is the duration of the packets in the gap (gap * duration) - the
latency (size) of the jitterbuffer (priv->latency_ns). This is the duration
that we make a "multi-lost" packet for.
The "late" concept made some sense in 0.10 as it reflected that a buffer
coming in had not been waited for at all, but had a timestamp that was
outside the jitterbuffer to wait for. With the rewrite of the waiting
(timeout) mechanism in 1.0, this no longer makes any sense, and the
variable no longer reflects anything meaningful (num > 0 is useless,
the duration is what matters)
Fixed up the tests that had been slightly modified in 1.0 to allow faulty
behavior to sneak in, and port some of them to use GstHarness.
https://bugzilla.gnome.org/show_bug.cgi?id=738363
Also make it so that the mtu is always set if specified, not
only in case of the rather weird bufferlist test code path.
This allows us to easily make the payloader fragment a payload
across multiple output packets by setting a small MTU on it.
Implementation according to RFC 4587.
Payloader create fragments on MB boundaries in order to match MTU size
the best it can. Some decoders/depayloaders in the wild are very strict
about receiving a continuous bit-stream (e.g. no no-op bits between
frames), so the payloader will shift the compressed bit-stream of a
frame to align with the last significant bit of the previous frame.
Depayloader does not try to be fancy in case of packet loss. It simply
drops all packets for a frame if there is a loss, keeping it simple.
https://bugzilla.gnome.org/show_bug.cgi?id=751886
If we have a clock, update "now" now with the very latest running time we have.
If timers are unscheduled below we otherwise wouldn't update now (it's only updated
when timers expire), and also for the very first loop iteration now would otherwise
always be 0.
Also the time is used for the timeout functions, e.g. to calculate any times
for the next timeouts and we would otherwise pass too old times there.
https://bugzilla.gnome.org/show_bug.cgi?id=751636
The mp4 muxer now writes a place-holder mdat as a free
atom followed by a 0-byte mdat that covers the rest of the
file, making it possible to rewrite it as 64-bit, or leave
it as-is if nothing else is written afterward
The calculations were a bit off everywhere, even before the changes done
recently to the delay for RTX of expected future packets. It only worked by
accident, but now the calculations are all correct again. Hopefully.
