ULP FEC, as defined in RFC 5109, has the protected and protection
packets sharing the same ssrc, and a different payload type, and
implies rewriting the seqnums of the protected stream when encoding
the protection packets. This has the unfortunate drawback of not
being able to tell whether a lost packet was a protection packet.
rtpbasedepayload relies on gaps in the seqnums to set the DISCONT
flag on buffers it outputs. Before that commit, this created two
problems:
* The protection packets don't make it as far as the depayloader,
which means it will mark buffers as DISCONT every time the previous
packets were protected
* While we could work around the previous issue by looking at
the protection packets ignored and dropped in rtpptdemux, we
would still mark buffers as DISCONT when a FEC packet was lost,
as we cannot know that it was indeed a FEC packet, even though
this should have no impact on the decoding of the stream
With this commit, we consider that when using ULPFEC, gaps in
the seqnums are not a reliable indicator of whether buffers should
be marked as DISCONT or not, and thus rewrite the seqnums on
the decoding side as well to form a perfect sequence, this
obviously doesn't prevent the jitterbuffer from doing its job
as the ulpfec decoder is downstream from it.
https://bugzilla.gnome.org/show_bug.cgi?id=794909
Fix compilation with MSVC. We still assume that attribute
is supported by all other relevant compilers, which seems
to be the case since we haven't had any complaints about
similar code in rtpsbcpay.
rtpulpfeccommon.c:432:27: error: format ‘%lx’ expects argument of type
‘long unsigned int’, but argument 10 has type ‘guint64 {aka long long unsigned int}’
https://bugzilla.gnome.org/show_bug.cgi?id=793732
The ulpfecenc "mux-seq" and "ssrc" properties were initially added
because the element did more than implement ULPFEC. As it was
decided that FLEXFEC would be implemented in a separate element,
both properties are now unneeded and confusing.
Change the default for the ulpfecenc multi-packet property,
as it is expected that most users of this element will be protecting video
streams.
Change the default property for the rtpredenc allow-no-red-blocks
property, as it should also be its default mode of operation.
https://bugzilla.gnome.org/show_bug.cgi?id=793843
It is expected that when connecting to a stream that has
already started, the caps will only arrive at the interval
specified on rtpgstpay, we shouldn't be warning as this is
a normal mode of operation.
https://bugzilla.gnome.org/show_bug.cgi?id=793798
We expose a set of new elements:
* ULPFEC encoder / decoder
* A storage element, which should be placed before jitterbuffers,
and is used to store packets in order to attempt reconstruction
after the jitterbuffer has sent PacketLost events
* RED encoder / decoder (RFC 2198), these are necessary to
use FEC in webrtc, as browsers will propose and expect ulpfec
packets to be wrapped in red packets
With contributions from:
Mathieu Duponchelle <mathieu@centricular.com>
Sebastian Dröge <sebastian@centricular.com>
https://bugzilla.gnome.org/show_bug.cgi?id=792696
All received configurations are parsed and added to a list, this lead
to an unbounded memory usage. As the configuration is resent every
second this quickly lead to a large memory usage.
Add a check to only add the config if it is not already available in
the list. This fix only handle the typical case of a well behaved
stream, a malicious server could still send many useless
configurations to raise the client memory usage.
Only for byte-stream or hev1. For hvc1 the SPS/PPS are in the
caps as codec_data field and in this case they shouldn't be in
the stream data as well. The output caps should be updated with
the new codec_data if needed, for hvc1.
We keep the boolean byte_stream around since it's nicer for
readability and most of the code just cares about byte_stream
or not. This is useful for future-proofing the code for when
we add support for hev1 output as well.
This would happen if input is byte-stream with four-byte
sync markers instead of three-byte ones. The code that
scans for sync markers will place the start of the NALU
on the third-last byte of the NALU sync marker, which
means that any additional zeros may be counted as belonging
to the previous NALU instead of being part of the next sync
marker. Fix that so we don't send VPS/SPS/PPS with trailing
zeros in this case.
See https://bugzilla.gnome.org/show_bug.cgi?id=732758
There is no difference between pushing out a buffer directly
with gst_rtp_base_depayload_push() and returning it from the
process function. The base class will just call _depayload_push()
on the returned buffer as well.
So instead of marshalling buffers through three layers and back,
just push them from one place in handle_nal() and always return
NULL from the process vfunc. This simplifies the code a little.
Also rename _push_fragmentation_unit() to _finish_fragmentation_unit()
for clarity. Push sounds like it means being pushed out, whereas
it might just be pushed into an adapter.
This change has the side-effect that multiple NALs in a single STAP
(such as SPS/PPS) may no longer be pushed out as a single buffer if
we output NALs in byte-stream format (i.e. not aggregate AUs), but
that shouldn't really make any difference to anyone.
This would happen if input is byte-stream with four-byte
sync markers instead of three-byte ones. The code that
scans for sync markers will place the start of the NALU
on the third-last byte of the NALU sync marker, which
means that any additional zeros may be counted as belonging
to the previous NALU instead of being part of the next sync
marker. Fix that so we don't send SPS/PPS with trailing
zeros in this case.
https://bugzilla.gnome.org/show_bug.cgi?id=732758
The G722 payload only accepts G722 audio with channels=1, so it must
specify the encoding-params=1 in its src caps, otherwise it causes issues
with farstream which thinks it supports 2 channels G722 and when
confronted with a remote that has G722/8000/2, it will negotiate it
and error out with a not-negotiated when the caps don't intersect
at runtime.
https://bugzilla.gnome.org/show_bug.cgi?id=789878
This then just counts samples and calculates the output timestamps based
on that and the very first observed timestamp. The timestamps on the
buffers are continued to be used to detect discontinuities that are too
big and reset the counter at that point.
When receiving data via Bluetooth, many devices put completely wrong
values into the RTP timestamp field. For example iOS seems to put a
timestamp in milliseconds in there, instead of something based on the
current sample offset (RTP clock-rate == sample rate).
https://bugzilla.gnome.org/show_bug.cgi?id=787297
Do not allocate payload size outbuf if appending payload buffer.
The commit 137672ff18 attached payload
to the output buffer but forgot to remove payload allocation. That
effectively doubled payload size and add zero'ed or random bytes.
Makes the following pipeline work again:
gst-launch-1.0 -v audiotestsrc wave=2 ! gsmenc ! rtpgsmpay ! rtpgsmdepay ! gsmdec ! autoaudiosink
https://bugzilla.gnome.org/show_bug.cgi?id=784616
There is no difference between pushing out a buffer directly
with gst_rtp_base_depayload_push() and returning it from the
process function. The base class will just call _depayload_push()
on the returned buffer as well.
So instead of marshalling buffers through three layers and back,
just push them from one place in handle_nal() and always return
NULL from the process vfunc. This simplifies the code a little.
Also rename _push_fragmentation_unit() to _finish_fragmentation_unit()
for clarity. Push sounds like it means being pushed out, whereas
it might just be pushed into an adapter.
This change has the side-effect that multiple NALs in a single STAP
(such as SPS/PPS) may no longer be pushed out as a single buffer if
we output NALs in byte-stream format (i.e. not aggregate AUs), but
that shouldn't really make any difference to anyone.
Use the ::process_rtp_packet() vfunc to avoid mapping the
RTP buffer twice.
gst_rtp_buffer_get_payload_buffer() returns a new sub-buffer
which will always be writable, so no need to make it writable.
Every g_quark_from_static_string() is a hash table lookup serialised
on the global quark lock in GLib. Let's just look up the two quarks
we need once and cache them locally for future use. While we're at it,
add new utility functions for the two most commonly used tags
(audio + video). Make first argument a gpointer so we don't have to
cast and make the code ugly. These are used for logging purposes
only anyway.
