Skip to content

rtpbin: Refactor synchronization / offsetting code

Sebastian Dröge requested to merge slomo/gstreamer:rtpbin-rtp-info into main
commit 9cb4e24d38380447e54e3e7162f90e1d868c78fa
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Thu Apr 18 09:16:05 2024 +0300

    rtpbin: Regularly emit the sync signal
    
    Even if no new synchronization information is available.
    
    This is necessary because the timestamp offset logic in rtpbin depends
    on the base RTP time that is determined by the jitterbuffer, but this
    changes all the time (especially in mode=slave) and the timestamp
    offsets have to be updated accordingly. Doing so is especially important
    if they're only determined by the RTP-Info, which never changes from the
    very beginning.
    
    The interval can be configured via the new min-sync-interval property.
    Synchronization happens at least that often, but at most as often as the
    old sync-interval property allows.
    Both intervals are now based on the monotonic system clock.
    
    Additionally, clean up synchronization code a bit, only emit either
    inband NTP or RTCP SR synchronization at the same time, based on which
    one has the more recent time information, and only emit RTP-Info
    synchronization if it wasn't provided previously at the same time as the
    NTP-based synchronization information.

commit 4b148ff8c52e3b34fde4a0cd3ee11f9a2f14185d
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 17:49:11 2024 +0300

    rtpjitterbuffer: Set max-rtcp-rtp-sync-time to -1 (disabled)
    
    There is generally no requirement to ignore RTCP SR if the RTP time of
    the SR differs a lot from the last received RTP packet. The mapping
    between RTP and NTP time stays valid until there was a stream reset, in
    which case we wouldn't use that information anyway.
    
    When using rtcp-sync-send-time=false the default of 1s difference can
    easily be exceeded, e.g. if encoding of the stream after capture adds
    more than 1s of latency.

commit e2ed2c2a6a220e831c6f58c831787e21c9950bb0
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 17:31:28 2024 +0300

    rtpbin: Allow synchronizing against RTP-Info without having received any RTCP
    
    Previously the information was provided from rtpjitterbuffer to rtpbin
    only once the first RTCP SR was received, which is not necessary at all
    as all required information is available from the caps already.
    
    Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1162

commit feffee643ec37a3722ada5f678d80975d13ade05
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 17:17:16 2024 +0300

    rtpbin: Add new never/ntp RTCP sync modes
    
    Never is useful for some RTSP servers that report plain garbage both via
    RTCP SR and RTP-Info, for example.
    
    NTP is useful if synchronization should only ever happen based on RTCP
    SR or NTP-64 RTP header extension.
    
    Also slightly change the behaviour of always/initial to take RTP-Info
    based synchronization into account too. It's supposed to give the same
    values as the RTCP SR and is available earlier, so will generally cause
    fewer synchronization glitches if it's made use of.

commit 25513f2a0faec9ec85f3dc1d9002d8820055656d
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 17:12:13 2024 +0300

    rtpbin: Handle switches between RTP-Info and NTP-based stream association better
    
    Instead of switching on the very first stream, require that all streams
    have switched before switching to the different synchronization
    mechanism.
    
    Without this there will be a noticeable gap during the switch. E.g. when
    going from RTP-Info to NTP-based association, first the first stream
    only would get an offset, then the first two, ... then all of them.
    Depending on the order of streams this will cause a lot of changes in
    ts-offset during the transition.

commit 734059b2829c3a49269daa0433d0d301cc7bae0e
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 17:10:03 2024 +0300

    rtpbin: Pass NPT start from rtpjitterbuffer to rtpbin
    
    And use it to detect synchronization changes (e.g. seeks) more reliably
    when doing RTP-Info based synchronization.

commit 380bb11fd56380d699ae050759d9352c0321b4fa
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 17:01:21 2024 +0300

    rtpbin: Clean up stream association state
    
    Use fewer magic numbers and keep track of the different synchronization
    mechanisms separately. Also keep track of more state to detect more
    situations when resynchronization should happen.

commit 053e3b6d7cf3fd92fdfd2c1941cc12551cc4aa88
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 16:50:30 2024 +0300

    rtpbin: Constify function parameters and use correct types
    
    Previously these parameters were randomly changed in the body of the
    function to avoid having to declare a new variable, which made the code
    very hard to follow. By marking them as const this won't be possible
    anymore in the future.
    
    Also the RTP clock-base (RTP time from RTSP RTP-Info) is an unsigned
    64 bit integer as it's an extended RTP timestamp.

commit 855a6a9ff1175468f4e44f8bee7b8441579ca464
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 13:44:38 2024 +0300

    rtpbin: Untangle NTP-based and RTP-Info based stream association
    
    Both were entangled previously and very hard to follow what happens
    under which conditions. Now as a very first step the code decides which
    of the two cases it is going to apply, and then proceeds accordingly.
    This also avoids calculating completely invalid values along the way and
    even printing them int the debug output.
    
    Also improve debug output in various places.
    
    This shouldn't cause any behaviour changes.

commit 5cde32d7cb50a684ed9bb68f6dd5a48ad8afe76c
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 13:33:41 2024 +0300

    rtpbin: Remove unused variable / function parameter

commit a58fc6c0de030be6d0caa0487af71f3b31c1b51e
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 13:30:42 2024 +0300

    rtpbin: Handle ntp-sync=true before everything else
    
    This simplifies the code as it's a much simpler case than the normal
    inter-stream synchronization, and interleaving it with that only
    reduces readability of the code.
    
    Also improve some debug output in this code path.

commit c491047c3b11db2fdeac61c53340736fa498c0d7
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 13:29:08 2024 +0300

    rtpbin: Add some documentation to gst_rtp_bin_associate()

commit 95234b042bf6e108841e1189d3cb724f18b06a92
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Wed Apr 17 13:21:08 2024 +0300

    rtpbin: Don't do any timestamp offsetting in rfc7273-sync=true mode
    
    Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1160
Edited by Sebastian Dröge

Merge request reports