[jitsi-dev] [jitsi/jitsi-videobridge] slight rework to LastNController to make it easier to override its (#273)


#1

behavior.

idea here is to tweak a couple things to make it easier to override the specific logic of determining which endpoints should be in last-n (we do some different things here, so have a custom HighfiveLastNController to implement the logic we need).
You can view, comment on, or merge this pull request online at:

  https://github.com/jitsi/jitsi-videobridge/pull/273

-- Commit Summary --

  * slight rework to LastNController to make it easier to override its

-- File Changes --

    M src/main/java/org/jitsi/videobridge/LastNController.java (35)

-- Patch Links --

https://github.com/jitsi/jitsi-videobridge/pull/273.patch
https://github.com/jitsi/jitsi-videobridge/pull/273.diff

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273


#2

cc @bgrozev

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#issuecomment-234674041


#3

Jenkins test again please

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#issuecomment-234809625


#4

LGTM, will wait for the tests to run again.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#issuecomment-234809882


#5

Jenkins, test this please

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#issuecomment-234944285


#6

jenkins test this please

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#issuecomment-235264977


#7

Merged #273.

···

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#event-735055189


#8

@brianh5 looking at the base determineLastNList method, it's parameter is unused. Do you use the parameter in your extended implementation, or can we remove it?

···

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#issuecomment-243221168


#9

@bgrozev yeah i do use it. memory is a bit fuzzy, but i think since we drive new last-n changes on new endpoints joining (to fill without activity if we have empty slots), there are times where an endpoint is on the new endpoints list, but isn't in conferenceSpeechActivityEndpoints

···

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#issuecomment-243229442


#10

Thanks, that makes sense.

···

--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/273#issuecomment-243248954