From 0a49d2cfa870e3ae8d10a0456ea09d61ea0cb6c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Theys?= Date: Wed, 15 Sep 2021 09:26:35 +0000 Subject: [PATCH] [IMP] mail: speed up time of RTC peers connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this PR, the RPC to notify peers was called too much time too quickly. Reducing request overload significantly improves the overall connection time. closes odoo/odoo#76625 X-original-commit: f4c1d974b5d55868f81cd3259c45e83d934686f5 Signed-off-by: Sébastien Theys (seb) --- addons/mail/controllers/discuss.py | 30 ++--- .../mail/models/mail_channel_rtc_session.py | 33 +++--- .../rtc_call_participant_card.xml | 5 + .../src/components/rtc_video/rtc_video.js | 2 +- .../messaging_notification_handler.js | 10 +- addons/mail/static/src/models/rtc/rtc.js | 108 ++++++++++++++---- .../rtc_peer_notification.js | 38 ++++++ .../src/models/rtc_session/rtc_session.js | 31 ++++- 8 files changed, 191 insertions(+), 66 deletions(-) create mode 100644 addons/mail/static/src/models/rtc_peer_notification/rtc_peer_notification.js diff --git a/addons/mail/controllers/discuss.py b/addons/mail/controllers/discuss.py index 45667a943ce..ddfc33fc59b 100644 --- a/addons/mail/controllers/discuss.py +++ b/addons/mail/controllers/discuss.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import json +from collections import defaultdict from datetime import datetime, timedelta from odoo import http @@ -374,25 +375,24 @@ class DiscussController(http.Controller): # -------------------------------------------------------------------------- @http.route('/mail/rtc/session/notify_call_members', methods=['POST'], type="json", auth="public") - def session_call_notify(self, sender_session_id, target_session_ids=None, content=None): + def session_call_notify(self, peer_notifications): """ Sends content to other session of the same channel, only works if the user is the user of that session. This is used to send peer to peer information between sessions. - :param int sender_session_id: id of the session from which the content is sent - :param list target_session_ids: list of the ids of the sessions that should receive the content - :param content: the content to send to the other sessions + :param peer_notifications: list of tuple with the following elements: + - int sender_session_id: id of the session from which the content is sent + - list target_session_ids: list of the ids of the sessions that should receive the content + - string content: the content to send to the other sessions """ - if request.env.user._is_public(): - guest = request.env['mail.guest']._get_guest_from_request(request) - if guest: - session = guest.env['mail.channel.rtc.session'].sudo().browse(int(sender_session_id)) - if session.exists() and session.guest_id == guest: - session._notify_peers(target_session_ids, content) - return - raise NotFound() - session = request.env['mail.channel.rtc.session'].sudo().browse(int(sender_session_id)) - if session.exists() and session.partner_id == request.env.user.partner_id: - session._notify_peers(target_session_ids, content) + guest = request.env['mail.guest']._get_guest_from_request(request) + notifications_by_session = defaultdict(list) + for sender_session_id, target_session_ids, content in peer_notifications: + session_sudo = guest.env['mail.channel.rtc.session'].sudo().browse(int(sender_session_id)).exists() + if not session_sudo or (session_sudo.guest_id and session_sudo.guest_id != guest) or (session_sudo.partner_id and session_sudo.partner_id != request.env.user.partner_id): + continue + notifications_by_session[session_sudo].append(([int(sid) for sid in target_session_ids], content)) + for session_sudo, notifications in notifications_by_session.items(): + session_sudo._notify_peers(notifications) @http.route('/mail/rtc/session/update_and_broadcast', methods=['POST'], type="json", auth="public") def session_update_and_broadcast(self, session_id, values): diff --git a/addons/mail/models/mail_channel_rtc_session.py b/addons/mail/models/mail_channel_rtc_session.py index 0f84ca98a71..1f4cfead1ae 100644 --- a/addons/mail/models/mail_channel_rtc_session.py +++ b/addons/mail/models/mail_channel_rtc_session.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. +from collections import defaultdict from dateutil.relativedelta import relativedelta from odoo import api, fields, models @@ -99,28 +100,24 @@ class MailRtcSession(models.Model): self.unlink() self.env['bus.bus'].sendmany(notifications) - def _notify_peers(self, target_session_ids, content): + def _notify_peers(self, notifications): """ Used for peer-to-peer communication, guarantees that the sender is the current guest or partner. - :param target_session_ids: a list of mail.channel.rtc.session ids - :param content: a dict with the content to be sent to the targets + :param notifications: list of tuple with the following elements: + - target_session_ids: a list of mail.channel.rtc.session ids + - content: a string with the content to be sent to the targets """ - notifications = [] - target_sessions = self.search([('id', 'in', [int(target) for target in target_session_ids]), ('channel_id', '=', self.channel_id.id)]) - for session in target_sessions: - model, record_id = ('mail.guest', session.guest_id.id) if session.guest_id else ('res.partner', session.partner_id.id) - notifications.append([ - (self._cr.dbname, model, record_id), - { - 'type': 'rtc_peer_notification', - 'payload': { - 'sender': self.id, - 'content': content, - }, - }, - ]) - return self.env['bus.bus'].sendmany(notifications) + self.ensure_one() + payload_by_target = defaultdict(lambda: {'sender': self.id, 'notifications': []}) + for target_session_ids, content in notifications: + for target_session in self.env['mail.channel.rtc.session'].browse(target_session_ids).exists(): + model, record_id = ('mail.guest', target_session.guest_id.id) if target_session.guest_id else ('res.partner', target_session.partner_id.id) + payload_by_target[(self._cr.dbname, model, record_id)]['notifications'].append(content) + return self.env['bus.bus'].sendmany([(target, { + 'type': 'rtc_peer_notification', + 'payload': payload, + }) for target, payload in payload_by_target.items()]) def _mail_rtc_session_format(self): self.ensure_one() diff --git a/addons/mail/static/src/components/rtc_call_participant_card/rtc_call_participant_card.xml b/addons/mail/static/src/components/rtc_call_participant_card/rtc_call_participant_card.xml index 82de94ad20e..02719b3204d 100644 --- a/addons/mail/static/src/components/rtc_call_participant_card/rtc_call_participant_card.xml +++ b/addons/mail/static/src/components/rtc_call_participant_card/rtc_call_participant_card.xml @@ -60,6 +60,11 @@ + + + + + diff --git a/addons/mail/static/src/components/rtc_video/rtc_video.js b/addons/mail/static/src/components/rtc_video/rtc_video.js index 5c1f2b8369f..7527b6bc20f 100644 --- a/addons/mail/static/src/components/rtc_video/rtc_video.js +++ b/addons/mail/static/src/components/rtc_video/rtc_video.js @@ -69,7 +69,7 @@ export class RtcVideo extends Component { async _onVideoLoadedMetaData(ev) { try { await ev.target.play(); - } catch (error) { + } catch (error) { if (typeof error === 'object' && error.name === 'NotAllowedError') { // Ignored as some browsers may reject play() calls that do not // originate from a user input. diff --git a/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js b/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js index 56ce19219ba..067ea1c0ff4 100644 --- a/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js +++ b/addons/mail/static/src/models/messaging_notification_handler/messaging_notification_handler.js @@ -546,11 +546,13 @@ function factory(dependencies) { /** * @private * @param {Object} data - * @param {string} [data.sender] - * @param {string} [data.content] + * @param {string} data.sender + * @param {string[]} data.notifications */ - _handleNotificationRtcPeerToPeer({ sender, content }) { - this.messaging.mailRtc.handleNotification(sender, content); + _handleNotificationRtcPeerToPeer({ sender, notifications }) { + for (const content of notifications) { + this.messaging.mailRtc.handleNotification(sender, content); + } } /** diff --git a/addons/mail/static/src/models/rtc/rtc.js b/addons/mail/static/src/models/rtc/rtc.js index 017605de6f8..5c7348a75b8 100644 --- a/addons/mail/static/src/models/rtc/rtc.js +++ b/addons/mail/static/src/models/rtc/rtc.js @@ -3,8 +3,8 @@ import { browser } from "@web/core/browser/browser"; import { registerNewModel } from '@mail/model/model_core'; -import { attr, one2one } from '@mail/model/model_field'; -import { clear, insert } from '@mail/model/model_field_command'; +import { attr, one2many, one2one } from '@mail/model/model_field'; +import { clear, create, insert, unlink } from '@mail/model/model_field_command'; import { monitorAudio } from '@mail/utils/media_monitoring/media_monitoring'; @@ -622,21 +622,17 @@ function factory(dependencies) { if (!targetTokens.length || !this.channel || !this.currentRtcSession) { return; } - const content = JSON.stringify({ - event, - channelId: this.channel.id, - payload, - }); - if (type === 'server') { - await this.env.services.rpc({ - route: '/mail/rtc/session/notify_call_members', - params: { - sender_session_id: this.currentRtcSession.id, - target_session_ids: targetTokens, - content, - }, - }, { shadow: true }); + this.update({ + peerNotificationsToSend: create({ + channelId: this.channel.id, + event, + payload, + senderId: this.currentRtcSession.id, + targetTokens + }), + }); + await this._sendPeerNotifications(); } if (type === 'peerToPeer') { @@ -645,7 +641,11 @@ function factory(dependencies) { if (!dataChannel || dataChannel.readyState !== 'open') { continue; } - dataChannel.send(content); + dataChannel.send(JSON.stringify({ + event, + channelId: this.channel.id, + payload, + })); } } } @@ -738,6 +738,50 @@ function factory(dependencies) { } } + /** + * Sends this peer notifications to send as soon as the last pending + * sending finishes. + * + * @private + */ + async _sendPeerNotifications() { + if (this.isNotifyPeersRPCInProgress) { + return; + } + this.update({ isNotifyPeersRPCInProgress: true }); + await new Promise(resolve => setTimeout(resolve, this.peerNotificationWaitDelay)); + const peerNotifications = this.peerNotificationsToSend; + try { + await this.env.services.rpc({ + route: '/mail/rtc/session/notify_call_members', + params: { + 'peer_notifications': peerNotifications.map(peerNotification => + [ + peerNotification.senderId, + peerNotification.targetTokens, + JSON.stringify({ + event: peerNotification.event, + channelId: peerNotification.channelId, + payload: peerNotification.payload, + }), + ], + ), + }, + }, { shadow: true }); + if (!this.exists()) { + return; + } + this.update({ peerNotificationsToSend: unlink(peerNotifications) }); + } finally { + if (this.exists()) { + this.update({ isNotifyPeersRPCInProgress: false }); + if (this.peerNotificationsToSend.length > 0) { + this._sendPeerNotifications(); + } + } + } + } + /** * Updates the "isTalking" state of the current user and sets the * enabled state of its audio track accordingly. @@ -1095,13 +1139,6 @@ function factory(dependencies) { currentRtcSession: one2one('mail.rtc_session', { inverse: 'mailRtc', }), - /** - * true if the browser supports webRTC - */ - isClientRtcCompatible: attr({ - compute: '_computeIsClientRtcCompatible', - default: true, - }), /** * ICE servers used by RTCPeerConnection to retrieve the public IP address (STUN) * or to relay packets when necessary (TURN). @@ -1116,6 +1153,16 @@ function factory(dependencies) { }, ], }), + /** + * true if the browser supports webRTC + */ + isClientRtcCompatible: attr({ + compute: '_computeIsClientRtcCompatible', + default: true, + }), + isNotifyPeersRPCInProgress: attr({ + default: false, + }), /** * Contains the logs of the current session by token. * { token: { name, logs } } @@ -1123,6 +1170,19 @@ function factory(dependencies) { logs: attr({ default: {}, }), + peerNotificationsToSend: one2many('mail.rtc_peer_notification', { + isCausal: true, + }), + /** + * Determines the delay to wait (in ms) before sending peer + * notifications to the server. Sending many notifications at once + * significantly increase the connection time because the server can't + * handle too many requests at once, but handles much faster one bigger + * request, even with a delay. The delay should however not be too high. + */ + peerNotificationWaitDelay: attr({ + default: 50, + }), /** * How long to wait before considering a connection as needing recovery in ms. */ diff --git a/addons/mail/static/src/models/rtc_peer_notification/rtc_peer_notification.js b/addons/mail/static/src/models/rtc_peer_notification/rtc_peer_notification.js new file mode 100644 index 00000000000..edc78e3fcd8 --- /dev/null +++ b/addons/mail/static/src/models/rtc_peer_notification/rtc_peer_notification.js @@ -0,0 +1,38 @@ +/** @odoo-module **/ + +import { registerNewModel } from '@mail/model/model_core'; +import { attr } from '@mail/model/model_field'; + +function factory(dependencies) { + + class RTCPeerNotification extends dependencies['mail.model'] { + } + + RTCPeerNotification.fields = { + channelId: attr({ + readonly: true, + required: true, + }), + event: attr({ + readonly: true, + required: true, + }), + payload: attr({ + readonly: true, + }), + senderId: attr({ + readonly: true, + required: true, + }), + targetTokens: attr({ + readonly: true, + required: true, + }), + }; + + RTCPeerNotification.modelName = 'mail.rtc_peer_notification'; + + return RTCPeerNotification; +} + +registerNewModel('mail.rtc_peer_notification', factory); diff --git a/addons/mail/static/src/models/rtc_session/rtc_session.js b/addons/mail/static/src/models/rtc_session/rtc_session.js index 3b42a79b8d2..cf096aca6ac 100644 --- a/addons/mail/static/src/models/rtc_session/rtc_session.js +++ b/addons/mail/static/src/models/rtc_session/rtc_session.js @@ -71,7 +71,12 @@ function factory(dependencies) { async setAudio({ audioStream, isMuted, isTalking }) { this._removeAudio(); const audioElement = this.audioElement || new window.Audio(); - audioElement.srcObject = audioStream; + try { + audioElement.srcObject = audioStream; + } catch (error) { + this.update({ isAudioInError: true }); + console.error(error); + } audioElement.load(); audioElement.volume = this.partner && this.partner.volumeSetting ? this.partner.volumeSetting.volume : this.volume; audioElement.muted = this.messaging.mailRtc.currentRtcSession.isDeaf; @@ -86,13 +91,20 @@ function factory(dependencies) { }); try { await audioElement.play(); - } catch (error) { + if (!this.exists()) { + return; + } + this.update({ isAudioInError: false }); + } catch (error) { if (typeof error === 'object' && error.name === 'NotAllowedError') { // Ignored as some browsers may reject play() calls that do not // originate from a user input. return; } - throw error; + if (this.exists()) { + this.update({ isAudioInError: true }); + } + console.error(error); } } @@ -284,10 +296,15 @@ function factory(dependencies) { } if (this.audioElement) { this.audioElement.pause(); - this.audioElement.srcObject = undefined; + try { + this.audioElement.srcObject = undefined; + } catch (error) { + // ignore error during remove, the value will be overwritten at next usage anyway + } } this.update({ audioStream: clear(), + isAudioInError: false, }); } @@ -345,6 +362,12 @@ function factory(dependencies) { calledChannels: one2many('mail.thread', { inverse: 'rtcInvitingSession', }), + /** + * States whether there is currently an error with the audio element. + */ + isAudioInError: attr({ + default: false, + }), /** * Determines if the user is broadcasting a video from a user device (camera). */