[stable-15.1] Split call participants and peer connections #2693
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rather than just providing a coarse "connected" or "not connected" value now the views receive the raw ICE connection state. Combined with other properties this will make possible to show a finer grained status (like done in the WebUI), although for now just "connected" or "not connected" is still shown as before. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Note that the session ID, user ID and the stream type attributes are still kept, as they can be useful to identify the instance when debugging. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Generic final attributes first, followed by object specific final attributes and then other object attributes. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Instead of explicitly calling "notifyDataSetChanged" after setting values on a ParticipantDisplayItem now the adapter observes all its items and calls "notifyDataSetChanged" automatically when any of them changes. Although this adds some boilerplate code it will make possible to update the ParticipantDisplayItems and automatically propagate the changes to the adapter when a model changes, rather than having to explicitly do it from the CallActivity. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Clients that modify the model would define the variables using the mutable subclass, while clients that only need to access the model are expected to use the read-only base class. The read-only class provides an observer; as it is expected that the model will be modified from background threads but observed from the main thread the observer can be registered along a handler to be notified on its thread, independently of on which thread the values were set. Currently there does not seem to be a need to observe each value on its own, so the observer is notified in a coarse way when any value changes. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Instead of explicitly setting the values on the ParticipantDisplayItems now the values are set on the CallParticipantModels, and the items are automatically updated from their model when they change. Different items are still used for the audio/video and screen shares of the same participant, so the type is used to select from which properties of the model is the item updated. As the model may be updated from background threads it is explicitly observed by the items from the main thread using a Handler shared by all the items. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
CallParticipant provides a read-only CallParticipantModel and internally handles the data channel and peer connection events that modify the model. Nevertheless, the CallParticipant requires certain properties to be externally set, like the userId or the peer connections. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The listeners for call participant messages and for the call participant nick provided by offers / answers were created and destroyed based on the peer connections, although they were implicitly associated to a call participant. Now they are explicitly created and destroyed based on its associated call participant. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The ParticipantDisplayItems were created and destroyed based on the peer connections. Now a ParticipantDisplayItem of "video" type is associated to a call participant, while an additional item is created and destroyed depending on the state of the screen peer connection of the call participant. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The connection state changes to "closed" only when the connection is closed. However, closing a connection does not fire any event (not even the "iceConnectionStateChanged" event), so the event handler can be removed as it will never be executed. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The observers were created for any peer connection, but after recent changes they ignored all changes but those from the self peer connection. Therefore it is enough to just add an explicit listener on that peer connection rather than on all of them. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
For now only the same signaling messages that were already handled are still handled; in the future it could be extended to handle other messages, like the one sent by the external signaling server when a participant leaves the room (in some cases no participants update message is sent if the participant leaves the call and room at the same time, which causes the participants to still be seen as in call until a new update is received). Signed-off-by: Daniel Calviño Sánchez <[email protected]>
As CallParticipantList starts listening on the signaling messages as soon as it is created it needs to be created and destroyed right before entering and exiting a call. Otherwise it could receive messages on other states (for example, while the "connection timeout" message is shown) and thus once the local participant joined the event would not include the other participants already in the call as joined (although they would be anyway reported as unchanged). Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Instead of trying to create a video peer connection for any joined participant now only a call participant is created for any joined participant, and a video peer connection is created only for those participants that are publishing audio or video. If a call participants does not have a video peer connection the call participant is now seen as "connected" from the UI, as there is no need to show a progress bar for that participant. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The peer connections will be of either "video" or "screen" type, so they can be simply removed based on the session id and an explicit type. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
backportbot-nextcloud
bot
requested review from
danxuliu,
mahibi and
timkrueger
Contributor
Lint
SpotBugs
SpotBugs increased! |
Contributor
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/2693-talk.apk |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
backport of #2600