Skip to content
  • Håvard Graff's avatar
    rtpjitterbuffer: don't send multiple instant RTX for the same packet · 63c7a9ae
    Håvard Graff authored
    Due to us not properly acknowleding the time when the last RTX was sent
    when scheduling a new one, it can easily happen that due to the packet
    you are requesting have a PTS that is slightly old (but not too old when
    adding the latency of the jitterbuffer), both its calculated second and
    third (etc.) timeout could already have passed. This would lead to a burst
    of RTX requests, which acts completely against its purpose, potentially
    spending a lot more bandwidth than needed.
    
    This has been properly reproduced in the test:
    test_rtx_not_bursting_requests
    
    The good news is that slightly re-thinking the logic concerning
    re-requesting RTX, made it a lot simpler to understand, and allows us
    to remove two members of the RtpTimer which no longer serves any purpose
    due to the refactoring. If desirable the whole "delay" concept can actually
    be removed completely from the timers, and simply just added to the timeout
    by the caller of the API. But that can be a change for a another time.
    
    The only external change (other than the improved behavior around bursting
    RTX) is that the "delay" field now stricly represents the delay between
    the PTS of the RTX-requested packet and the time it is requested on,
    whereas before this calculation was more about the theoretical calculated
    delay. This is visible in three other RTX-tests where the delay had
    to be adjusted slightly. I am confident however that this change is
    correct.
    
    Part-of: <!789>
    63c7a9ae