Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions mobile/lib/constants/app_constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ class AppConstants {
/// Minimum following videos needed before loading discovery feed
static const int followingVideoThreshold = 5;

// ============================================================================
// FEED PLAYBACK
// ============================================================================

/// Hard cap on how long any feed video plays before it loops.
///
/// Nothing in the feed plays longer than a Vine, including kind-34236 events
/// published by other Nostr clients that point at arbitrarily long files.
/// The cap is applied as a native clip end (`VideoClip.end`), so the loop
/// point stays inside the platform player — a Dart-side seek-to-zero is what
/// produced the audible loop seam in #5544.
///
/// Deliberately above `VideoEditorConstants.maxDuration` (6.3s, the
/// *recording* limit): classic Vine assets measure 6.500–6.533s, and capping
/// at the recording limit would clip their musical loop point (#6421).
static const Duration maxFeedPlaybackDuration = Duration(seconds: 7);

// ============================================================================
// VIDEO PROCESSING
// ============================================================================
Expand Down
10 changes: 7 additions & 3 deletions mobile/lib/widgets/video_feed_item/feed_videos.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import 'package:openvine/blocs/video_interactions/video_interactions_bloc.dart';
import 'package:openvine/blocs/video_playback_status/video_playback_status_cubit.dart';
import 'package:openvine/blocs/video_playback_status/video_playback_status_state.dart';
import 'package:openvine/blocs/video_volume/video_volume_cubit.dart';
import 'package:openvine/constants/app_constants.dart';
import 'package:openvine/extensions/video_event_extensions.dart';
import 'package:openvine/features/feature_flags/models/feature_flag.dart';
import 'package:openvine/features/feature_flags/providers/feature_flag_providers.dart';
Expand Down Expand Up @@ -364,9 +365,12 @@ class FeedVideosState extends ConsumerState<FeedVideos> with RouteAware {
_resumeAutoAdvanceAfterSwipe();
widget.onActiveVideoChanged?.call(video, index);
},
// Do not pass maxLoopDuration here. Feed playback should loop at the
// asset boundary; restarting with a Dart seek at the 6.3s recording
// limit creates an audible seam.
// Nothing in the feed plays longer than a Vine, not even a 60s file a
// foreign Nostr client points at. The cap is a native clip end, so the
// loop point stays in the platform player — deliberately NOT
// maxLoopDuration, whose Dart seek at the 6.3s recording limit both
// truncated classic Vines and created an audible seam (#5544, #6421).
maxPlaybackDuration: AppConstants.maxFeedPlaybackDuration,
onVideoLoopCompleted: _handleAutoAdvanceCompleted,
shouldPortraitExpand: widget.shouldPortraitExpand,
canAutoPlay: _canAutoPlayVideo,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,18 @@ final class DivineVideoPlayerInstance: NSObject, FlutterStreamHandler {
let startTime = CMTime(value: startMs, timescale: 1000)
let endTime: CMTime
if let endMs {
endTime = CMTime(value: endMs.int64Value, timescale: 1000)
// Clamp to the asset: insertTimeRange silently inserts only the
// media that exists, so an endMs past the source would leave
// clipDuration (and therefore the reported totalDuration)
// longer than what actually plays. The feed relies on this —
// it caps every clip at maxFeedPlaybackDuration without knowing
// the source length up front. ExoPlayer clamps the equivalent
// ClippingConfiguration itself.
let requestedEnd = CMTime(value: endMs.int64Value, timescale: 1000)
endTime =
(assetDuration.isNumeric && CMTimeCompare(requestedEnd, assetDuration) > 0)
? assetDuration
: requestedEnd
} else {
endTime = assetDuration
}
Expand Down
6 changes: 5 additions & 1 deletion mobile/packages/divine_video_player/lib/src/video_clip.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ class VideoClip {

/// End position within the source video.
///
/// When `null`, the clip plays to the end of the source.
/// When `null`, the clip plays to the end of the source. On the Android,
/// Apple and web backends an [end] past the source duration is clamped to
/// it, so a caller capping playback without knowing the source length still
/// gets the natural end for shorter sources. The Linux backend does not
/// clamp and reports the requested length.
final Duration? end;

/// Audio volume for this clip (0.0 = muted, 1.0 = full volume).
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import 'dart:math' as math;

/// Resolves the playback duration the web backend reports for a clip, in
/// seconds.
///
/// [sourceDurationSeconds] is the `<video>` element's own duration, which is
/// `NaN` until the browser has loaded metadata and `Infinity` for a live
/// stream.
///
/// A [clipEndSeconds] past the source is clamped to the source, matching the
/// Android and Apple backends, so a caller capping playback without knowing
/// the source length still reports the natural end for shorter sources.
/// Without the clamp a blind cap makes every shorter video report the cap as
/// its duration, which breaks any consumer that arms on `duration - epsilon`.
double resolveClipDurationSeconds({
required double clipStartSeconds,
required double? clipEndSeconds,
required double sourceDurationSeconds,
}) {
final hasSourceDuration =
!sourceDurationSeconds.isNaN && sourceDurationSeconds.isFinite;

final double? endSeconds;
if (clipEndSeconds == null) {
endSeconds = hasSourceDuration ? sourceDurationSeconds : null;
} else {
endSeconds = hasSourceDuration
? math.min(clipEndSeconds, sourceDurationSeconds)
: clipEndSeconds;
}
if (endSeconds == null) return 0;

return math.max(0, endSeconds - clipStartSeconds);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'dart:ui_web' as ui_web;
import 'package:divine_video_player/src/audio_track.dart' as divine;
import 'package:divine_video_player/src/video_clip.dart';
import 'package:divine_video_player/src/video_player_state.dart';
import 'package:divine_video_player/src/web/web_clip_duration.dart';
import 'package:divine_video_player/src/web/web_video_player_backend.dart';
import 'package:flutter/widgets.dart';
import 'package:unified_logger/unified_logger.dart';
Expand Down Expand Up @@ -419,17 +420,14 @@ class HtmlVideoElementBackend implements WebVideoPlayerBackend {
final position = Duration(
microseconds: math.max(0, (positionSeconds * 1e6).round()),
);
final rawDurationSeconds = element.duration;
final hasDuration =
!rawDurationSeconds.isNaN && rawDurationSeconds.isFinite;
final clipEnd = _clipEnd;
final effectiveDurationSeconds = clipEnd != null
? _toSeconds(clipEnd) - _toSeconds(_clipStart)
: hasDuration
? rawDurationSeconds - _toSeconds(_clipStart)
: 0.0;
final effectiveDurationSeconds = resolveClipDurationSeconds(
clipStartSeconds: _toSeconds(_clipStart),
clipEndSeconds: clipEnd == null ? null : _toSeconds(clipEnd),
sourceDurationSeconds: element.duration,
);
final duration = Duration(
microseconds: math.max(0, (effectiveDurationSeconds * 1e6).round()),
microseconds: (effectiveDurationSeconds * 1e6).round(),
);

var bufferedPosition = position;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import 'dart:io';

import 'package:flutter_test/flutter_test.dart';

/// `insertTimeRange` silently inserts only the media that exists, so an
/// out-of-range clip end never truncates playback — but it does inflate the
/// `clipDuration` the Swift side derives from the *requested* end, and that
/// value becomes the reported `totalDuration`. Callers that cap playback
/// without knowing the source length (the feed's `maxFeedPlaybackDuration`)
/// depend on the clamp: without it every shorter video reports the cap as its
/// duration, corrupting the progress fraction and the loop-completion timing
/// that arms on `duration - endThreshold`.
///
/// ExoPlayer clamps the equivalent `ClippingConfiguration` inside
/// `ClippingTimeline`, so the guarantee documented on `VideoClip.end` holds on
/// Android without repo code. On Apple it is this branch and nothing else, and
/// it has no Dart runtime surface — the package's CI runs Dart and Kotlin
/// tests only, so a refactor that drops it keeps every other test green.
void main() {
group('Apple native clip-end clamp contract', () {
test('clamps a requested clip end to the asset duration', () {
final source = _appleSourceFile().readAsStringSync();

expect(
source,
contains('CMTimeCompare(requestedEnd, assetDuration) > 0'),
reason:
'A requested end past the asset must resolve to the asset '
'duration, not to the requested value.',
);
expect(
source,
contains('assetDuration.isNumeric'),
reason:
'A non-numeric asset duration (indefinite/unloaded) cannot be '
'compared against, so the requested end has to stand there.',
);
});

test('clamps before deriving the clip duration', () {
final source = _appleSourceFile().readAsStringSync();

final clamp = source.indexOf(
'CMTimeCompare(requestedEnd, assetDuration) > 0',
);
final clipDuration = source.indexOf(
'CMTimeSubtract(endTime, startTime)',
);

expect(clamp, greaterThanOrEqualTo(0));
expect(clipDuration, greaterThanOrEqualTo(0));
expect(
clamp,
lessThan(clipDuration),
reason:
'clipDuration feeds the reported totalDuration, so it must be '
'derived from the clamped end rather than the requested one.',
);
});
});
}

/// The iOS and macOS players share a single Darwin source tree
/// (`darwin/divine_video_player/Sources/`), so the clamp contract is asserted
/// once.
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',
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import 'package:divine_video_player/src/web/web_clip_duration.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
group('resolveClipDurationSeconds', () {
test('reports the source length when no clip end is set', () {
expect(
resolveClipDurationSeconds(
clipStartSeconds: 0,
clipEndSeconds: null,
sourceDurationSeconds: 3.2,
),
3.2,
);
});

test('clamps a clip end past the source to the source length', () {
expect(
resolveClipDurationSeconds(
clipStartSeconds: 0,
clipEndSeconds: 7,
sourceDurationSeconds: 3.2,
),
3.2,
reason:
'The feed caps every clip blindly; without the clamp a 3.2s video '
'reports the 7s cap and no consumer arming on duration - epsilon '
'ever fires.',
);
});

test('keeps a clip end inside the source', () {
expect(
resolveClipDurationSeconds(
clipStartSeconds: 0,
clipEndSeconds: 7,
sourceDurationSeconds: 60,
),
7,
);
});

test('subtracts the clip start from the resolved end', () {
expect(
resolveClipDurationSeconds(
clipStartSeconds: 1,
clipEndSeconds: 3,
sourceDurationSeconds: 60,
),
2,
);
expect(
resolveClipDurationSeconds(
clipStartSeconds: 1,
clipEndSeconds: null,
sourceDurationSeconds: 3,
),
2,
);
});

test('uses the requested end while the source duration is unknown', () {
expect(
resolveClipDurationSeconds(
clipStartSeconds: 0,
clipEndSeconds: 7,
sourceDurationSeconds: double.nan,
),
7,
reason:
'element.duration is NaN until metadata loads; reporting zero '
'there would flap the duration on every refresh before playback.',
);
});

test('uses the requested end for a live source', () {
expect(
resolveClipDurationSeconds(
clipStartSeconds: 0,
clipEndSeconds: 7,
sourceDurationSeconds: double.infinity,
),
7,
);
});

test('reports zero when neither a clip end nor a source length exists', () {
expect(
resolveClipDurationSeconds(
clipStartSeconds: 0,
clipEndSeconds: null,
sourceDurationSeconds: double.nan,
),
0,
);
});

test('never reports a negative duration for a start past the end', () {
expect(
resolveClipDurationSeconds(
clipStartSeconds: 5,
clipEndSeconds: 2,
sourceDurationSeconds: 60,
),
0,
);
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,18 @@ class SourceLoadAborted implements Exception {
/// Returns a record of `(source, attemptIndex)` for the URL that opened.
/// Logs each failure via [log] and re-throws the last error when every
/// source fails.
///
/// [maxPlaybackDuration] becomes the clip's end position, so the native
/// player stops (and loops) there. Sources shorter than the cap are
/// unaffected — both backends clamp the clip end to the real duration.
Future<(String, int)> setSourceWithFallbacks({
required int index,
required DivineVideoPlayerController controller,
required List<String> sources,
required void Function(String) log,
Map<String, String>? Function(String source)? httpHeadersForSource,
bool Function()? isLoadCurrent,
Duration? maxPlaybackDuration,
SourceLoadDelay delay = Future<void>.delayed,
}) async {
Object? lastError;
Expand All @@ -59,6 +64,7 @@ Future<(String, int)> setSourceWithFallbacks({
await controller.setSource(
VideoClip.network(
source,
end: maxPlaybackDuration,
httpHeaders: httpHeadersForSource?.call(source) ?? const {},
),
);
Expand Down Expand Up @@ -101,6 +107,7 @@ Future<(String, int)> setSourceWithFallbacks({
await controller.setSource(
VideoClip.network(
source,
end: maxPlaybackDuration,
httpHeaders: httpHeadersForSource?.call(source) ?? const {},
),
);
Expand Down
Loading
Loading