Skip to content

Commit dcdd2f9

Browse files
authored
feat: enhance PeerConnectionManager with RTP traffic monitoring, disposal barrier, and pretty JSON logging (#864)
* refactor: extract PeerConnectionManager logic and implement disposal barrier - Extract `RTCPeerConnection` lifecycle management (create, retrieve, dispose) into `PeerConnectionManager`. - Implement `PeerConnectionFactory` interface to abstract native WebRTC calls for testability. - Add "Disposal Barrier" mechanism to prevent race conditions during rapid call cycling. - Remove inline `Completer` logic from `CallBloc`, simplifying the state machine. - Add unit and integration tests covering race conditions, zombie connections, and timeouts. * test: migrate PeerConnectionManager tests to integration_test binding * refactor: make DefaultPeerConnectionFactory configurable and reuse default ICE config * docs: clarify PeerConnectionManager factory responsibility for connection configuration * test: update PeerConnectionManager unit tests for factory API and verify close on replacement * fix: log dispose errors when cleaning up peer connections * feat: add RTP traffic monitoring with scoped logging and wire into PeerConnectionManager * refactor: simplify RTP monitor logging and adjust configuration notes * refactor: clarify PeerConnectionManager variable naming and improve event logging * docs: document non-awaited peer connection disposal and disposal barrier rationale * refactor: simplify RTP traffic logging delegate configuration * refactor: inject PeerConnectionManager from MainShell * fix: align logPretty calls with new (data, tag) signature after rebase
1 parent 69c35d1 commit dcdd2f9

9 files changed

Lines changed: 1486 additions & 216 deletions

File tree

integration_test/features/call/utils/peer_connection_manager_integration_test.dart

Lines changed: 454 additions & 0 deletions
Large diffs are not rendered by default.

lib/app/router/main_shell.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,11 @@ class _MainShellState extends State<MainShell> with WidgetsBindingObserver {
404404
// If CDRs feature is disabled, the worker will be null and no sync will be performed
405405
final cdrsSyncWorker = context.readOrNull<CdrsSyncWorker>();
406406

407+
final peerConnectionManager = PeerConnectionManager(
408+
retrieveTimeout: kPeerConnectionRetrieveTimeout,
409+
monitorDelegatesFactory: (callId, logger) => [LoggingRtpTrafficMonitorDelegate(logger: logger)],
410+
);
411+
407412
return CallBloc(
408413
coreUrl: appBloc.state.session.coreUrl!,
409414
tenantId: appBloc.state.session.tenantId,
@@ -439,6 +444,7 @@ class _MainShellState extends State<MainShell> with WidgetsBindingObserver {
439444
DiagnosticType.androidCallkeepOnly,
440445
extras: {'callId': id, 'error': error.name},
441446
),
447+
peerConnectionManager: peerConnectionManager,
442448
)..add(const CallStarted());
443449
},
444450
),

lib/features/call/bloc/call_bloc.dart

Lines changed: 110 additions & 216 deletions
Large diffs are not rendered by default.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import 'package:flutter_webrtc/flutter_webrtc.dart';
2+
import 'package:logging/logging.dart';
3+
4+
import 'package:webtrit_phone/extensions/logger_extensions.dart';
5+
6+
import 'rtp_traffic_monitor.dart';
7+
8+
class LoggingRtpTrafficMonitorDelegate implements RtpTrafficMonitorDelegate {
9+
LoggingRtpTrafficMonitorDelegate({required this.logger, this.prettyPrintData = true});
10+
11+
final Logger logger;
12+
13+
final bool prettyPrintData;
14+
15+
@override
16+
void onStatsUpdated(RtpTrafficContext context, StatsReport report, RtpTrafficMetrics metrics) {
17+
_logSummary(context, metrics);
18+
19+
if (prettyPrintData) {
20+
_logDetails(context, report);
21+
}
22+
}
23+
24+
void _logSummary(RtpTrafficContext context, RtpTrafficMetrics metrics) {
25+
final directionStr = context.direction.name.toUpperCase();
26+
final kindStr = context.kind.name.toUpperCase();
27+
final statusStr = metrics.isFlowing ? 'Flowing' : 'STALLED';
28+
final sign = metrics.deltaBytes > 0 ? '+' : '';
29+
30+
logger.finer(
31+
'[$directionStr $kindStr] '
32+
'Bytes: $sign${metrics.deltaBytes} | '
33+
'Frames: ${metrics.deltaFrames} | '
34+
'Status: $statusStr',
35+
);
36+
}
37+
38+
void _logDetails(RtpTrafficContext context, StatsReport report) {
39+
logger.logPretty(report.values, tag: '${context.kind.name} ${context.direction.name}-rtp data', level: Level.INFO);
40+
}
41+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import 'package:flutter_webrtc/flutter_webrtc.dart';
2+
3+
/// Abstract factory to create [RTCPeerConnection] instances.
4+
abstract interface class PeerConnectionFactory {
5+
Future<RTCPeerConnection> create([
6+
Map<String, dynamic> configuration = const {},
7+
Map<String, dynamic> constraints = const {},
8+
]);
9+
}
10+
11+
/// Default implementation using the actual Flutter WebRTC plugin.
12+
class DefaultPeerConnectionFactory implements PeerConnectionFactory {
13+
static const Map<String, dynamic> _defaultIceConfiguration = {
14+
'iceServers': [
15+
{'url': 'stun:stun.l.google.com:19302'},
16+
],
17+
};
18+
19+
final Map<String, dynamic> _defaultConfiguration;
20+
21+
const DefaultPeerConnectionFactory({Map<String, dynamic> defaultConfiguration = _defaultIceConfiguration})
22+
: _defaultConfiguration = defaultConfiguration;
23+
24+
@override
25+
Future<RTCPeerConnection> create([
26+
Map<String, dynamic> configuration = const {},
27+
Map<String, dynamic> constraints = const {},
28+
]) {
29+
// Use default configuration only if the provided one is empty.
30+
// This allows passing specific configurations when needed.
31+
final effectiveConfiguration = configuration.isEmpty ? _defaultConfiguration : configuration;
32+
33+
return createPeerConnection(effectiveConfiguration, constraints);
34+
}
35+
}

0 commit comments

Comments
 (0)