From 1be1334794e8f115ed217d92298f477d967762eb Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Fri, 24 Feb 2023 09:53:57 +0100 Subject: [PATCH] fix memory leak in local video track --- .../xmpp/jingle/JingleRtpConnection.java | 22 +++++++--- .../xmpp/jingle/TrackWrapper.java | 18 +++++--- .../xmpp/jingle/WebRTCWrapper.java | 16 ++++++- .../ui/activity/RtpSessionActivity.java | 4 ++ .../ui/widget/SurfaceViewRenderer.java | 1 + .../android/xmpp/XmppConnection.java | 7 ++- .../xmpp/manager/JingleConnectionManager.java | 28 +++++++++--- .../android/xmpp/model/error/Error.java | 13 ++++++ .../model/jingle/error/JingleCondition.java | 44 +++++++++++++++++++ 9 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 app/src/main/java/im/conversations/android/xmpp/model/jingle/error/JingleCondition.java diff --git a/app/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java b/app/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java index 92ff5d895..49380bfea 100644 --- a/app/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/app/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -44,6 +44,7 @@ import im.conversations.android.xmpp.manager.JingleConnectionManager; import im.conversations.android.xmpp.model.disco.external.Service; import im.conversations.android.xmpp.model.error.Condition; import im.conversations.android.xmpp.model.error.Error; +import im.conversations.android.xmpp.model.jingle.error.JingleCondition; import im.conversations.android.xmpp.model.jmi.Accept; import im.conversations.android.xmpp.model.jmi.JingleMessage; import im.conversations.android.xmpp.model.jmi.Proceed; @@ -587,9 +588,11 @@ public class JingleRtpConnection extends AbstractJingleConnection final Set removeSummary = ContentAddition.summary(receivedContentRemove); if (contentAddSummary.equals(removeSummary)) { + LOGGER.info("Retracting content {}", removeSummary); this.incomingContentAdd = null; updateEndUserState(); } else { + LOGGER.info("content add summary {} did not match remove summary {}", contentAddSummary, removeSummary); webRTCWrapper.close(); sendSessionTerminate( Reason.FAILED_APPLICATION, @@ -1767,7 +1770,6 @@ public class JingleRtpConnection extends AbstractJingleConnection private void send(final JinglePacket jinglePacket) { jinglePacket.setTo(id.with); connection.sendIqPacket(jinglePacket, this::handleIqResponse); - connection.sendIqPacket(jinglePacket, this::handleIqResponse); } private synchronized void handleIqResponse(final Iq response) { @@ -1840,18 +1842,26 @@ public class JingleRtpConnection extends AbstractJingleConnection private void respondWithTieBreak(final Iq jinglePacket) { respondWithJingleError( - jinglePacket, "tie-break", Error.Type.CANCEL, new Condition.Conflict()); + jinglePacket, + new JingleCondition.TieBreak(), + Error.Type.CANCEL, + new Condition.Conflict()); } private void respondWithOutOfOrder(final Iq jinglePacket) { respondWithJingleError( - jinglePacket, "out-of-order", Error.Type.WAIT, new Condition.UnexpectedRequest()); + jinglePacket, + new JingleCondition.OutOfOrder(), + Error.Type.WAIT, + new Condition.UnexpectedRequest()); } private void respondWithJingleError( - final Iq original, String jingleCondition, final Error.Type type, Condition condition) { - // TODO add jingle condition - connection.sendErrorFor(original, type, condition); + final Iq original, + JingleCondition jingleCondition, + final Error.Type type, + Condition condition) { + connection.sendErrorFor(original, type, condition, jingleCondition); } private void respondOk(final Iq jinglePacket) { diff --git a/app/src/main/java/eu/siacs/conversations/xmpp/jingle/TrackWrapper.java b/app/src/main/java/eu/siacs/conversations/xmpp/jingle/TrackWrapper.java index 061214048..2093fbf18 100644 --- a/app/src/main/java/eu/siacs/conversations/xmpp/jingle/TrackWrapper.java +++ b/app/src/main/java/eu/siacs/conversations/xmpp/jingle/TrackWrapper.java @@ -38,7 +38,7 @@ class TrackWrapper { final RtpTransceiver transceiver = peerConnection == null ? null : getTransceiver(peerConnection, trackWrapper); if (transceiver == null) { - Log.w(Config.LOGTAG, "unable to detect transceiver for " + trackWrapper.rtpSender.id()); + Log.w(Config.LOGTAG, "unable to detect transceiver for " + trackWrapper.getRtpSenderId()); return Optional.of(trackWrapper.track); } final RtpTransceiver.RtpTransceiverDirection direction = transceiver.getDirection(); @@ -51,13 +51,21 @@ class TrackWrapper { } } + public String getRtpSenderId() { + try { + return track.id(); + } catch (final IllegalStateException e) { + return null; + } + } + public static RtpTransceiver getTransceiver( @Nonnull final PeerConnection peerConnection, final TrackWrapper trackWrapper) { - final RtpSender rtpSender = trackWrapper.rtpSender; + final String rtpSenderId = trackWrapper.getRtpSenderId(); for (final RtpTransceiver transceiver : peerConnection.getTransceivers()) { - if (transceiver.getSender().id().equals(rtpSender.id())) { - return transceiver; - } + if (transceiver.getSender().id().equals(rtpSenderId)) { + return transceiver; + } } return null; } diff --git a/app/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java b/app/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java index ec05e6b71..e3fe639e9 100644 --- a/app/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/app/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -216,6 +216,14 @@ public class WebRTCWrapper { } } + private static void dispose(final VideoTrack videoTrack) { + try { + videoTrack.dispose(); + } catch (final IllegalStateException e) { + Log.e(Config.LOGTAG, "unable to dispose of video track", e); + } + } + public void setup( final Context service, @Nonnull final AppRTCAudioManager.SpeakerPhonePreference speakerPhonePreference) @@ -441,6 +449,7 @@ public class WebRTCWrapper { final VideoSourceWrapper videoSourceWrapper = this.videoSourceWrapper; final AppRTCAudioManager audioManager = this.appRTCAudioManager; final EglBase eglBase = this.eglBase; + final var localVideoTrack = this.localVideoTrack; if (peerConnection != null) { this.peerConnection = null; dispose(peerConnection); @@ -449,7 +458,10 @@ public class WebRTCWrapper { ToneManager.getInstance(context).setAppRtcAudioManagerHasControl(false); mainHandler.post(audioManager::stop); } - this.localVideoTrack = null; + if (localVideoTrack != null) { + this.localVideoTrack = null; + dispose(localVideoTrack.track); + } this.remoteVideoTrack = null; if (videoSourceWrapper != null) { this.videoSourceWrapper = null; @@ -461,8 +473,8 @@ public class WebRTCWrapper { videoSourceWrapper.dispose(); } if (eglBase != null) { - eglBase.release(); this.eglBase = null; + eglBase.release(); } if (peerConnectionFactory != null) { this.peerConnectionFactory = null; diff --git a/app/src/main/java/im/conversations/android/ui/activity/RtpSessionActivity.java b/app/src/main/java/im/conversations/android/ui/activity/RtpSessionActivity.java index 660d90eb6..28aef0e75 100644 --- a/app/src/main/java/im/conversations/android/ui/activity/RtpSessionActivity.java +++ b/app/src/main/java/im/conversations/android/ui/activity/RtpSessionActivity.java @@ -610,9 +610,13 @@ public class RtpSessionActivity extends BaseActivity final WeakReference weakReference = this.rtpConnectionReference; final JingleRtpConnection jingleRtpConnection = weakReference == null ? null : weakReference.get(); + final var jmc = this.jingleConnectionManager; if (jingleRtpConnection != null) { releaseVideoTracks(jingleRtpConnection); } + if (jmc != null) { + jmc.removeOnJingleRtpConnectionUpdate(this); + } releaseProximityWakeLock(); super.onStop(); } diff --git a/app/src/main/java/im/conversations/android/ui/widget/SurfaceViewRenderer.java b/app/src/main/java/im/conversations/android/ui/widget/SurfaceViewRenderer.java index 6f400a79e..f9583c18e 100644 --- a/app/src/main/java/im/conversations/android/ui/widget/SurfaceViewRenderer.java +++ b/app/src/main/java/im/conversations/android/ui/widget/SurfaceViewRenderer.java @@ -20,6 +20,7 @@ public class SurfaceViewRenderer extends org.webrtc.SurfaceViewRenderer { super(context, attrs); } + @Override public void onFrameResolutionChanged(int videoWidth, int videoHeight, int rotation) { super.onFrameResolutionChanged(videoWidth, videoHeight, rotation); final int rotatedWidth = rotation != 0 && rotation != 180 ? videoHeight : videoWidth; diff --git a/app/src/main/java/im/conversations/android/xmpp/XmppConnection.java b/app/src/main/java/im/conversations/android/xmpp/XmppConnection.java index 8704e3942..6a5b74e1c 100644 --- a/app/src/main/java/im/conversations/android/xmpp/XmppConnection.java +++ b/app/src/main/java/im/conversations/android/xmpp/XmppConnection.java @@ -1791,7 +1791,11 @@ public class XmppConnection extends AbstractAccountService implements Runnable { this.sendPacket(response); } - public void sendErrorFor(final Iq request, final Error.Type type, final Condition condition) { + public void sendErrorFor( + final Iq request, + final Error.Type type, + final Condition condition, + final Error.Extension... extensions) { final var from = request.getFrom(); final var id = request.getId(); final var response = new Iq(Iq.Type.ERROR); @@ -1800,6 +1804,7 @@ public class XmppConnection extends AbstractAccountService implements Runnable { final Error error = response.addExtension(new Error()); error.setType(type); error.setCondition(condition); + error.addExtensions(extensions); this.sendPacket(response); } diff --git a/app/src/main/java/im/conversations/android/xmpp/manager/JingleConnectionManager.java b/app/src/main/java/im/conversations/android/xmpp/manager/JingleConnectionManager.java index a7b5c8ed1..4ec440d86 100644 --- a/app/src/main/java/im/conversations/android/xmpp/manager/JingleConnectionManager.java +++ b/app/src/main/java/im/conversations/android/xmpp/manager/JingleConnectionManager.java @@ -32,6 +32,7 @@ import im.conversations.android.xml.Namespace; import im.conversations.android.xmpp.XmppConnection; import im.conversations.android.xmpp.model.error.Condition; import im.conversations.android.xmpp.model.error.Error; +import im.conversations.android.xmpp.model.jingle.error.JingleCondition; import im.conversations.android.xmpp.model.jmi.Accept; import im.conversations.android.xmpp.model.jmi.JingleMessage; import im.conversations.android.xmpp.model.jmi.Proceed; @@ -87,7 +88,10 @@ public class JingleConnectionManager extends AbstractManager { final String sessionId = packet.getSessionId(); if (sessionId == null) { respondWithJingleError( - iq, "unknown-session", Error.Type.CANCEL, new Condition.ItemNotFound()); + iq, + new JingleCondition.UnknownSession(), + Error.Type.CANCEL, + new Condition.ItemNotFound()); return; } final AbstractJingleConnection.Id id = AbstractJingleConnection.Id.of(packet); @@ -123,9 +127,10 @@ public class JingleConnectionManager extends AbstractManager { } connection = new JingleRtpConnection(context, this.connection, id, from); } else { + // TODO this is probably the wrong jingle error condition respondWithJingleError( packet, - "unsupported-info", + new JingleCondition.UnsupportedInfo(), Error.Type.CANCEL, new Condition.FeatureNotImplemented()); return; @@ -135,7 +140,10 @@ public class JingleConnectionManager extends AbstractManager { } else { Log.d(Config.LOGTAG, "unable to route jingle packet: " + packet); respondWithJingleError( - packet, "unknown-session", Error.Type.CANCEL, new Condition.ItemNotFound()); + packet, + new JingleCondition.UnknownSession(), + Error.Type.CANCEL, + new Condition.ItemNotFound()); } } @@ -225,9 +233,11 @@ public class JingleConnectionManager extends AbstractManager { } private void respondWithJingleError( - final Iq original, String jingleCondition, final Error.Type type, Condition condition) { - // TODO add jingle condition - connection.sendErrorFor(original, type, condition); + final Iq original, + final JingleCondition jingleCondition, + final Error.Type type, + Condition condition) { + connection.sendErrorFor(original, type, condition, jingleCondition); } public void handle(final Message message) { @@ -771,6 +781,12 @@ public class JingleConnectionManager extends AbstractManager { this.onJingleRtpConnectionUpdate = listener; } + public void removeOnJingleRtpConnectionUpdate(final OnJingleRtpConnectionUpdate listener) { + if (this.onJingleRtpConnectionUpdate == listener) { + this.onJingleRtpConnectionUpdate = null; + } + } + public RtpSessionNotification getNotificationService() { return this.rtpSessionNotification; } diff --git a/app/src/main/java/im/conversations/android/xmpp/model/error/Error.java b/app/src/main/java/im/conversations/android/xmpp/model/error/Error.java index 699092d07..c542705e6 100644 --- a/app/src/main/java/im/conversations/android/xmpp/model/error/Error.java +++ b/app/src/main/java/im/conversations/android/xmpp/model/error/Error.java @@ -28,10 +28,23 @@ public class Error extends Extension { this.setAttribute("type", type.toString().toLowerCase(Locale.ROOT)); } + public void addExtensions(final Extension[] extensions) { + for (final Extension extension : extensions) { + this.addExtension(extension); + } + } + public enum Type { MODIFY, CANCEL, AUTH, WAIT } + + public static class Extension extends im.conversations.android.xmpp.model.Extension { + + public Extension(Class clazz) { + super(clazz); + } + } } diff --git a/app/src/main/java/im/conversations/android/xmpp/model/jingle/error/JingleCondition.java b/app/src/main/java/im/conversations/android/xmpp/model/jingle/error/JingleCondition.java new file mode 100644 index 000000000..f798c15ab --- /dev/null +++ b/app/src/main/java/im/conversations/android/xmpp/model/jingle/error/JingleCondition.java @@ -0,0 +1,44 @@ +package im.conversations.android.xmpp.model.jingle.error; + +import im.conversations.android.annotation.XmlElement; +import im.conversations.android.xml.Namespace; +import im.conversations.android.xmpp.model.error.Error; + +public abstract class JingleCondition extends Error.Extension { + + private JingleCondition(Class clazz) { + super(clazz); + } + + @XmlElement(namespace = Namespace.JINGLE_ERRORS) + public static class OutOfOrder extends JingleCondition { + + public OutOfOrder() { + super(OutOfOrder.class); + } + } + + @XmlElement(namespace = Namespace.JINGLE_ERRORS) + public static class TieBreak extends JingleCondition { + + public TieBreak() { + super(TieBreak.class); + } + } + + @XmlElement(namespace = Namespace.JINGLE_ERRORS) + public static class UnknownSession extends JingleCondition { + + public UnknownSession() { + super(UnknownSession.class); + } + } + + @XmlElement(namespace = Namespace.JINGLE_ERRORS) + public static class UnsupportedInfo extends JingleCondition { + + public UnsupportedInfo() { + super(UnsupportedInfo.class); + } + } +}