Commit graph

4825 commits

Author SHA1 Message Date
Andy Scherzinger
9ae722659f
Spotbugs: don't doubleCheck Map contains value, just check for null
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:37:19 +01:00
Andy Scherzinger
efdfe83507
Spotbugs: remove NPE deference
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:37:19 +01:00
Andy Scherzinger
8b9996814f
Spotbugs: NPE deference, NPE-equals, unused variable, make vars final, reformat code for line-length 120 chars
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:37:19 +01:00
Andy Scherzinger
a33f3fe400
Spotbug: Simple field is used like an enum
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:37:18 +01:00
Andy Scherzinger
f48575bfec
Spotbug: Method stores return result in local before immediately returning it
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:37:18 +01:00
Andy Scherzinger
c5067b7a60
Spotbug: split message processing to reduce complexity
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:37:00 +01:00
Andy Scherzinger
7b86b02c21
Spotbugs prevent NPR deference
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:37:00 +01:00
Andy Scherzinger
08936279b6
codacy: inefficient use of StringBuffer.toString using call StringBuffer.length instead
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:37:00 +01:00
Andy Scherzinger
b07ee434fe
housekeeping: cleanup after spotbug fixes
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:59 +01:00
Andy Scherzinger
8b61808fda
Spotbug: make constructor-called methods final
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:59 +01:00
Andy Scherzinger
09012cce6a
Spotbugs: remove superclass field masking fields inherited from BaseActivity
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:59 +01:00
Andy Scherzinger
093e6a15bc
Spotbugs: proper equals and hashCode
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:58 +01:00
Andy Scherzinger
bf8c113f9a
Spotbugs: prevent possible NPE deference
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:58 +01:00
Andy Scherzinger
946ec09315
Spotbug: don't access Array with constant index
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:58 +01:00
Andy Scherzinger
ff3dffd051
Spotbugs: literal string comparison
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:57 +01:00
Andy Scherzinger
deada5cf94
Spotbug: remove toString() output concatenation
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:57 +01:00
Andy Scherzinger
6cd0481d14
remove unneeded logging statement
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:57 +01:00
Andy Scherzinger
7eef68ef36
use proper register-method instead of reflection
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:56 +01:00
Andy Scherzinger
7a4785e083
Spotbugs: precize allocation of a collection
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:56 +01:00
Andy Scherzinger
698ebdfd1c
remove unneeded toString() call
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:56 +01:00
Andy Scherzinger
7261f75549
Spotbugs: remove needless boxing of boolean constant
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
2022-12-29 11:36:52 +01:00
Daniel Calviño Sánchez
4516de4add Remove no longer needed condition
Now that the event is posted only for proximity sensor changes the
condition is no longer needed.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:46 +01:00
Daniel Calviño Sánchez
5d7b5160b7 Rename PeerConnectionEvent to ProximitySensorEvent
Proximity sensor events should not have been part of
PeerConnectionEvent. However, now that all the peer connection related
properties were removed the remaining event can be renamed to something
more accurate.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
c8398695f4 Remove no longer needed code after removing EventBus message
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
34498efa72 Rewrite if/else chain as if/return blocks
Just a matter of preference :-)

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
04f1679e2a Add observer for peer connections
The observer is just an adapter for the "PeerConnection.Observer"
provided by the WebRTC library; a custom observer is used to expose only
the events needed outside "PeerConnectionWrapper".

For now only the same events that were already handled are taken into
account, but at a later point additional events (like "onAddTrack"
instead of "onAddStream", which is deprecated) could be added too.

Note that the thread used to handle the events has changed; the EventBus
subscriber mode was "MAIN", but as the events were posted from a
PeerConnection observer, which run in a worker thread rather than in the
main thread, the subscriber was executed in the main thread rather than
in the same thread as the poster. Due to this the actions performed by
the handler now must be explicitly run in the main thread.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
fcbfc1926d Post MediaStreamEvents for each connection state
Rather than simplifying the states to "CONNECTED" and "DISCONNECTED" now
the raw state is posted, and the handler then decides how to treat them
(which, for now, is exactly as before).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
337f3d4b5e Extract methods to handle connections and disconnections of peers
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
4457e92504 Generalize PUBLISHER_FAILED event
Rather than emitting PUBLISHER_FAILED when the publisher connection
fails now PEER_FAILED is emitted when any connection fails, and the
handler checks if the connection was the publisher one to apply the
specific behaviour.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
64c4f8c7ee Remove unneeded condition
The publisher peer connection when the HPB is used is a sender only
connection, so it never adds or removes a remote stream.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
30aafed0e8 Post MediaStreamEvent when the stream is actually added
The MediaStreamEvent was posted when the connection with the remote peer
was established. However, the MediaStream is added earlier (as soon as
the remote description is received), so the event is moved to better
reflect that.

Note that checking if the connection is an MCU publisher is no longer
needed, as publisher connections are sender only and therefore no remote
stream is added for publisher connections.

In any case, note that the stream will not start until the connection is
established, and a progress bar will be anyway shown on the
ParticipantDisplayItem until it is connected.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
29117e8b1b Get user ID from signaling message when creating ParticipantDisplayItem
The user ID set when creating the ParticipantDisplayItem was got from
the join event when the external signaling server was used, and from an
API call when the internal signaling server was used. However, the user
ID is already known from the signaling message that updates the
participant list, and is the one set on the ParticipantDisplayItems
when a participant joins the call. Therefore the other sources are not
needed, so now it is unified to always use the value from the signaling
message.

Note that in the rare cases in which a ParticipantDisplayItem is created
before the participant is seen as in the call the user ID will be
temporary unknown, although it will be automatically fixed once the
participant list update is received. Moreover, even if the other sources
were used it was not guaranteed that the user ID was known, so this
should not be a problem.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:45 +01:00
Daniel Calviño Sánchez
6e222e7cd2 Create ParticipantDisplayItem when creating the connections
The ParticipantDisplayItem for "video" was created when a participant
joined the room, but the item for "screen" was created when the stream
for the screen was connected. Due to this the item for the remote screen
share just "popped up" once connected, but there was no hint of a
connection being established, like done with the video streams.

Now the ParticipantDisplayItems are created as soon as a
PeerConnectionWrapper is created and then updated as needed (for example
to set the user ID or the stream), which causes the item to immediately
appear and a progress bar to be shown until the connection is
established.

Although the ParticipantDisplayItem may be created on a different thread
(the main thread) than the PeerConnectionWrapper "runOnUiThread"
executes the enqueued messages in order (and it also establishes a
"happens-before" relation with further calls of "runOnUiThread" due to
the internal use of synchronized blocks, although it is not explicitly
documented), so the item will be already created when later updated.

This also holds true when an offer is received even before the
participant is seen as joined (a very rare case that can happen if the
signaling message with the updated participant list is lost for any
reason); the ParticipantDisplayItem is created when the offer is
received and it is later updated with the user ID once a new signaling
message with the updated participant list is received.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:48:43 +01:00
Daniel Calviño Sánchez
29d0574667 Create ParticipantDisplayItem on media event if it does not exist yet
Instead of creating a new ParticipantDisplayItem from scratch, which
resets the full grid, now the existing one is updated; a new one is
created only if no item existed already for the session and video stream
type of the media event.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:41:46 +01:00
Daniel Calviño Sánchez
dc53023572 Update displayed nick when received in offers or answers
The nick was displayed when updated through a data channel message, or
when a ParticipantDisplayItem was created and the nick was already
received. However, when the HPB is not used the nick is not sent after a
connection is established, as it was sent already in the offer or
answer. The nick from the offer or answer has not been received yet when
the ParticipantDisplayItem is initially created, so the nick only
appeared because a new ParticipantDisplayItem is created again when the
connection is finally established. Due to all that the displayed nick is
now updated as soon as it is received in an offer or answer, which
ensures that the nick is shown independently of when was the
ParticipantDisplayItem created.

Note that this only applies to non-HPB scenarios; when the HPB is used
the nick is got from the participant list update sent through signaling
messages, so it is already known when creating the display item (in some
very strange cases it might happen that an offer is received before the
participant list was updated, but this should not happen, and in any
case it will be handled at a later point).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 15:41:46 +01:00
Daniel Calviño Sánchez
3762526318 Rewrite if/else chain as if/return blocks
Just a matter of preference :-)

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:56 +01:00
Daniel Calviño Sánchez
dceb4a6d79 Add listener for data channel messages
For now only the same data channel messages that were already handled
are taken into account, but at a later point the missing messages
("speaking" and "stoppedSpeaking") could be added too.

Note that the thread used to handle the data channel messages has
changed; the EventBus subscriber mode was "MAIN", but as the messages
were posted from a DataChannel observer, which run in a worker thread
rather than in the main thread, the subscriber was executed in the main
thread rather than in the same thread as the poster. Due to this the
actions performed by the handler now must be explicitly run in the main
thread.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
a65e56a9ce Remove unneeded condition
If the call is a voice only call there will be no received video tracks
(they would have been stopped when each connection is established), so
changing the enabled state has no effect (as the adapter only tries to
show the received video if it is available).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
ac4be52b84 Do not guard code that can not throw the caught exception
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
af514b142a Reorder code that handles nick changed events
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
81f353f7f0 Return empty display name rather than default nick from web socket
If the display name is not known whether "Guest" or something else needs
to be shown is not a responsibility of the web socket, so now an empty
string is returned instead.

In practice this should not make any difference, though, as the display
name of users is always known as soon as the user joined, and if the
nick of a guest is not known the UI will set it to "Guest".

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
0dcdd6161f Move nick handling out of PeerConnectionWrapper
PeerConnectionWrappers should not be concerned with the nick of
participants. Moreover, the nick is included in offers and answers due
to legacy reasons and only when the internal signaling server is used.
Due to that the nick was moved out of PeerConnectionWrapper; although
the handling is now different the end result should be the same (there
might be some differences in very specific sequences of events, but in
any case all this is just a temporary step and any leftover issue should
be addressed once call participants and peer connections are split).

As the PeerConnectionWrapper does not keep track of the nick now the
nick changed event is always emitted when a nick changed data channel
message is received, even if the nick did not actually change.
Nevertheless, before it was anyway always emitted if it was for a user
and only when it was for a guest it was emitted only on real changes. In
any case this is not expected to cause any issue (other than some
unneeded view updates, but that will be addressed at a later point by
updating the views only when the model actually changed).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
2eac8c2cba Move default nick out of PeerConnectionWrapper
If the nick is not known whether "Guest" or something else needs to be
shown is a responsability of the UI, so now the PeerConnectionWrapper
just returns an empty string and the UI shows the default guest nick if
needed.

Moreover, the nick stored in the PeerConnectionWrapper was not always
correct, as if no nick was received it was returned as "Guest" even
if the connection belonged to a user. Now "Guest" is used only for
actual guests.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
68cf4ee028 Use generic data channel message instead of nick specific one
The generic data channel message works fine for receiving, but it could
not be used for sending, because the serialization of the payload failed
(the generated JsonMapper did not call 'writeFieldName("payload")',
apparently because the payload was defined as "Any", so there was no
field name set when serializing the payload contents).

It is very likely that the nick data channel message, which has an
explicit payload type and was used only for sending but not for
receiving, was added back in the day just to work around that
limitation. However, due to how the JsonMappers are generated if several
properties with the same name are defined only the first one will be
parsed, and only those with a value will be serialized. This makes
possible to define first a generic payload property and then a payload
property with an explicit type to have a single data channel message
class that can be used both for sending and receiving.

As the nick data channel message is now no longer needed it was removed.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
faf25f8071 Replace concrete implementation with interface
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-28 11:47:55 +01:00
Daniel Calviño Sánchez
65ff4efcb9
Send signaling messages directly from PeerConnectionWrapper
Note that the thread used to send the message does not change; the
EventBus subscriber mode was "BACKGROUND", but as the messages were
posted from a WebSocket handler (when requesting offers to the HPB) and
peer connection observers (when sending offers/answers and candidates,
both with and without HPB), which run in worker threads rather than in
the main thread, the subscriber was executed in the same thread as
the poster.

For legacy reasons, when the internal signaling server is used the
offers and answers are expected to also provide the nick of the local
participant. When the external signaling server is used the field can be
included, but it is just ignored and not sent to the other clients. As
the local participant nick is a value unrelated to the peer connection
and is only needed with one type of signaling server the messages are
adjusted as needed before being sent rather than handling this inside
the PeerConnectionWrapper.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-27 14:28:57 +01:00
Daniel Calviño Sánchez
58e371c98c
Do not apply "toLowerCase()" on already lower case canonical form
The canonical form is already in lower case (see
https://webrtc.googlesource.com/src/+/79d8df02/sdk/android/api/org/webrtc/SessionDescription.java#29),
so there is no need to apply "toLowerCase()".

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-27 14:28:57 +01:00
Daniel Calviño Sánchez
473b8b238d
Extract interface to send signaling messages
Like done with SignalingMessageReceiver, an implementation specific to
each signaling server type (internal or external) is added.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-27 14:28:57 +01:00
Daniel Calviño Sánchez
6b032fc55a
Replace constant String of length 1 with character
Fixes UCPM_USE_CHARACTER_PARAMETERIZED_METHOD issue from SpotBugs.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-27 14:28:57 +01:00
Daniel Calviño Sánchez
4086499a32
Remove special method to request offers
As the "requestoffer" message is just a signaling message the generic
method can be used instead.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2022-12-27 14:28:57 +01:00