From 246f3c634f93f39c99d4cb59e1bdf4efad4f0b42 Mon Sep 17 00:00:00 2001 From: hm21 Date: Mon, 27 Jul 2026 10:36:50 +0200 Subject: [PATCH 1/2] fix(video-player): loop where both tracks still have content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An mp4's declared duration is its longest track, and capture and export pipelines routinely stop the audio and video tracks tens of milliseconds apart. Playing to the container duration therefore ends on a stretch where one track has already run out — silence, or a frozen last frame. On the feed's looping player that stretch is replayed every cycle, which is the seam reported in #6386. A survey of 622 published uploads plus 58 archived Vines found 12-19% of assets with a >=40ms mismatch, the worst at 280ms. It is a property of the asset, not of the player, so it cannot be fixed by re-encoding what is already published: Blossom is content-addressed, and the sha256 is the identity. Clips can now opt into ending where every track still has content, and feed playback does. Apple takes the minimum of the two track ends when building the composition, which costs nothing because both tracks are already loaded there. Android reads the per-track durations with MediaExtractor and passes the minimum to the clipping configuration — for local files only, because probing a remote source would block playback start on a network round trip, and the feed prefetches to disk anyway. Clamping only ever shortens; an explicit trim that ends earlier still wins. --- .../DivineVideoPlayerInstance.kt | 64 +++++++++- .../DivineVideoPlayerInstance.swift | 16 ++- .../lib/src/video_clip.dart | 28 +++++ .../src/loop_seam_trim_contract_test.dart | 110 ++++++++++++++++++ .../test/src/video_clip_test.dart | 19 +++ .../lib/src/utils/source_loader.dart | 2 + .../lib/src/widgets/infinite_video_feed.dart | 3 + .../test/src/utils/source_loader_test.dart | 20 ++++ 8 files changed, 258 insertions(+), 4 deletions(-) create mode 100644 mobile/packages/divine_video_player/test/src/loop_seam_trim_contract_test.dart diff --git a/mobile/packages/divine_video_player/android/src/main/kotlin/com/divinevideo/divine_video_player/DivineVideoPlayerInstance.kt b/mobile/packages/divine_video_player/android/src/main/kotlin/com/divinevideo/divine_video_player/DivineVideoPlayerInstance.kt index 1c86777d45..375dbae52c 100644 --- a/mobile/packages/divine_video_player/android/src/main/kotlin/com/divinevideo/divine_video_player/DivineVideoPlayerInstance.kt +++ b/mobile/packages/divine_video_player/android/src/main/kotlin/com/divinevideo/divine_video_player/DivineVideoPlayerInstance.kt @@ -1,6 +1,8 @@ package com.divinevideo.divine_video_player import android.content.Context +import android.media.MediaExtractor +import android.media.MediaFormat import android.net.Uri import android.os.Handler import android.os.Looper @@ -415,6 +417,18 @@ internal class DivineVideoPlayerInstance( } val startMs = (map["startMs"] as? Number)?.toLong() ?: 0L val endMs = (map["endMs"] as? Number)?.toLong() + val trimToCommonTrackEnd = map["trimToCommonTrackEnd"] as? Boolean ?: false + // The container duration is the *longest* track, so ending there + // leaves a stretch where the shorter track has already run out — + // silence, or a frozen frame. On a looping player that stretch is + // the seam. Clamping may only ever shorten: an earlier explicit + // trim still wins. + val commonEndMs = if (trimToCommonTrackEnd) { + commonTrackEndMs(uri)?.takeIf { it > startMs } + } else { + null + } + val effectiveEndMs = listOfNotNull(endMs, commonEndMs).minOrNull() val clipVol = (map["volume"] as? Number)?.toFloat() ?: 1.0f val clipSpeed = ((map["playbackSpeed"] as? Number)?.toFloat() ?: 1.0f) .coerceAtLeast(MIN_PLAYBACK_SPEED) @@ -436,7 +450,7 @@ internal class DivineVideoPlayerInstance( MediaItem.ClippingConfiguration.Builder() .setStartPositionMs(startMs) .apply { - if (endMs != null) setEndPositionMs(endMs) + if (effectiveEndMs != null) setEndPositionMs(effectiveEndMs) } .build(), ) @@ -450,8 +464,8 @@ internal class DivineVideoPlayerInstance( // Offsets accumulate in playback time so the global timeline // matches what the editor UI shows (slow clips occupy more // space, fast clips less). - if (endMs != null) { - accumulated += sourceToPlaybackMs(endMs - startMs, clipSpeed) + if (effectiveEndMs != null) { + accumulated += sourceToPlaybackMs(effectiveEndMs - startMs, clipSpeed) } } @@ -529,6 +543,50 @@ internal class DivineVideoPlayerInstance( mainHandler.postDelayed(setClipsTimeoutRunnable, SET_CLIPS_TIMEOUT_MS) } + /** + * The point up to which *every* track of [uri] still has content, in + * milliseconds, or `null` when it cannot be determined without I/O that + * would delay playback. + * + * Only local files are probed. Reading a remote source's metadata means a + * network round trip on the platform thread before playback can start, so + * remote clips keep the container duration — and the seam — instead. + */ + private fun commonTrackEndMs(uri: String): Long? { + val path = when { + uri.startsWith("/") -> uri + uri.startsWith("file://") -> Uri.parse(uri).path + else -> null + } ?: return null + + val extractor = MediaExtractor() + return try { + extractor.setDataSource(path) + var videoUs = -1L + var audioUs = -1L + for (i in 0 until extractor.trackCount) { + val format = extractor.getTrackFormat(i) + val mime = format.getString(MediaFormat.KEY_MIME) ?: continue + if (!format.containsKey(MediaFormat.KEY_DURATION)) continue + val durationUs = format.getLong(MediaFormat.KEY_DURATION) + when { + mime.startsWith("video/") && videoUs < 0 -> videoUs = durationUs + mime.startsWith("audio/") && audioUs < 0 -> audioUs = durationUs + } + } + // A clip without both track types has no mismatch to trim. + if (videoUs <= 0 || audioUs <= 0) null else minOf(videoUs, audioUs) / 1000 + } catch (e: Exception) { + DivineVideoPlayerLog.warning( + "Player $playerId could not read track durations: $e", + name = "DivineVideoPlayer.Load", + ) + null + } finally { + extractor.release() + } + } + private fun handleSeekTo(call: MethodCall, result: MethodChannel.Result) { val globalMs = (call.argument("positionMs"))?.toLong() ?: 0L val exoPlayer = ensurePlayer() diff --git a/mobile/packages/divine_video_player/darwin/divine_video_player/Sources/divine_video_player/DivineVideoPlayerInstance.swift b/mobile/packages/divine_video_player/darwin/divine_video_player/Sources/divine_video_player/DivineVideoPlayerInstance.swift index 3883a9d2ab..4fccc43679 100644 --- a/mobile/packages/divine_video_player/darwin/divine_video_player/Sources/divine_video_player/DivineVideoPlayerInstance.swift +++ b/mobile/packages/divine_video_player/darwin/divine_video_player/Sources/divine_video_player/DivineVideoPlayerInstance.swift @@ -342,6 +342,8 @@ final class DivineVideoPlayerInstance: NSObject, FlutterStreamHandler { let clipVol = (clipMap["volume"] as? NSNumber)?.floatValue ?? 1.0 let clipSpeed = (clipMap["playbackSpeed"] as? NSNumber)?.doubleValue ?? 1.0 let httpHeaders = clipMap["httpHeaders"] as? [String: String] + let trimToCommonTrackEnd = + (clipMap["trimToCommonTrackEnd"] as? NSNumber)?.boolValue ?? false let url: URL if uri.hasPrefix("/") { @@ -387,7 +389,7 @@ final class DivineVideoPlayerInstance: NSObject, FlutterStreamHandler { let standardizedTransform = transform.standardized(for: naturalSize) let startTime = CMTime(value: startMs, timescale: 1000) - let endTime: CMTime + var endTime: CMTime if let endMs { // Clamp to the asset: insertTimeRange silently inserts only the // media that exists, so an endMs past the source would leave @@ -404,6 +406,18 @@ final class DivineVideoPlayerInstance: NSObject, FlutterStreamHandler { } else { endTime = assetDuration } + // The asset duration is the *longest* track, so ending there leaves + // a stretch where the shorter track has already run out — silence, + // or a frozen frame. On a looping player that stretch is the seam. + // Clamping may only ever shorten: an earlier explicit trim wins. + if trimToCommonTrackEnd, let sourceAudioTrack = assetAudioTracks.first { + let videoRange = try await sourceVideoTrack.load(.timeRange) + let audioRange = try await sourceAudioTrack.load(.timeRange) + let commonEnd = CMTimeMinimum(videoRange.end, audioRange.end) + if commonEnd.isNumeric, CMTimeCompare(commonEnd, startTime) > 0 { + endTime = CMTimeMinimum(endTime, commonEnd) + } + } let timeRange = CMTimeRange(start: startTime, end: endTime) let clipDuration = CMTimeSubtract(endTime, startTime) guard CMTimeCompare(clipDuration, .zero) > 0 else { diff --git a/mobile/packages/divine_video_player/lib/src/video_clip.dart b/mobile/packages/divine_video_player/lib/src/video_clip.dart index fbc7c30a95..107868960e 100644 --- a/mobile/packages/divine_video_player/lib/src/video_clip.dart +++ b/mobile/packages/divine_video_player/lib/src/video_clip.dart @@ -22,6 +22,7 @@ class VideoClip { this.volume = 1.0, this.playbackSpeed = 1.0, this.httpHeaders = const {}, + this.trimToCommonTrackEnd = false, }); /// Creates a [VideoClip] from a local file path. @@ -32,6 +33,7 @@ class VideoClip { this.volume = 1.0, this.playbackSpeed = 1.0, this.httpHeaders = const {}, + this.trimToCommonTrackEnd = false, }) : uri = path; /// Creates a [VideoClip] from a network URL. @@ -42,6 +44,7 @@ class VideoClip { this.volume = 1.0, this.playbackSpeed = 1.0, this.httpHeaders = const {}, + this.trimToCommonTrackEnd = false, }) : uri = url; /// Creates a [VideoClip] from a Flutter asset. @@ -55,6 +58,7 @@ class VideoClip { double volume = 1.0, double playbackSpeed = 1.0, AssetBundle? bundle, + bool trimToCommonTrackEnd = false, }) async { final (data, dir) = await ( (bundle ?? rootBundle).load(assetPath), @@ -70,6 +74,7 @@ class VideoClip { end: end, volume: volume, playbackSpeed: playbackSpeed, + trimToCommonTrackEnd: trimToCommonTrackEnd, ); } @@ -84,6 +89,7 @@ class VideoClip { Duration? end, double volume = 1.0, double playbackSpeed = 1.0, + bool trimToCommonTrackEnd = false, }) async { final dir = await getTemporaryDirectory(); final file = File('${dir.path}/divine_player_memory/$fileName'); @@ -95,6 +101,7 @@ class VideoClip { end: end, volume: volume, playbackSpeed: playbackSpeed, + trimToCommonTrackEnd: trimToCommonTrackEnd, ); } @@ -122,6 +129,26 @@ class VideoClip { /// HTTP headers to attach when [uri] resolves to a network source. final Map httpHeaders; + /// Whether to end the clip where *all* of the source's tracks still have + /// content, instead of at the container duration. + /// + /// An mp4's declared duration is the longest of its tracks, and capture and + /// export pipelines routinely let the audio and video tracks end tens of + /// milliseconds apart. Playing to the container duration therefore ends on a + /// stretch where one track has already run out — silence, or a frozen last + /// frame. On a looping player that stretch is the loop seam. + /// + /// Set this for looping playback of a single clip. It shortens the clip by + /// the track mismatch (typically < 100 ms), so it is wrong for a clip in the + /// middle of a multi-clip timeline, where it would cut content rather than a + /// seam. Clamping only ever shortens: [end], when set, still wins if it is + /// earlier. + /// + /// Support is per-platform: Apple always honours it, Android honours it for + /// local files only (a remote source would need a blocking metadata read + /// before playback could start), and the web and Linux backends ignore it. + final bool trimToCommonTrackEnd; + /// Serializes this clip for platform channel transport. Map toMap() { return { @@ -131,6 +158,7 @@ class VideoClip { 'volume': volume, 'playbackSpeed': playbackSpeed, if (httpHeaders.isNotEmpty) 'httpHeaders': httpHeaders, + if (trimToCommonTrackEnd) 'trimToCommonTrackEnd': true, }; } } diff --git a/mobile/packages/divine_video_player/test/src/loop_seam_trim_contract_test.dart b/mobile/packages/divine_video_player/test/src/loop_seam_trim_contract_test.dart new file mode 100644 index 0000000000..03043594a2 --- /dev/null +++ b/mobile/packages/divine_video_player/test/src/loop_seam_trim_contract_test.dart @@ -0,0 +1,110 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('loop seam trim contract', () { + test('Apple clamps the clip to the shorter of the two tracks', () { + final source = _appleSourceFile().readAsStringSync(); + + expect( + source, + contains('trimToCommonTrackEnd'), + reason: + 'Apple must honour the flag; without it a looping clip replays ' + 'the tail where one track has already ended.', + ); + expect( + source, + contains('CMTimeMinimum(videoRange.end, audioRange.end)'), + reason: + 'The clip must end where both tracks still have content, not at ' + 'the asset duration, which is the longest track.', + ); + expect( + source, + contains('CMTimeMinimum(endTime, commonEnd)'), + reason: + 'Clamping may only ever shorten a clip — an explicit trim that ' + 'ends earlier still wins.', + ); + }); + + test('Android clamps the clipping configuration, not just endMs', () { + final source = _androidSourceFile().readAsStringSync(); + + expect( + source, + contains('commonTrackEndMs'), + reason: + 'Android must resolve the point where both tracks still have ' + 'content before building the clipping configuration.', + ); + expect( + source, + contains('listOfNotNull(endMs, commonEndMs).minOrNull()'), + reason: + 'Clamping may only ever shorten a clip — an explicit trim that ' + 'ends earlier still wins.', + ); + expect( + source, + contains('setEndPositionMs(effectiveEndMs)'), + reason: + 'The clamped end must reach ExoPlayer; setting the raw endMs ' + 'would leave the seam in place.', + ); + }); + + test('Android probes local files only', () { + final source = _androidSourceFile().readAsStringSync(); + + expect( + source, + contains('MediaExtractor'), + reason: + 'Per-track durations are not exposed by the ExoPlayer ' + 'timeline, which reports the longest track.', + ); + expect( + source, + contains(RegExp(r'uri\.startsWith\("file://"\)')), + reason: + 'Probing a remote source would block playback start on a network ' + 'round trip, so only local files may be read.', + ); + }); + }); +} + +File _appleSourceFile() { + final packageRelative = File( + 'darwin/divine_video_player/Sources/divine_video_player/' + 'DivineVideoPlayerInstance.swift', + ); + if (packageRelative.existsSync()) { + return packageRelative; + } + + return File( + 'packages/divine_video_player/' + 'darwin/divine_video_player/Sources/divine_video_player/' + 'DivineVideoPlayerInstance.swift', + ); +} + +File _androidSourceFile() { + final packageRelative = File( + 'android/src/main/kotlin/com/divinevideo/divine_video_player/' + 'DivineVideoPlayerInstance.kt', + ); + if (packageRelative.existsSync()) { + return packageRelative; + } + + return File( + 'packages/divine_video_player/' + 'android/src/main/kotlin/com/divinevideo/divine_video_player/' + 'DivineVideoPlayerInstance.kt', + ); +} diff --git a/mobile/packages/divine_video_player/test/src/video_clip_test.dart b/mobile/packages/divine_video_player/test/src/video_clip_test.dart index acc38b2371..aeb1357822 100644 --- a/mobile/packages/divine_video_player/test/src/video_clip_test.dart +++ b/mobile/packages/divine_video_player/test/src/video_clip_test.dart @@ -147,6 +147,25 @@ void main() { equals({'Authorization': 'Nostr token'}), ); }); + + test('serializes trimToCommonTrackEnd only when opted in', () { + const plain = VideoClip(uri: 'test.mp4'); + const trimmed = VideoClip(uri: 'test.mp4', trimToCommonTrackEnd: true); + + expect(plain.toMap(), isNot(contains('trimToCommonTrackEnd'))); + expect(trimmed.toMap()['trimToCommonTrackEnd'], isTrue); + }); + + test('carries trimToCommonTrackEnd through file and network', () { + const file = VideoClip.file('/a.mp4', trimToCommonTrackEnd: true); + const network = VideoClip.network( + 'https://example.com/a.mp4', + trimToCommonTrackEnd: true, + ); + + expect(file.toMap()['trimToCommonTrackEnd'], isTrue); + expect(network.toMap()['trimToCommonTrackEnd'], isTrue); + }); }); group('asset', () { diff --git a/mobile/packages/infinite_video_feed/lib/src/utils/source_loader.dart b/mobile/packages/infinite_video_feed/lib/src/utils/source_loader.dart index 156e75f7a1..230c6bb75d 100644 --- a/mobile/packages/infinite_video_feed/lib/src/utils/source_loader.dart +++ b/mobile/packages/infinite_video_feed/lib/src/utils/source_loader.dart @@ -66,6 +66,7 @@ Future<(String, int)> setSourceWithFallbacks({ source, end: maxPlaybackDuration, httpHeaders: httpHeadersForSource?.call(source) ?? const {}, + trimToCommonTrackEnd: true, ), ); abortIfStale(source); @@ -109,6 +110,7 @@ Future<(String, int)> setSourceWithFallbacks({ source, end: maxPlaybackDuration, httpHeaders: httpHeadersForSource?.call(source) ?? const {}, + trimToCommonTrackEnd: true, ), ); abortIfStale(source); diff --git a/mobile/packages/infinite_video_feed/lib/src/widgets/infinite_video_feed.dart b/mobile/packages/infinite_video_feed/lib/src/widgets/infinite_video_feed.dart index dd9afbbe02..d8bfc4f226 100644 --- a/mobile/packages/infinite_video_feed/lib/src/widgets/infinite_video_feed.dart +++ b/mobile/packages/infinite_video_feed/lib/src/widgets/infinite_video_feed.dart @@ -1165,6 +1165,7 @@ class InfiniteVideoFeedState extends State { VideoClip.file( cachedFile.path, end: widget.maxPlaybackDuration, + trimToCommonTrackEnd: true, ), ); if (!guardInitOwnership('setSource(cache)')) return; @@ -1387,6 +1388,7 @@ class InfiniteVideoFeedState extends State { nextSource, end: widget.maxPlaybackDuration, httpHeaders: _httpHeadersByIndex[index] ?? const {}, + trimToCommonTrackEnd: true, ), ); if (index == _currentIndex && _isActive && _canAutoPlayAt(index)) { @@ -1483,6 +1485,7 @@ class InfiniteVideoFeedState extends State { source, end: widget.maxPlaybackDuration, httpHeaders: _httpHeadersByIndex[index] ?? const {}, + trimToCommonTrackEnd: true, ), ); if (!guardRetryOwnership('setSource')) return; diff --git a/mobile/packages/infinite_video_feed/test/src/utils/source_loader_test.dart b/mobile/packages/infinite_video_feed/test/src/utils/source_loader_test.dart index 7b61d489a5..78d40ba35c 100644 --- a/mobile/packages/infinite_video_feed/test/src/utils/source_loader_test.dart +++ b/mobile/packages/infinite_video_feed/test/src/utils/source_loader_test.dart @@ -71,6 +71,26 @@ void main() { expect(clips.map((clip) => clip.end), equals([cap, cap])); }); + test('opts every source into loop-seam trimming', () async { + final controller = FakeController(); + addTearDown(controller.dispose); + + await setSourceWithFallbacks( + index: 0, + controller: controller, + sources: ['urlA'], + log: logs.add, + ); + + expect( + controller.lastSource?.trimToCommonTrackEnd, + isTrue, + reason: + 'Feed playback loops, so a clip must end where both tracks still ' + 'have content rather than at the container duration.', + ); + }); + test('passes headers for the selected source', () async { final controller = FakeController(); addTearDown(controller.dispose); From f963cfc03797cea93a5b632e233baf04803e3cde Mon Sep 17 00:00:00 2001 From: Liz Sweigart <127434495+NotThatKindOfDrLiz@users.noreply.github.com> Date: Mon, 27 Jul 2026 10:52:42 -0500 Subject: [PATCH 2/2] fix(video-player): bound common track loop trim --- .../DivineVideoPlayerInstance.kt | 43 +++++++++++++- .../DivineVideoPlayerInstance.swift | 57 +++++++++++++++++-- .../lib/src/video_clip.dart | 12 ++-- .../src/loop_seam_trim_contract_test.dart | 43 ++++++++++++-- 4 files changed, 138 insertions(+), 17 deletions(-) diff --git a/mobile/packages/divine_video_player/android/src/main/kotlin/com/divinevideo/divine_video_player/DivineVideoPlayerInstance.kt b/mobile/packages/divine_video_player/android/src/main/kotlin/com/divinevideo/divine_video_player/DivineVideoPlayerInstance.kt index 375dbae52c..10b9d141c1 100644 --- a/mobile/packages/divine_video_player/android/src/main/kotlin/com/divinevideo/divine_video_player/DivineVideoPlayerInstance.kt +++ b/mobile/packages/divine_video_player/android/src/main/kotlin/com/divinevideo/divine_video_player/DivineVideoPlayerInstance.kt @@ -424,7 +424,7 @@ internal class DivineVideoPlayerInstance( // the seam. Clamping may only ever shorten: an earlier explicit // trim still wins. val commonEndMs = if (trimToCommonTrackEnd) { - commonTrackEndMs(uri)?.takeIf { it > startMs } + boundedCommonTrackEndMs(uri, startMs, endMs) } else { null } @@ -552,7 +552,11 @@ internal class DivineVideoPlayerInstance( * network round trip on the platform thread before playback can start, so * remote clips keep the container duration — and the seam — instead. */ - private fun commonTrackEndMs(uri: String): Long? { + private fun boundedCommonTrackEndMs( + uri: String, + startMs: Long, + requestedEndMs: Long?, + ): Long? { val path = when { uri.startsWith("/") -> uri uri.startsWith("file://") -> Uri.parse(uri).path @@ -575,7 +579,16 @@ internal class DivineVideoPlayerInstance( } } // A clip without both track types has no mismatch to trim. - if (videoUs <= 0 || audioUs <= 0) null else minOf(videoUs, audioUs) / 1000 + if (videoUs <= 0 || audioUs <= 0) { + null + } else { + boundedCommonTrackEndMs( + startMs = startMs, + requestedEndMs = requestedEndMs, + videoEndMs = videoUs / 1000, + audioEndMs = audioUs / 1000, + ) + } } catch (e: Exception) { DivineVideoPlayerLog.warning( "Player $playerId could not read track durations: $e", @@ -587,6 +600,26 @@ internal class DivineVideoPlayerInstance( } } + private fun boundedCommonTrackEndMs( + startMs: Long, + requestedEndMs: Long?, + videoEndMs: Long, + audioEndMs: Long, + ): Long? { + val containerEndMs = maxOf(videoEndMs, audioEndMs) + val playbackEndMs = minOf(requestedEndMs ?: containerEndMs, containerEndMs) + val commonEndMs = minOf(videoEndMs, audioEndMs) + val trimMs = playbackEndMs - commonEndMs + if (commonEndMs <= startMs || trimMs <= 0) return null + + val playableDurationMs = playbackEndMs - startMs + if (playableDurationMs <= 0) return null + + val relativeLimitMs = (playableDurationMs * MAX_COMMON_TRACK_END_TRIM_RATIO).toLong() + val trimLimitMs = minOf(MAX_COMMON_TRACK_END_TRIM_MS, relativeLimitMs) + return commonEndMs.takeIf { trimMs <= trimLimitMs } + } + private fun handleSeekTo(call: MethodCall, result: MethodChannel.Result) { val globalMs = (call.argument("positionMs"))?.toLong() ?: 0L val exoPlayer = ensurePlayer() @@ -1294,6 +1327,10 @@ internal class DivineVideoPlayerInstance( */ private const val MIN_PLAYBACK_SPEED = 0.001f + /** Maximum tail considered an encoder/export track-end mismatch. */ + private const val MAX_COMMON_TRACK_END_TRIM_MS = 500L + private const val MAX_COMMON_TRACK_END_TRIM_RATIO = 0.10 + /** * How many times an editing/preview player re-prepares after a * transient decoder error before giving up. Small: a couple of diff --git a/mobile/packages/divine_video_player/darwin/divine_video_player/Sources/divine_video_player/DivineVideoPlayerInstance.swift b/mobile/packages/divine_video_player/darwin/divine_video_player/Sources/divine_video_player/DivineVideoPlayerInstance.swift index 4fccc43679..f0e567a4fb 100644 --- a/mobile/packages/divine_video_player/darwin/divine_video_player/Sources/divine_video_player/DivineVideoPlayerInstance.swift +++ b/mobile/packages/divine_video_player/darwin/divine_video_player/Sources/divine_video_player/DivineVideoPlayerInstance.swift @@ -33,6 +33,8 @@ final class DivineVideoPlayerInstance: NSObject, FlutterStreamHandler { private static let setClipsTimeoutMs = 10_000 private static let bufferingStallMs = 8_000 + private static let maxCommonTrackEndTrimMs = 500.0 + private static let maxCommonTrackEndTrimRatio = 0.10 /// AVFoundation's asset-option key for per-asset HTTP request headers. /// @@ -411,11 +413,23 @@ final class DivineVideoPlayerInstance: NSObject, FlutterStreamHandler { // or a frozen frame. On a looping player that stretch is the seam. // Clamping may only ever shorten: an earlier explicit trim wins. if trimToCommonTrackEnd, let sourceAudioTrack = assetAudioTracks.first { - let videoRange = try await sourceVideoTrack.load(.timeRange) - let audioRange = try await sourceAudioTrack.load(.timeRange) - let commonEnd = CMTimeMinimum(videoRange.end, audioRange.end) - if commonEnd.isNumeric, CMTimeCompare(commonEnd, startTime) > 0 { - endTime = CMTimeMinimum(endTime, commonEnd) + do { + let videoRange = try await sourceVideoTrack.load(.timeRange) + let audioRange = try await sourceAudioTrack.load(.timeRange) + if let commonEnd = boundedCommonTrackEnd( + startTime: startTime, + requestedEnd: endTime, + videoEnd: videoRange.end, + audioEnd: audioRange.end + ) { + endTime = CMTimeMinimum(endTime, commonEnd) + } + } catch { + DivineVideoPlayerLog.shared.warning( + "Player \(playerId) could not read track durations: " + + "\(error.localizedDescription)", + name: "DivineVideoPlayer.Load" + ) } } let timeRange = CMTimeRange(start: startTime, end: endTime) @@ -741,6 +755,39 @@ final class DivineVideoPlayerInstance: NSObject, FlutterStreamHandler { } } + private func boundedCommonTrackEnd( + startTime: CMTime, + requestedEnd: CMTime, + videoEnd: CMTime, + audioEnd: CMTime + ) -> CMTime? { + let containerEnd = CMTimeMaximum(videoEnd, audioEnd) + let playbackEnd = CMTimeMinimum(requestedEnd, containerEnd) + let commonEnd = CMTimeMinimum(videoEnd, audioEnd) + guard commonEnd.isNumeric, + playbackEnd.isNumeric, + CMTimeCompare(commonEnd, startTime) > 0, + CMTimeCompare(playbackEnd, commonEnd) > 0 + else { + return nil + } + + let trimMs = CMTimeSubtract(playbackEnd, commonEnd).seconds * 1000 + let playableDurationMs = CMTimeSubtract(playbackEnd, startTime).seconds * 1000 + guard trimMs.isFinite, + playableDurationMs.isFinite, + playableDurationMs > 0 + else { + return nil + } + + let trimLimitMs = min( + Self.maxCommonTrackEndTrimMs, + playableDurationMs * Self.maxCommonTrackEndTrimRatio + ) + return trimMs <= trimLimitMs ? commonEnd : nil + } + /// Calls `AVPlayer.preroll(atRate:)` only when the player is ready; /// otherwise defers via a one-shot KVO on `status`. No-op while /// `player.rate != 0` (preroll is only useful when paused). diff --git a/mobile/packages/divine_video_player/lib/src/video_clip.dart b/mobile/packages/divine_video_player/lib/src/video_clip.dart index 107868960e..9f5896620f 100644 --- a/mobile/packages/divine_video_player/lib/src/video_clip.dart +++ b/mobile/packages/divine_video_player/lib/src/video_clip.dart @@ -138,11 +138,13 @@ class VideoClip { /// stretch where one track has already run out — silence, or a frozen last /// frame. On a looping player that stretch is the loop seam. /// - /// Set this for looping playback of a single clip. It shortens the clip by - /// the track mismatch (typically < 100 ms), so it is wrong for a clip in the - /// middle of a multi-clip timeline, where it would cut content rather than a - /// seam. Clamping only ever shortens: [end], when set, still wins if it is - /// earlier. + /// Set this for looping playback of a single clip. It shortens the clip only + /// when the track mismatch is small (currently at most 500 ms and at most + /// 10% of the playable duration), so obviously malformed assets keep their + /// container duration instead of collapsing into a tiny loop. It is wrong for + /// a clip in the middle of a multi-clip timeline, where it would cut content + /// rather than a seam. Clamping only ever shortens: [end], when set, still + /// wins if it is earlier. /// /// Support is per-platform: Apple always honours it, Android honours it for /// local files only (a remote source would need a blocking metadata read diff --git a/mobile/packages/divine_video_player/test/src/loop_seam_trim_contract_test.dart b/mobile/packages/divine_video_player/test/src/loop_seam_trim_contract_test.dart index 03043594a2..e93ddfe5c9 100644 --- a/mobile/packages/divine_video_player/test/src/loop_seam_trim_contract_test.dart +++ b/mobile/packages/divine_video_player/test/src/loop_seam_trim_contract_test.dart @@ -16,10 +16,10 @@ void main() { ); expect( source, - contains('CMTimeMinimum(videoRange.end, audioRange.end)'), + contains('boundedCommonTrackEnd('), reason: - 'The clip must end where both tracks still have content, not at ' - 'the asset duration, which is the longest track.', + 'The clip must end where both tracks still have content only when ' + 'the mismatch is small enough to be a seam, not a malformed asset.', ); expect( source, @@ -28,6 +28,27 @@ void main() { 'Clamping may only ever shorten a clip — an explicit trim that ' 'ends earlier still wins.', ); + expect( + source, + contains('catch {'), + reason: + 'A failure to read optional track ranges must leave the clip ' + 'untrimmed rather than failing composition playback.', + ); + expect( + source, + contains('maxCommonTrackEndTrimMs = 500.0'), + reason: + 'The clamp must be bounded so stub audio tracks do not collapse a ' + 'normal-length video into a tiny loop.', + ); + expect( + source, + contains('maxCommonTrackEndTrimRatio = 0.10'), + reason: + 'The clamp must also be relative to playable duration so short ' + 'clips cannot lose an excessive fraction of content.', + ); }); test('Android clamps the clipping configuration, not just endMs', () { @@ -35,7 +56,7 @@ void main() { expect( source, - contains('commonTrackEndMs'), + contains('boundedCommonTrackEndMs'), reason: 'Android must resolve the point where both tracks still have ' 'content before building the clipping configuration.', @@ -54,6 +75,20 @@ void main() { 'The clamped end must reach ExoPlayer; setting the raw endMs ' 'would leave the seam in place.', ); + expect( + source, + contains('MAX_COMMON_TRACK_END_TRIM_MS = 500L'), + reason: + 'The clamp must be bounded so stub audio tracks do not collapse a ' + 'normal-length video into a tiny loop.', + ); + expect( + source, + contains('MAX_COMMON_TRACK_END_TRIM_RATIO = 0.10'), + reason: + 'The clamp must also be relative to playable duration so short ' + 'clips cannot lose an excessive fraction of content.', + ); }); test('Android probes local files only', () {