diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt index 8e5eb6f3..4fb6a587 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt @@ -1290,10 +1290,20 @@ class ExoPlayerCore(private val activity: Activity) : Player.Listener { } activity.runOnUiThread { + // Skip when already at the target size: setLayoutParams always schedules a + // layout pass, and this runs from the global-layout listener — re-applying + // equal params would keep the UI thread laying out every frame (#1261). + val current = subtitle.layoutParams as? FrameLayout.LayoutParams + if (current != null && + current.width == subWidth && + current.height == subHeight && + current.gravity == Gravity.CENTER + ) { + return@runOnUiThread + } subtitle.layoutParams = FrameLayout.LayoutParams(subWidth, subHeight).apply { gravity = Gravity.CENTER } - subtitle.requestLayout() } } @@ -1305,8 +1315,11 @@ class ExoPlayerCore(private val activity: Activity) : Player.Listener { fun setBoxFitMode(mode: Int) { if (disposing) return + val resizeMode = boxFitModeToResizeMode(mode.coerceIn(0, 2)) activity.runOnUiThread { - videoAspectContainer?.resizeMode = boxFitModeToResizeMode(mode.coerceIn(0, 2)) + val container = videoAspectContainer ?: return@runOnUiThread + if (container.resizeMode == resizeMode) return@runOnUiThread + container.resizeMode = resizeMode lastVideoSize?.let { vs -> if (vs.width > 0 && vs.height > 0) { updateSubtitleViewSize(vs.width, vs.height, vs.pixelWidthHeightRatio) @@ -1319,6 +1332,7 @@ class ExoPlayerCore(private val activity: Activity) : Player.Listener { if (disposing) return val clamped = scale.coerceIn(0.5, 2.0).toFloat() activity.runOnUiThread { + if (clamped == videoZoomScale) return@runOnUiThread videoZoomScale = clamped videoAspectContainer?.scaleX = clamped videoAspectContainer?.scaleY = clamped diff --git a/lib/services/video_filter_manager.dart b/lib/services/video_filter_manager.dart index 37f8bb93..613a520f 100644 --- a/lib/services/video_filter_manager.dart +++ b/lib/services/video_filter_manager.dart @@ -47,6 +47,16 @@ class VideoFilterManager { /// Debounced video filter update with leading edge execution late final Debounce _debouncedUpdateVideoFilter; + /// Last values actually written to the player. A missing key means unknown, + /// so the next run rewrites it. + final Map _appliedProps = {}; + int? _appliedBoxFitMode; + double? _appliedVideoZoom; + + /// In-progress update loop; concurrent callers mark it dirty and share it. + Future? _updateLoop; + bool _updateDirty = false; + /// Callback invoked when boxFitMode changes, for external persistence final void Function(int mode)? onBoxFitModeChanged; @@ -167,46 +177,92 @@ class VideoFilterManager { } /// Update the video scaling and positioning based on current display mode. + /// Writes are diffed against the last applied values and serialized: while a + /// run is in flight, further calls coalesce into one trailing re-run instead + /// of interleaving stale writes (pinch zoom calls this per gesture tick). /// When ambient lighting is active, video-aspect-override is managed by ambient lighting. - Future updateVideoFilter() async { + Future updateVideoFilter() { + final running = _updateLoop; + if (running != null) { + _updateDirty = true; + return running; + } + final loop = _runUpdateLoop(); + _updateLoop = loop; + return loop; + } + + Future _runUpdateLoop() async { try { + do { + _updateDirty = false; + await _applyVideoFilter(); + } while (_updateDirty); + } finally { + _updateLoop = null; + } + } + + Future _applyVideoFilter() async { + try { + final boxFitMode = _boxFitMode; + final zoomScale = _zoomScale; + final playerSize = _playerSize; + final ambientActive = ambientLightingService?.isEnabled == true; + final coverMode = boxFitMode == 1; + // ExoPlayer handles scaling via AspectRatioFrameLayout (no-op on mpv // backends). The MPV properties below still run — on ExoPlayer they // forward to setMpvProperty, which queues them for any future fallback. - await player.setBoxFitMode(_boxFitMode); - await player.setVideoZoom(_zoomScale); - - if (ambientLightingService?.isEnabled != true) { - await player.setProperty('video-aspect-override', 'no'); + if (_appliedBoxFitMode != boxFitMode) { + _appliedBoxFitMode = null; + await player.setBoxFitMode(boxFitMode); + _appliedBoxFitMode = boxFitMode; + } + if (_appliedVideoZoom != zoomScale) { + _appliedVideoZoom = null; + await player.setVideoZoom(zoomScale); + _appliedVideoZoom = zoomScale; } - await player.setProperty('sub-ass-force-margins', 'no'); - await player.setProperty('panscan', '0'); - await player.setProperty('video-zoom', videoZoomPropertyForScale(_zoomScale).toString()); - if (_boxFitMode == 1) { - // Cover mode - use panscan to fill screen while maintaining aspect ratio - await player.setProperty('panscan', '1.0'); - await player.setProperty('sub-ass-force-margins', 'yes'); - } else if (_boxFitMode == 2) { + // Compute final target values up-front: each mpv write takes effect + // immediately, so transient intermediate values would flash on screen. + String? aspectOverride = ambientActive ? null : 'no'; + if (boxFitMode == 2) { // Fill/stretch mode - override aspect ratio to match player (stretches video) - final playerSize = _playerSize; if (playerSize != null && playerSize.width > 0 && playerSize.height > 0) { final playerAspect = playerSize.width / playerSize.height; if (playerAspect.isFinite && playerAspect > 0) { - await player.setProperty('video-aspect-override', playerAspect.toString()); + aspectOverride = playerAspect.toString(); appLogger.d('Stretch mode: aspect-override=$playerAspect (player: $playerSize)'); } } } - if (_zoomScale > 1.0001) { - await player.setProperty('sub-ass-force-margins', 'yes'); + if (aspectOverride != null) { + await _applyProperty('video-aspect-override', aspectOverride); } + if (ambientActive) { + // Ambient lighting writes video-aspect-override out-of-band, so any + // cached value is unreliable; forget it so the next run rewrites it. + _appliedProps.remove('video-aspect-override'); + } + await _applyProperty('sub-ass-force-margins', coverMode || zoomScale > 1.0001 ? 'yes' : 'no'); + await _applyProperty('panscan', coverMode ? '1.0' : '0'); + await _applyProperty('video-zoom', videoZoomPropertyForScale(zoomScale).toString()); } catch (e) { appLogger.w('Failed to update video filter', error: e); } } + Future _applyProperty(String name, String value) async { + if (_appliedProps[name] == value) return; + // Uncache while in flight so a failed write is retried on the next run. + _appliedProps.remove(name); + await player.setProperty(name, value); + _appliedProps[name] = value; + } + /// Debounced version of updateVideoFilter for resize events. /// Uses leading-edge debounce: first call executes immediately, /// subsequent calls within 50ms are debounced. diff --git a/test/services/video_filter_manager_test.dart b/test/services/video_filter_manager_test.dart index b392c55f..bc4e5379 100644 --- a/test/services/video_filter_manager_test.dart +++ b/test/services/video_filter_manager_test.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:plezy/mpv/mpv.dart'; +import 'package:plezy/services/ambient_lighting_service.dart'; import 'package:plezy/services/video_filter_manager.dart'; void main() { @@ -57,10 +58,105 @@ void main() { expect(aspectWrites, isNotEmpty); expect(double.parse(aspectWrites.last.value), closeTo(16 / 9, 0.0001)); }); + + test('cover-mode zoom change writes only video-zoom', () async { + final player = _RecordingPlayer(); + final manager = VideoFilterManager(player: player, initialBoxFitMode: 1); + addTearDown(manager.dispose); + + await manager.updateVideoFilter(); + player.clearRecords(); + + manager.setZoomScale(1.5); + await Future.delayed(Duration.zero); + + expect(player.boxFitCalls, isEmpty); + expect(player.zoomCalls, [1.5]); + expect(player.writes, hasLength(1)); + expect(player.writes.single.key, 'video-zoom'); + expect(player.writes.single.value, VideoFilterManager.videoZoomPropertyForScale(1.5).toString()); + }); + + test('repeated run with unchanged state writes nothing', () async { + final player = _RecordingPlayer(); + final manager = VideoFilterManager(player: player); + addTearDown(manager.dispose); + + await manager.updateVideoFilter(); + player.clearRecords(); + + await manager.updateVideoFilter(); + + expect(player.writes, isEmpty); + expect(player.boxFitCalls, isEmpty); + expect(player.zoomCalls, isEmpty); + }); + + test('concurrent calls coalesce into one trailing re-run', () async { + final player = _SlowRecordingPlayer(); + final manager = VideoFilterManager(player: player); + addTearDown(manager.dispose); + + final first = manager.updateVideoFilter(); + manager.setZoomScale(0.8); + await first; + + final zoomWrites = player.writes.where((write) => write.key == 'video-zoom').toList(); + expect(zoomWrites, hasLength(2)); + expect(zoomWrites.last.value, VideoFilterManager.videoZoomPropertyForScale(0.8).toString()); + expect(player.writes.where((write) => write.key == 'panscan'), hasLength(1)); + expect(player.writes.where((write) => write.key == 'sub-ass-force-margins'), hasLength(1)); + expect(player.zoomCalls, [1.0, 0.8]); + }); + + test('ambient-active run leaves aspect-override unknown', () async { + final player = _RecordingPlayer(); + final ambient = _FakeAmbientLightingService(player); + final manager = VideoFilterManager(player: player)..ambientLightingService = ambient; + addTearDown(manager.dispose); + + await manager.updateVideoFilter(); + player.clearRecords(); + + ambient.fakeEnabled = true; + await manager.updateVideoFilter(); + expect(player.writes.where((write) => write.key == 'video-aspect-override'), isEmpty); + + ambient.fakeEnabled = false; + await manager.updateVideoFilter(); + final aspectWrites = player.writes.where((write) => write.key == 'video-aspect-override').toList(); + expect(aspectWrites, hasLength(1)); + expect(aspectWrites.single.value, 'no'); + }); + + test('fill mode rewrites aspect on player size change', () async { + final player = _RecordingPlayer(); + final manager = VideoFilterManager(player: player, initialBoxFitMode: 2, initialPlayerSize: const Size(1920, 1080)); + addTearDown(manager.dispose); + + await manager.updateVideoFilter(); + player.clearRecords(); + + manager.updatePlayerSize(const Size(1000, 1000)); + // Cover the 50ms leading+trailing debounce. + await Future.delayed(const Duration(milliseconds: 120)); + + final aspectWrites = player.writes.where((write) => write.key == 'video-aspect-override').toList(); + expect(aspectWrites, hasLength(1)); + expect(double.parse(aspectWrites.single.value), closeTo(1.0, 0.0001)); + }); } class _RecordingPlayer implements Player { final writes = >[]; + final boxFitCalls = []; + final zoomCalls = []; + + void clearRecords() { + writes.clear(); + boxFitCalls.clear(); + zoomCalls.clear(); + } @override Future setProperty(String name, String value) async { @@ -68,12 +164,14 @@ class _RecordingPlayer implements Player { } @override - // ignore: no-empty-block - native-layer call, irrelevant to property recording - Future setBoxFitMode(int mode) async {} + Future setBoxFitMode(int mode) async { + boxFitCalls.add(mode); + } @override - // ignore: no-empty-block - native-layer call, irrelevant to property recording - Future setVideoZoom(double scale) async {} + Future setVideoZoom(double scale) async { + zoomCalls.add(scale); + } @override PlayerState get state => const PlayerState(); @@ -81,3 +179,21 @@ class _RecordingPlayer implements Player { @override dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); } + +/// Delays each property write so single-flight coalescing can be observed. +class _SlowRecordingPlayer extends _RecordingPlayer { + @override + Future setProperty(String name, String value) async { + await super.setProperty(name, value); + await Future.delayed(const Duration(milliseconds: 2)); + } +} + +class _FakeAmbientLightingService extends AmbientLightingService { + _FakeAmbientLightingService(super.player); + + bool fakeEnabled = false; + + @override + bool get isEnabled => fakeEnabled; +}