pion / interceptor

Pluggable RTP/RTCP processors for building real time communication
https://pion.ly/
MIT License
113 stars 62 forks source link

GCC: Maybe bug in kalman filter #286

Closed sterlingdeng closed 4 days ago

sterlingdeng commented 1 week ago

Your environment.

What happened?

The bug lies in this section of code in the Kalman filter.. https://github.com/pion/interceptor/blob/master/pkg/gcc/kalman.go#L81-L84 I think it should be an if else statement, as this is what I'm seeing in another implementation https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/blob/main/net/rtp/src/gcc/imp/kalman_estimator.rs?ref_type=heads#L53-59 and how I interpret it from the RFC

If z(i) > 3sqrt(var_v_hat) the filter is updated with 3sqrt(var_v_hat) rather than z(i).

In https://datatracker.ietf.org/doc/html/draft-ietf-rmcat-gcc-02#section-5.3

It would be great to have another pair of eyes on it

sterlingdeng commented 1 week ago

@mengelbart Could you take a look?

kcaffrey commented 1 week ago

I wasn't involved in writing the GCC implementation, but I agree with your assessment that it should be an if else statement.

mengelbart commented 1 week ago

Yes, I think you are correct. Want to open a PR @sterlingdeng ?

sterlingdeng commented 6 days ago

Yep I can push a PR for this.