fix(player): pinch zoom flicker from per-tick relayout and filter churn
close #1261
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<String, String> _appliedProps = {};
|
||||
int? _appliedBoxFitMode;
|
||||
double? _appliedVideoZoom;
|
||||
|
||||
/// In-progress update loop; concurrent callers mark it dirty and share it.
|
||||
Future<void>? _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<void> updateVideoFilter() async {
|
||||
Future<void> updateVideoFilter() {
|
||||
final running = _updateLoop;
|
||||
if (running != null) {
|
||||
_updateDirty = true;
|
||||
return running;
|
||||
}
|
||||
final loop = _runUpdateLoop();
|
||||
_updateLoop = loop;
|
||||
return loop;
|
||||
}
|
||||
|
||||
Future<void> _runUpdateLoop() async {
|
||||
try {
|
||||
do {
|
||||
_updateDirty = false;
|
||||
await _applyVideoFilter();
|
||||
} while (_updateDirty);
|
||||
} finally {
|
||||
_updateLoop = null;
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> _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<void> _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.
|
||||
|
||||
@@ -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<void>.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<void>.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 = <MapEntry<String, String>>[];
|
||||
final boxFitCalls = <int>[];
|
||||
final zoomCalls = <double>[];
|
||||
|
||||
void clearRecords() {
|
||||
writes.clear();
|
||||
boxFitCalls.clear();
|
||||
zoomCalls.clear();
|
||||
}
|
||||
|
||||
@override
|
||||
Future<void> 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<void> setBoxFitMode(int mode) async {}
|
||||
Future<void> setBoxFitMode(int mode) async {
|
||||
boxFitCalls.add(mode);
|
||||
}
|
||||
|
||||
@override
|
||||
// ignore: no-empty-block - native-layer call, irrelevant to property recording
|
||||
Future<void> setVideoZoom(double scale) async {}
|
||||
Future<void> 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<void> setProperty(String name, String value) async {
|
||||
await super.setProperty(name, value);
|
||||
await Future<void>.delayed(const Duration(milliseconds: 2));
|
||||
}
|
||||
}
|
||||
|
||||
class _FakeAmbientLightingService extends AmbientLightingService {
|
||||
_FakeAmbientLightingService(super.player);
|
||||
|
||||
bool fakeEnabled = false;
|
||||
|
||||
@override
|
||||
bool get isEnabled => fakeEnabled;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user