Skip to content

Conversation

@MishaBaranov
Copy link
Contributor

Cherry pick of:

  1. (twcc: Handle wrapping of reference time )[https://gitlab.freedesktop.org/gstreamer/gstreamer/-/commit/c830f87a32355c915433af2e8721ca35cc18ab82]
    2.. twcc: Fix reference timestamp wrapping (again)]
    (c) Johan Sternerup

havardgraff and others added 30 commits October 10, 2024 21:23
Basically, all frames are "i-frames" here.
Can't have a mix of GstValueArray and GValueArray here.
Changes lost in monorepo merge; originally from
7bbcd29
Although 60 seconds is often enough, we have seen cases where
the harnessed pipeline could taken longer than that to process a buffer.

This is especially true when we run under additional tools like Valgrind
or Debug builds.
For "limiting" this means using the token bucket algorithm, allowing
a sender to control the bitrate being consumed by rtx-packets,
in case of an "out-of-control" receiver asking for way too much packets.

'max-kbps' sets the maximum kilobit per second that the RTX element will
give out based on requests, limiting the ability for a receiver to
congest the available network bandwidth.

'max-bucket-size' is related to the token bucket algorithm, and will
allow control of the "burstiness" of the RTX requests, preventing a huge
spike in the network.

For "stuffing" this means adding RTX packets as stuffing to
meet the total target bitrate 'stuffing-kbps'. There are cases when
output rate sent on the network needs to be near constant, but the
media isn't (Encoders producing variable bitrates)

Also, for probing the network for available bitrate, the stuffing
can be used to create redundant information at a certain bitrate.

Co-authored-by: Tulio Beloqui <tulio@pexip.com>
Co-authored-by: Stian Selnes <stian@pexip.com>
Squashed commit of the following:

commit c2e3ed1
Author: Camilo Celis Guzman <camilo@pexip.com>
Date:   Tue Nov 2 22:30:12 2021 +0900

    rtphdrextroi: correct max. number of roi-types

commit ac3c6c4
Author: Camilo Celis Guzman <camilo@pexip.com>
Date:   Fri Oct 29 23:00:14 2021 +0900

    rtphdrextroi: add test for multiple roi-types

commit 1fd0bc8
Author: Camilo Celis Guzman <camilo@pexip.com>
Date:   Thu Oct 28 19:42:04 2021 +0900

    rtphdrextroi: WIP: multiple roi-types support

commit 4b9f2d8
Author: Camilo Celis Guzman <camilo@pexip.com>
Date:   Thu Oct 28 17:19:57 2021 +0900

    rtphdrextroi: add test for case of multiple RoI metas with the same roi-type

    The expected behaviour is that only one of them is payloaded

commit b17f460
Author: Camilo Celis Guzman <camilo@pexip.com>
Date:   Thu Oct 28 17:03:55 2021 +0900

    rtphdrextroi: make roi-type construct only
Fix suppresion to support release and debug builds.

Here is debug build call stack:
```
==10707==    by 0x48B5520: g_malloc (gmem.c:106)
==10707==    by 0x48D19DC: g_slice_alloc (gslice.c:1069)
==10707==    by 0x48D3947: g_slist_copy_deep (gslist.c:619)
==10707==    by 0x48D38B8: g_slist_copy (gslist.c:567)
==10707==    by 0x4ADC90B: gst_debug_remove_with_compare_func (gstinfo.c:1504)
```

In release build `g_slist_copy (gslist.c:567)` got inlined:
```
==15419==    by 0x48963E0: g_malloc (gmem.c:106)
==15419==    by 0x48AA382: g_slice_alloc (gslice.c:1069)
==15419==    by 0x48AB732: g_slist_copy_deep (gslist.c:619)
==15419==    by 0x4A39B8F: gst_debug_remove_with_compare_func (gstinfo.c:1504)
```
doing a sync point request

all_headers set to FALSE stops the FIR request in gstrtpsession.
disabled

If the checks are not built, it will leave behind some "unused"
variables, so this warnings will always trigger.
This allows user-level signal handler to catch this

Co-authored-by: Frederik Vestre <frederik.vestre@pexip.com>
Enables device enumeration and monitor using the ACameraManager API
 execute regardless of the provider being stopped.
Calling destroy_session() without first calling finish() deadlocks almost
every time, due to the explanation given in gst_vtenc_finish_encoding()
@MishaBaranov MishaBaranov force-pushed the misha/handle_wrapping_base_time branch from 2fef5c2 to 145f6a7 Compare May 22, 2025 18:17
tbeloqui and others added 6 commits May 23, 2025 13:57
In the test, adding and removing 50.000 pads, on my machine it goes from
about 14 seconds to about 0,05 seconds.
…lement_dispose`

`gst_element_dispose` could be called more then one time. So we have to
handle it accordingly.

Fix pexip/media#1062
Avoid leaking pad_name in error paths
@MishaBaranov MishaBaranov force-pushed the misha/handle_wrapping_base_time branch from 133d320 to 71bbe6c Compare June 24, 2025 09:10
tbeloqui and others added 10 commits June 30, 2025 15:09
Temporal fix for the flickering on resizing
…3de656d16fd79b791414813ef9c8e

Squashed commit of the following:

commit f01baab
Author: Will Miller <will.miller@pexip.com>
Date:   Mon Jun 30 21:19:32 2025 +0100

    absl: remove safe int parse functions

    These choke on Windows, and we don't need them for dcsctp

commit f74466f
Author: Will Miller <will.miller@pexip.com>
Date:   Mon Jun 30 17:59:12 2025 +0100

    dcsctp: fixes for darwin build

commit 7bb17a8
Author: Will Miller <will.miller@pexip.com>
Date:   Mon Jun 30 14:53:42 2025 +0100

    dcsctp: reapply non-void control fixups

commit 1fb040f
Author: Will Miller <will.miller@pexip.com>
Date:   Mon Jun 30 13:10:40 2025 +0100

    dcsctp: updating to https://webrtc.googlesource.com/src/+/4f422cb71cf3de656d16fd79b791414813ef9c8e
There was a segfault when processing fec block with missing info for a redundnant packet
Allow directly sending a User-Initiated abort with a given string from
the application level. This is useful to simulate scenarios of receiving
such a chunk off the wire in a test, even when an implementation such as
dcsctp might not natively ever provoke such a scenario.
We can't assume the association's socket will always exist, as it gets
torn down when we experience a force-close (due to a timeout, for
example), or a client-received abort chunk. Remove assertions as to the
state of the socket where we can't guarantee it, and replace them with
conditionals. Additionally, ensure we only ever access the socket member
from async handlers to give thread safety.
These checks are no longer required because the underlying dcsctp
library performs send queueing and doesn't need to be protected from
calling the methods early.
@MishaBaranov MishaBaranov force-pushed the misha/handle_wrapping_base_time branch from 71bbe6c to 496f430 Compare August 5, 2025 11:41
camilo-celis and others added 5 commits August 12, 2025 23:46
This is now a re-entrant mutex; so use to corresponding API

This was introduced in https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2740
to avoid deadlocks on repeated initializations.

Fixup of d2742d3
…end by handling empty payloads explicitly

- Replace vector range constructor with explicit assign and empty check
- Avoid constructing std::vector from empty or invalid ranges
- Prevents compiler error and undefined behavior due to invalid pointer deletion

Reported by gcc as:
```
In member function ‘void std::__new_allocator<_Tp>::deallocate(_Tp*, size_type) [with _Tp = unsigned char]’,
    inlined from ‘static void std::allocator_traits<std::allocator<_CharT> >::deallocate(allocator_type&, pointer, size_type) [with _Tp = unsigned char]’ at /opt/gcc-future/include/c++/14.3.0/bits/alloc_traits.h:550:23,
    inlined from ‘std::vector<_Tp, _Alloc>::_M_realloc_append(_Args&& ...)::_Guard::~_Guard() [with _Args = {unsigned char}; _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /opt/gcc-future/include/c++/14.3.0/bits/vector.tcc:616:18,
    inlined from ‘void std::vector<_Tp, _Alloc>::_M_realloc_append(_Args&& ...) [with _Args = {unsigned char}; _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /opt/gcc-future/include/c++/14.3.0/bits/vector.tcc:688:7,
    inlined from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {unsigned char}; _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /opt/gcc-future/include/c++/14.3.0/bits/vector.tcc:123:21,
    inlined from ‘void std::vector<_Tp, _Alloc>::push_back(value_type&&) [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /opt/gcc-future/include/c++/14.3.0/bits/stl_vector.h:1314:21,
    inlined from ‘SctpSocket_SendStatus sctp_socket_send(SctpSocket*, const uint8_t*, size_t, uint16_t, uint32_t, bool, int32_t*, size_t*)’ at /opt/gstreamer/subprojects/gst-plugins-bad/ext/sctp/dcsctp/sctpsocket.cc:400:31:
/opt/gcc-future/include/c++/14.3.0/bits/new_allocator.h:172:33: error: ‘void operator delete(void*, std::size_t)’ called on pointer ‘<unknown>’ with nonzero offset [1, 9223372036854775807] [-Werror=free-nonheap-object]
  172 |         _GLIBCXX_OPERATOR_DELETE(_GLIBCXX_SIZED_DEALLOC(__p, __n));
      |                                 ^
In member function ‘_Tp* std::__new_allocator<_Tp>::allocate(size_type, const void*) [with _Tp = unsigned char]’,
    inlined from ‘static _Tp* std::allocator_traits<std::allocator<_CharT> >::allocate(allocator_type&, size_type) [with _Tp = unsigned char]’ at /opt/gcc-future/include/c++/14.3.0/bits/alloc_traits.h:515:28,
    inlined from ‘std::_Vector_base<_Tp, _Alloc>::pointer std::_Vector_base<_Tp, _Alloc>::_M_allocate(std::size_t) [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /opt/gcc-future/include/c++/14.3.0/bits/stl_vector.h:383:33,
    inlined from ‘void std::vector<_Tp, _Alloc>::_M_range_initialize_n(_Iterator, _Iterator, size_type) [with _Iterator = const unsigned char*; _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /opt/gcc-future/include/c++/14.3.0/bits/stl_vector.h:1716:23,
    inlined from ‘void std::vector<_Tp, _Alloc>::_M_range_initialize(_ForwardIterator, _ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator = const unsigned char*; _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /opt/gcc-future/include/c++/14.3.0/bits/stl_vector.h:1705:25,
    inlined from ‘std::vector<_Tp, _Alloc>::vector(_InputIterator, _InputIterator, const allocator_type&) [with _InputIterator = const unsigned char*; <template-parameter-2-2> = void; _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /opt/gcc-future/include/c++/14.3.0/bits/stl_vector.h:724:23,
    inlined from ‘SctpSocket_SendStatus sctp_socket_send(SctpSocket*, const uint8_t*, size_t, uint16_t, uint32_t, bool, int32_t*, size_t*)’ at /opt/gstreamer/subprojects/gst-plugins-bad/ext/sctp/dcsctp/sctpsocket.cc:393:57:
/opt/gcc-future/include/c++/14.3.0/bits/new_allocator.h:151:55: note: returned from ‘void* operator new(std::size_t)’
  151 |         return static_cast<_Tp*>(_GLIBCXX_OPERATOR_NEW(__n * sizeof(_Tp)));
      |                                                       ^
cc1plus: all warnings being treated as errors
```
Previously the wrapping of the 24-bit reference time was not handled
correctly when transforming it into GstClockTime. Given the unit of 64ms
the span that could be represented by 24 bits is 12 days and depending
on the start value we could get a wrapping problem anytime within this
time frame. This turned out to be particularly problematic for the GCC
algorithm in gst-plugins-rs which tried to evict old packages based on
the "oldest" timestamp, which due to wrapping problems could be in the
future. Thus, the container managing the packets could grow without
limits for a long time thereby creating both CPU and memory problems.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7527>
With
https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7527
an attempt was made to fix the wrapping of the TWCC 24-bit reference
time. Unfortunately this fix was not correct due to a late change and
the lack of unit tests to catch it. This time unit tests are provided to
make sure we have a correct fix for all the edge cases.
@camilo-celis camilo-celis force-pushed the misha/handle_wrapping_base_time branch from 496f430 to 5bfc76c Compare August 13, 2025 05:18
@camilo-celis
Copy link
Contributor

(rebased to bring in gcc-future compilation fixes)

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.