Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TWCC accounts RTX recovery_pct in more general way #14

Open
wants to merge 359 commits into
base: main
Choose a base branch
from

Conversation

MishaBaranov
Copy link
Contributor

This is an intermediate PR introducing:

  • Repair Meta information (definitely not a perfect name) for Buffers with redundant packet -- they contain an array of seqnums which are covered by this (and might be not only this) packet
  • Reception Stats module -- keeps track of sent and actually delivered data packets and redundant packets, concluding if a certain was delivered, recovered or lost. Engaged for RTX only in this commit

w-miller and others added 30 commits September 27, 2023 17:46
This is created when using the gst-gdb Python helper script.
Some python-bindings are doing this twice, so we need to be resilient.
We also leak during removal of a function
A very common pipeline is feeding an encoder inheriting from
GstAudioEncoder into a payloader inheriting from GstRtpBasePayload.

If this is the case, the rtp-timestamp will, if perfect_rtptime is TRUE,
reflect the amount of bytes in each packet, and not (like it should)
reflect the timestamp of the packets in the clock-rate domain.

Example:
20ms AAC packets with clock-rate of 90000, should have the rtp-timestamp
incrementing with 90000 * 20 / 1000 = 1800.

Currently, because perfect_rtptime is default TRUE, the AAC-packets
will be incrementing with the buffer-size of the packets, which is
typically 64000 * 20 / 1000 = 1280.

In the case that bitrate and clock-rate are the same (like with ALAW and
MULAW) it will work by chance, which is probably why it has not been
discovered earlier.

https://bugzilla.gnome.org/show_bug.cgi?id=761943
When doing DTX generation in decoders, they are indeed
producing buffers without having a corresponding input
buffer, so this code needs to be revisited, and perhaps
need a special method for pushing buffers like this?

https://bugzilla.gnome.org/show_bug.cgi?id=773106
…eo alignment requirements" [pexhack]

This reverts commit 8b96b52.
Depayloaders should be less strict about spec conformance on incoming
packets.
With this patch
pexip/mcu@f819d92
@johnbassett discovered that old frames would be produced if stopping and
starting a layer. However, setting drop-only always to true means we can
never increase the framerate making the caps negotiation fail.
This hack sets drop-only to true when the input rate is higher or equal
to the output one.

A proper fix would be to identify the bug @johnbassett discovered and do
a test and fix for that in the videorate element.
…f frame"

This is because we set this flag ourselves in the implementations.

This reverts commit 7013a58.
A way to embed ROI meta in the RTP extension header.
...and deal with non-modulo 4 padding bytes for non-standard RTCP.

This is thanks to Chrome sending these packets to us when doing TWCC.

Whereas the standard says that non-modulo 4 bytes padding are not allowed,
for reduced size and TWCC in particular, this does not seem to apply.

In order to be able to safely parse FCIs from these packets, a new API
is proposed: gst_rtcp_packet_fb_get_fci_length_bytes, that will
respect the non-mod4 padding and subtract that from the valid FCI length.

Co-Authored-By: Tulio Beloqui <[email protected]>
Co-Authored-By: John-Mark Bell <[email protected]>
We don't always want the PTS of the frame to be updated.
The default value is kept TRUE to comply with the previous behaviour.

Co-Authored-By: Håvard Graff <[email protected]>
Basically, don't set a DISCONT flag when detecting a seqnum jump.
This is useful for ULPFEC, as a FEC packet might be hiding inside the
original stream, and this stream is actually completely valid, even if
the FEC packet is removed.
Don't send a segment event if we haven't produced data and set the
output caps yet. Doing so causes a g_warning about sticky event
misordering.

https://bugzilla.gnome.org/show_bug.cgi?id=773510
Allows it to be configured differently for different use-cases,
specifically the live scenario would probably never want this enabled.

https://bugzilla.gnome.org/show_bug.cgi?id=773511
This is a Pexip specific change since we are only interested in using
gstflvdemux live.
This happens all the time for us, so no point warning about it.
bitflows and others added 25 commits December 11, 2023 08:41
This allows run_main_with_nsapp() to actually join the thread and use the
return code from gst_thread_func.
With terminate(), that code will completely be skipped.

Co-Authored-By: Knut Saastad <[email protected]>
Added meson build file to build dcsctp, which includes its minimal
dependencies of abseil-cpp and rtc_base module.

A thin C layer (sctpsocket) on top of dcsctp has been added to interop with the
library.

Removed usrsctp in favor of dcsctp.

dcsctp version:
https://webrtc.googlesource.com/src/+/61c5e86dca59b0ac8240444e7df5c31da27ee40f

sctpenc/sctpdec:
The sctpenc and sctpdec remains almost intact, only operations over the
sctp-association has been changed, as now they are not allowed to
ref/unref the association.
The elements should not have any direct impact on the associations lifetime.

sctp-factory:
Introduced a sctp-factory which manages the lifetime of the associations
and also manages the main loop and context in which all associations
opertions will live.
All the associations are handled in a single main loop now.

sctp-association:
The sctp-association has been restructured to use the new dcsctp socket
API.
The internal mutex of the sctp-association has been changed to be a
recursive mutex. In dcsctp, the association is the one that provides the
timers to the socket, which eventually can call back into the
association itself.
gst_wasapi_device_is_in_list()

Fixes default device info not always changing.

Co-Authored-By: Knut Saastad <[email protected]>
This fixes interop (invalid checksum issues) with webRTC.

Summary of changes:

- Ported the cmake project to meson.
- Removed the crc32 implementation and use the crc32c one.
- Introduced crc32c from chromium project:
https://chromium.googlesource.com/external/github.com/google/crc32c
Updated from commit: 21fc8ef30415a635e7351ffa0e5d5367943d4a94

Co-Authored-By: Knut Saastad <[email protected]>
The problem is that once you unlock, the other thread can "sneak in",
and push its buffer before the one you are currently processing, which
creates reordered RTP sequencenumbers!

By holding the stream lock through the push (in the same way the funnel
does it), you remove this problem.
subprojects/gst-plugins-good/gst/isomp4/atoms.c[5793,5] in
build_vpcC_extension: Memory error: Potential leak of memory pointed to
by 'bw.parent.data'
subprojects/gst-plugins-good/ext/libpng/gstpngenc.c[380,5] in
gst_pngenc_handle_frame: Memory error: Potential leak of memory pointed
to by 'row_pointers'
subprojects/gst-plugins-base/gst-libs/gst/allocators/gstfdmemory.c[74,3]
in gst_fd_mem_free: Memory error: Use of memory after it is freed.
Relying on the device-index only to select a camera doesn't work properly.
The reason is that the order of devices returned from
[AVCaptureDevice devicesWithMediaType:mediaType]; is not kept across runs.
So by the time we use an index to do a selection under openDeviceInput(),
that index might be pointing to a different device in the list.

Using the uniqueID solves this problem, but I have kept the device-index
around as it is also used to select a NSScreen for desktop capture.
avfdeviceprovider:
 - introduced start/stop for monitoring devices
 - removed device-index from GstAVDevice, is not used anymore

avfvideosrc: do not use deviceIndex for camera selection, prefer deviceUniqueID
To get cameras order sorted like from front to back
Possibly PexHack, need some more investigation on why not
any custom downstream seem to make it through the encoders.
Since we started using kAudioUnitSubType_VoiceProcessingIO in CoreAudio
we don't want that audio instance to end up as a "device" in the enumeration

Moved to use GST_DEVICE_CAST() rather than GST_DEVICE() since the latest
does more checks that we do not need
…ange_cb()

We need to hold a lock when gst_osx_audio_device_change_cb() is called
from CoreAudio, this fixes a data race when disconnecting devices.

Also added lookup for the given device by "class" as device ID can change
and can be shared.
…te()

This allows the element to acquire (lock) the device when we go to READY state,
so it is going to be ready when start() is called.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.