jamulussoftware / jamulus

Jamulus enables musicians to perform real-time jam sessions over the internet.
https://jamulus.io
Other
1.01k stars 224 forks source link

Refactor UpdateDisplay() to call UpdateUploadRate() #3369

Closed ann0see closed 1 month ago

ann0see commented 2 months ago

Short description of changes

Refactor UpdateDispay() method. This was extracted from https://github.com/jamulussoftware/jamulus/pull/2550 and is related to: https://github.com/jamulussoftware/jamulus/pull/3364/files/21815c1a0708979fe0414c30e0294feaa7632be3#r1759185427

It makes sense to call UpdateUploadRate() from UpdateDisplay() since it gets same functionality to the same place.

Not sure, if the condition is correct, as we do not check if pClient is running, but rather it is connected now.

CHANGELOG: SKIP

Context: Fixes an issue?

No.

Does this change need documentation? What needs to be documented and how?

No. This even adds documentation

Status of this Pull Request

Ready for review

What is missing until this pull request can be merged?

Review. Tested locally by disconnecting and connecting via GUI and changing Audio quality and enabling/disabling small network buffers. Network rate shows correctly. Also checked by starting Jamulus with -c flag and seems to do what it should do: Label shows correct data if connected and --- if not connected.

Checklist

pljones commented 1 month ago

Currently we show the rate only on the settings page.

The factor that connecting to a server affects in the calculation is the "Small Buffers" overhead, I think. I can't think of anything else that would need to check for server support before performing the calculation.

Originally - before "Small Buffers" - I think it was calculated just from the audio settings, as the network overheads were fixed.

So I guess this change makes sense and is, therefore, long overdue.

ann0see commented 1 month ago

@softins could you please have a look at this?