GNOME Bugzilla – Bug 747863
rtpsession: Use bandwidth calculation by default instead of some arbitrary hardcoded value
Last modified: 2015-11-05 04:20:35 UTC
Just always using 64000 for the bandwidth by default does not make any sense at all.
Created attachment 301564 [details] [review] rtpsession: Use bandwidth calculation by default instead of some arbitrary hardcoded value
I think that would be an ABI break for gst 1.0, so we should target this to 2.0.. Also, anything based on an estimation of the current bandwidth is problematic if you are sending or receiving a sparse stream.
Yeah, for the RTX streams it's probably giving completely wrong results (well, it would go to 8000 probably because of being smaller than that usually). Good point. But OTOH using a fixed and arbitrary 64000 is also not going to work very well. Strictly speaking it's an ABI change, but it might be ok if it only improves things (which it might not because sparse streams). What would you suggest to do here by default (if ABI breakage was not a problem)?
FWIW, I think even for sparse streams this is going to be better than the magic number we currently have ;)
I'm not sure what the right answer is, maybe something automatic with a reasonable minimum, like 64kbps.
kbyte or kbit? I assume the latter, and that would make sense to me too.
kbit, ie that of a 8000hz PCMA/PCMU stream.
Created attachment 302243 [details] [review] rtpsession: Bandwidth is supposed to be in bits/s, not bytes/s
Created attachment 302244 [details] [review] rtpsession: Use bandwidth calculation by default instead of some arbitrary hardcoded value
Created attachment 302245 [details] [review] rtpstats: Use the same lower limit for RTCP bandwidth to stop sending RTCP everywhere
Created attachment 302246 [details] [review] rtpstats: Average RTCP packet size is in bytes, bandwidths in bits We need to convert the size to bits for our calculations.
These patches are doing that now, and at the same time fix up some bits vs bytes confusion. Seems to calculate the right things now.
commit 73c0c2920f9aca96982a4de0c20b3417aa148b81 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Apr 23 18:57:37 2015 +0200 rtpstats: Average RTCP packet size is in bytes, bandwidths in bits We need to convert the size to bits for our calculations. https://bugzilla.gnome.org/show_bug.cgi?id=747863 commit 475b1e607e75f3ac18af4917b2e60b297f5e03ad Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Apr 23 18:53:39 2015 +0200 rtpstats: Use the same lower limit for RTCP bandwidth to stop sending RTCP everywhere https://bugzilla.gnome.org/show_bug.cgi?id=747863 commit 7596ed91b844057f57566c09727339dea99043f7 Author: Sebastian Dröge <sebastian@centricular.com> Date: Tue Apr 14 18:41:07 2015 +0200 rtpsession: Use bandwidth calculation by default instead of some arbitrary hardcoded value https://bugzilla.gnome.org/show_bug.cgi?id=747863 commit 928cd110bcea5d143cab3ea747991851d52ecbad Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Apr 23 18:49:37 2015 +0200 rtpsession: Bandwidth is supposed to be in bits/s, not bytes/s https://bugzilla.gnome.org/show_bug.cgi?id=747863
Created attachment 314817 [details] [review] rtpmanager: Document properties that are expressed in bits per second This changed in 928cd110bcea5d143cab3ea747991851d52ecbad and 73c0c2920f9aca96982a4de0c20b3417aa148b81 but was not documented.
This one is a bit nasty, I admit, so maybe the correct thing to do is keep the bytes/s meaning on the property and do a conversion. However, since this is already in a release, I figure documenting things the way they are is more straightforward.
Comment on attachment 314817 [details] [review] rtpmanager: Document properties that are expressed in bits per second Attachment 314817 [details] pushed as 7e22ea5 - rtpmanager: Document properties that are expressed in bits per second