From 0bade5984ebcde21365dba8e091fd34ba9ee39a7 Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Wed, 21 Aug 2024 18:11:15 +0200 Subject: [PATCH 01/11] split video_player changes from more_fps --- .../video_player_avfoundation/CHANGELOG.md | 4 +- .../darwin/RunnerTests/VideoPlayerTests.m | 105 ++++++++++++++++-- .../FVPVideoPlayerPlugin.m | 54 +++++---- .../video_player_avfoundation/pubspec.yaml | 2 +- 4 files changed, 130 insertions(+), 35 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/CHANGELOG.md b/packages/video_player/video_player_avfoundation/CHANGELOG.md index e51f69fd613..7ad372b3603 100644 --- a/packages/video_player/video_player_avfoundation/CHANGELOG.md +++ b/packages/video_player/video_player_avfoundation/CHANGELOG.md @@ -1,5 +1,7 @@ -## NEXT +## 2.6.2 +* Adds possibility to play videos at more than 30 FPS. +* Fixes playing state not updating in some paths. * Updates minimum supported SDK version to Flutter 3.19/Dart 3.3. ## 2.6.1 diff --git a/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m b/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m index 3ec96e78538..eafbe99f55e 100644 --- a/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m +++ b/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m @@ -125,6 +125,7 @@ @interface StubFVPDisplayLinkFactory : NSObject /** This display link to return. */ @property(nonatomic, strong) FVPDisplayLink *displayLink; +@property(nonatomic) void (^fireDisplayLink)(void); - (instancetype)initWithDisplayLink:(FVPDisplayLink *)displayLink; @@ -138,6 +139,7 @@ - (instancetype)initWithDisplayLink:(FVPDisplayLink *)displayLink { } - (FVPDisplayLink *)displayLinkWithRegistrar:(id)registrar callback:(void (^)(void))callback { + self.fireDisplayLink = callback; return self.displayLink; } @@ -243,13 +245,14 @@ - (void)testSeekToWhilePausedStartsDisplayLinkTemporarily { OCMStub([mockVideoOutput hasNewPixelBufferForItemTime:kCMTimeZero]) .ignoringNonObjectArgs() .andReturn(YES); - // Any non-zero value is fine here since it won't actually be used, just NULL-checked. - CVPixelBufferRef fakeBufferRef = (CVPixelBufferRef)1; + CVPixelBufferRef bufferRef; + CVPixelBufferCreate(NULL, 1, 1, kCVPixelFormatType_32BGRA, NULL, &bufferRef); OCMStub([mockVideoOutput copyPixelBufferForItemTime:kCMTimeZero itemTimeForDisplay:NULL]) .ignoringNonObjectArgs() - .andReturn(fakeBufferRef); + .andReturn(bufferRef); // Simulate a callback from the engine to request a new frame. - [player copyPixelBuffer]; + stubDisplayLinkFactory.fireDisplayLink(); + CFRelease([player copyPixelBuffer]); // Since a frame was found, and the video is paused, the display link should be paused again. OCMVerify([mockDisplayLink setRunning:NO]); } @@ -294,14 +297,15 @@ - (void)testInitStartsDisplayLinkTemporarily { OCMStub([mockVideoOutput hasNewPixelBufferForItemTime:kCMTimeZero]) .ignoringNonObjectArgs() .andReturn(YES); - // Any non-zero value is fine here since it won't actually be used, just NULL-checked. - CVPixelBufferRef fakeBufferRef = (CVPixelBufferRef)1; + CVPixelBufferRef bufferRef; + CVPixelBufferCreate(NULL, 1, 1, kCVPixelFormatType_32BGRA, NULL, &bufferRef); OCMStub([mockVideoOutput copyPixelBufferForItemTime:kCMTimeZero itemTimeForDisplay:NULL]) .ignoringNonObjectArgs() - .andReturn(fakeBufferRef); + .andReturn(bufferRef); // Simulate a callback from the engine to request a new frame. FVPVideoPlayer *player = videoPlayerPlugin.playersByTextureId[textureId]; - [player copyPixelBuffer]; + stubDisplayLinkFactory.fireDisplayLink(); + CFRelease([player copyPixelBuffer]); // Since a frame was found, and the video is paused, the display link should be paused again. OCMVerify([mockDisplayLink setRunning:NO]); } @@ -357,13 +361,14 @@ - (void)testSeekToWhilePlayingDoesNotStopDisplayLink { OCMStub([mockVideoOutput hasNewPixelBufferForItemTime:kCMTimeZero]) .ignoringNonObjectArgs() .andReturn(YES); - // Any non-zero value is fine here since it won't actually be used, just NULL-checked. - CVPixelBufferRef fakeBufferRef = (CVPixelBufferRef)1; + CVPixelBufferRef bufferRef; + CVPixelBufferCreate(NULL, 1, 1, kCVPixelFormatType_32BGRA, NULL, &bufferRef); OCMStub([mockVideoOutput copyPixelBufferForItemTime:kCMTimeZero itemTimeForDisplay:NULL]) .ignoringNonObjectArgs() - .andReturn(fakeBufferRef); + .andReturn(bufferRef); // Simulate a callback from the engine to request a new frame. - [player copyPixelBuffer]; + stubDisplayLinkFactory.fireDisplayLink(); + CFRelease([player copyPixelBuffer]); // Since the video was playing, the display link should not be paused after getting a buffer. OCMVerify(never(), [mockDisplayLink setRunning:NO]); } @@ -790,6 +795,82 @@ - (void)testPublishesInRegistration { XCTAssertTrue([publishedValue isKindOfClass:[FVPVideoPlayerPlugin class]]); } +- (void)testPlayerShouldNotDropEverySecondFrame { + NSObject *registrar = + [GetPluginRegistry() registrarForPlugin:@"testPlayerShouldNotDropEverySecondFrame"]; + NSObject *partialRegistrar = OCMPartialMock(registrar); + NSObject *mockTextureRegistry = + OCMProtocolMock(@protocol(FlutterTextureRegistry)); + OCMStub([partialRegistrar textures]).andReturn(mockTextureRegistry); + + FVPDisplayLink *displayLink = [[FVPDisplayLink alloc] initWithRegistrar:registrar + callback:^(){ + }]; + StubFVPDisplayLinkFactory *stubDisplayLinkFactory = + [[StubFVPDisplayLinkFactory alloc] initWithDisplayLink:displayLink]; + AVPlayerItemVideoOutput *mockVideoOutput = OCMPartialMock([[AVPlayerItemVideoOutput alloc] init]); + FVPVideoPlayerPlugin *videoPlayerPlugin = [[FVPVideoPlayerPlugin alloc] + initWithAVFactory:[[StubFVPAVFactory alloc] initWithPlayer:nil output:mockVideoOutput] + displayLinkFactory:stubDisplayLinkFactory + registrar:partialRegistrar]; + + FlutterError *error; + [videoPlayerPlugin initialize:&error]; + XCTAssertNil(error); + FVPCreationOptions *create = [FVPCreationOptions + makeWithAsset:nil + uri:@"https://flutter.github.io/assets-for-api-docs/assets/videos/bee.mp4" + packageName:nil + formatHint:nil + httpHeaders:@{}]; + NSNumber *textureId = [videoPlayerPlugin createWithOptions:create error:&error]; + FVPVideoPlayer *player = videoPlayerPlugin.playersByTextureId[textureId]; + + __block CMTime currentTime = kCMTimeZero; + OCMStub([mockVideoOutput itemTimeForHostTime:0]) + .ignoringNonObjectArgs() + .andDo(^(NSInvocation *invocation) { + [invocation setReturnValue:¤tTime]; + }); + __block NSMutableSet *pixelBuffers = NSMutableSet.new; + OCMStub([mockVideoOutput hasNewPixelBufferForItemTime:kCMTimeZero]) + .ignoringNonObjectArgs() + .andDo(^(NSInvocation *invocation) { + CMTime itemTime; + [invocation getArgument:&itemTime atIndex:2]; + BOOL has = [pixelBuffers containsObject:[NSValue valueWithCMTime:itemTime]]; + [invocation setReturnValue:&has]; + }); + OCMStub([mockVideoOutput copyPixelBufferForItemTime:kCMTimeZero + itemTimeForDisplay:[OCMArg anyPointer]]) + .ignoringNonObjectArgs() + .andDo(^(NSInvocation *invocation) { + CMTime itemTime; + [invocation getArgument:&itemTime atIndex:2]; + CVPixelBufferRef bufferRef = NULL; + if ([pixelBuffers containsObject:[NSValue valueWithCMTime:itemTime]]) { + CVPixelBufferCreate(NULL, 1, 1, kCVPixelFormatType_32BGRA, NULL, &bufferRef); + } + [pixelBuffers removeObject:[NSValue valueWithCMTime:itemTime]]; + [invocation setReturnValue:&bufferRef]; + }); + void (^advanceFrame)(void) = ^{ + currentTime.value++; + [pixelBuffers addObject:[NSValue valueWithCMTime:currentTime]]; + }; + + advanceFrame(); + OCMExpect([mockTextureRegistry textureFrameAvailable:textureId.intValue]); + stubDisplayLinkFactory.fireDisplayLink(); + OCMVerifyAllWithDelay(mockTextureRegistry, 10); + + advanceFrame(); + OCMExpect([mockTextureRegistry textureFrameAvailable:textureId.intValue]); + CFRelease([player copyPixelBuffer]); + stubDisplayLinkFactory.fireDisplayLink(); + OCMVerifyAllWithDelay(mockTextureRegistry, 10); +} + #if TARGET_OS_IOS - (void)validateTransformFixForOrientation:(UIImageOrientation)orientation { AVAssetTrack *track = [[FakeAVAssetTrack alloc] initWithOrientation:orientation]; diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index 14ee7ccefde..33fd5bef05a 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -21,8 +21,8 @@ @interface FVPFrameUpdater : NSObject @property(nonatomic, weak, readonly) NSObject *registry; // The output that this updater is managing. @property(nonatomic, weak) AVPlayerItemVideoOutput *videoOutput; -// The last time that has been validated as avaliable according to hasNewPixelBufferForItemTime:. -@property(nonatomic, assign) CMTime lastKnownAvailableTime; +@property(nonatomic) CVPixelBufferRef latestPixelBuffer; +@property(nonatomic) dispatch_queue_t pixelBufferSynchronizationQueue; @end @implementation FVPFrameUpdater @@ -30,7 +30,6 @@ - (FVPFrameUpdater *)initWithRegistry:(NSObject *)regist NSAssert(self, @"super init cannot be nil"); if (self == nil) return nil; _registry = registry; - _lastKnownAvailableTime = kCMTimeInvalid; return self; } @@ -38,10 +37,22 @@ - (void)displayLinkFired { // Only report a new frame if one is actually available. CMTime outputItemTime = [self.videoOutput itemTimeForHostTime:CACurrentMediaTime()]; if ([self.videoOutput hasNewPixelBufferForItemTime:outputItemTime]) { - _lastKnownAvailableTime = outputItemTime; + dispatch_async(self.pixelBufferSynchronizationQueue, ^{ + if (self.latestPixelBuffer) { + CFRelease(self.latestPixelBuffer); + } + self.latestPixelBuffer = [self.videoOutput copyPixelBufferForItemTime:outputItemTime + itemTimeForDisplay:NULL]; + }); [_registry textureFrameAvailable:_textureId]; } } + +- (void)dealloc { + if (_latestPixelBuffer) { + CFRelease(_latestPixelBuffer); + } +} @end @interface FVPDefaultAVFactory : NSObject @@ -92,6 +103,7 @@ @interface FVPVideoPlayer () // (e.g., after a seek while paused). If YES, the display link should continue to run until the next // frame is successfully provided. @property(nonatomic, assign) BOOL waitingForFrame; +@property(nonatomic) dispatch_queue_t pixelBufferSynchronizationQueue; - (instancetype)initWithURL:(NSURL *)url frameUpdater:(FVPFrameUpdater *)frameUpdater @@ -234,9 +246,8 @@ - (AVMutableVideoComposition *)getVideoCompositionWithTransform:(CGAffineTransfo } videoComposition.renderSize = CGSizeMake(width, height); - // TODO(@recastrodiaz): should we use videoTrack.nominalFrameRate ? - // Currently set at a constant 30 FPS - videoComposition.frameDuration = CMTimeMake(1, 30); + videoComposition.sourceTrackIDForFrameTiming = videoTrack.trackID; + videoComposition.frameDuration = videoTrack.minFrameDuration; return videoComposition; } @@ -283,6 +294,10 @@ - (instancetype)initWithPlayerItem:(AVPlayerItem *)item error:nil] == AVKeyValueStatusLoaded) { // Rotate the video by using a videoComposition and the preferredTransform self->_preferredTransform = FVPGetStandardizedTransformForTrack(videoTrack); + // do not use video composition when it is not needed + if (CGAffineTransformIsIdentity(self->_preferredTransform)) { + return; + } // Note: // https://developer.apple.com/documentation/avfoundation/avplayeritem/1388818-videocomposition // Video composition can only be used with file-based media and is not supported for @@ -320,6 +335,10 @@ - (instancetype)initWithPlayerItem:(AVPlayerItem *)item _videoOutput = [avFactory videoOutputWithPixelBufferAttributes:pixBuffAttributes]; frameUpdater.videoOutput = _videoOutput; + _pixelBufferSynchronizationQueue = + dispatch_queue_create("io.flutter.video_player.pixelBufferSynchronizationQueue", NULL); + frameUpdater.pixelBufferSynchronizationQueue = _pixelBufferSynchronizationQueue; + [self addObserversForItem:item player:_player]; [asset loadValuesAsynchronouslyForKeys:@[ @"tracks" ] completionHandler:assetCompletionHandler]; @@ -358,7 +377,6 @@ - (void)observeValueForKeyPath:(NSString *)path case AVPlayerItemStatusReadyToPlay: [item addOutput:_videoOutput]; [self setupEventSinkIfReadyToPlay]; - [self updatePlayingState]; break; } } else if (context == presentationSizeContext || context == durationContext) { @@ -368,7 +386,6 @@ - (void)observeValueForKeyPath:(NSString *)path // its presentation size or duration. When these properties are finally set, re-check if // all required properties and instantiate the event sink if it is not already set up. [self setupEventSinkIfReadyToPlay]; - [self updatePlayingState]; } } else if (context == playbackLikelyToKeepUpContext) { [self updatePlayingState]; @@ -447,6 +464,8 @@ - (void)setupEventSinkIfReadyToPlay { } _isInitialized = YES; + [self updatePlayingState]; + _eventSink(@{ @"event" : @"initialized", @"duration" : @(duration), @@ -543,18 +562,11 @@ - (void)setPlaybackSpeed:(double)speed { } - (CVPixelBufferRef)copyPixelBuffer { - CVPixelBufferRef buffer = NULL; - CMTime outputItemTime = [_videoOutput itemTimeForHostTime:CACurrentMediaTime()]; - if ([_videoOutput hasNewPixelBufferForItemTime:outputItemTime]) { - buffer = [_videoOutput copyPixelBufferForItemTime:outputItemTime itemTimeForDisplay:NULL]; - } else { - // If the current time isn't available yet, use the time that was checked when informing the - // engine that a frame was available (if any). - CMTime lastAvailableTime = self.frameUpdater.lastKnownAvailableTime; - if (CMTIME_IS_VALID(lastAvailableTime)) { - buffer = [_videoOutput copyPixelBufferForItemTime:lastAvailableTime itemTimeForDisplay:NULL]; - } - } + __block CVPixelBufferRef buffer = NULL; + dispatch_sync(self.pixelBufferSynchronizationQueue, ^{ + buffer = self.frameUpdater.latestPixelBuffer; + self.frameUpdater.latestPixelBuffer = NULL; + }); if (self.waitingForFrame && buffer) { self.waitingForFrame = NO; diff --git a/packages/video_player/video_player_avfoundation/pubspec.yaml b/packages/video_player/video_player_avfoundation/pubspec.yaml index dad297f4020..63952af7c43 100644 --- a/packages/video_player/video_player_avfoundation/pubspec.yaml +++ b/packages/video_player/video_player_avfoundation/pubspec.yaml @@ -2,7 +2,7 @@ name: video_player_avfoundation description: iOS and macOS implementation of the video_player plugin. repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_avfoundation issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 -version: 2.6.1 +version: 2.6.2 environment: sdk: ^3.3.0 From ad0ba7d5d76624e685dbe73980ef452b3e9f364f Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Fri, 20 Sep 2024 18:13:36 +0200 Subject: [PATCH 02/11] rework few details --- .../FVPVideoPlayerPlugin.m | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index 33fd5bef05a..d2918f2c9eb 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -21,7 +21,9 @@ @interface FVPFrameUpdater : NSObject @property(nonatomic, weak, readonly) NSObject *registry; // The output that this updater is managing. @property(nonatomic, weak) AVPlayerItemVideoOutput *videoOutput; +// Latest copied pixel buffer ready to display. @property(nonatomic) CVPixelBufferRef latestPixelBuffer; +// Synchronization queue for sending and receiving pixel buffers. @property(nonatomic) dispatch_queue_t pixelBufferSynchronizationQueue; @end @@ -30,6 +32,9 @@ - (FVPFrameUpdater *)initWithRegistry:(NSObject *)regist NSAssert(self, @"super init cannot be nil"); if (self == nil) return nil; _registry = registry; + dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class(NULL, QOS_CLASS_USER_INTERACTIVE, -1); + _pixelBufferSynchronizationQueue = + dispatch_queue_create("io.flutter.video_player.pixelBufferSynchronizationQueue", attributes); return self; } @@ -37,21 +42,28 @@ - (void)displayLinkFired { // Only report a new frame if one is actually available. CMTime outputItemTime = [self.videoOutput itemTimeForHostTime:CACurrentMediaTime()]; if ([self.videoOutput hasNewPixelBufferForItemTime:outputItemTime]) { - dispatch_async(self.pixelBufferSynchronizationQueue, ^{ - if (self.latestPixelBuffer) { - CFRelease(self.latestPixelBuffer); - } - self.latestPixelBuffer = [self.videoOutput copyPixelBufferForItemTime:outputItemTime - itemTimeForDisplay:NULL]; + CVPixelBufferRef pixelBuffer = [self.videoOutput copyPixelBufferForItemTime:outputItemTime + itemTimeForDisplay:NULL]; + dispatch_sync(self.pixelBufferSynchronizationQueue, ^{ + CVBufferRelease(self.latestPixelBuffer); + self.latestPixelBuffer = pixelBuffer; }); [_registry textureFrameAvailable:_textureId]; } } +- (CVPixelBufferRef)acquirePixelBuffer +{ + __block CVPixelBufferRef pixelBuffer; + dispatch_sync(self.pixelBufferSynchronizationQueue, ^{ + pixelBuffer = self.latestPixelBuffer; + self.latestPixelBuffer = NULL; + }); + return pixelBuffer; +} + - (void)dealloc { - if (_latestPixelBuffer) { - CFRelease(_latestPixelBuffer); - } + CVBufferRelease(_latestPixelBuffer); } @end @@ -103,7 +115,6 @@ @interface FVPVideoPlayer () // (e.g., after a seek while paused). If YES, the display link should continue to run until the next // frame is successfully provided. @property(nonatomic, assign) BOOL waitingForFrame; -@property(nonatomic) dispatch_queue_t pixelBufferSynchronizationQueue; - (instancetype)initWithURL:(NSURL *)url frameUpdater:(FVPFrameUpdater *)frameUpdater @@ -247,7 +258,12 @@ - (AVMutableVideoComposition *)getVideoCompositionWithTransform:(CGAffineTransfo videoComposition.renderSize = CGSizeMake(width, height); videoComposition.sourceTrackIDForFrameTiming = videoTrack.trackID; - videoComposition.frameDuration = videoTrack.minFrameDuration; + if (CMTIME_IS_VALID(videoTrack.minFrameDuration)) { + videoComposition.frameDuration = videoTrack.minFrameDuration; + } else { + NSLog(@"Warning: videoTrack.minFrameDuration for input video is invalid, please report this."); + videoComposition.frameDuration = CMTimeMake(1, 120); + } return videoComposition; } @@ -294,7 +310,7 @@ - (instancetype)initWithPlayerItem:(AVPlayerItem *)item error:nil] == AVKeyValueStatusLoaded) { // Rotate the video by using a videoComposition and the preferredTransform self->_preferredTransform = FVPGetStandardizedTransformForTrack(videoTrack); - // do not use video composition when it is not needed + // Do not use video composition when it is not needed. if (CGAffineTransformIsIdentity(self->_preferredTransform)) { return; } @@ -335,10 +351,6 @@ - (instancetype)initWithPlayerItem:(AVPlayerItem *)item _videoOutput = [avFactory videoOutputWithPixelBufferAttributes:pixBuffAttributes]; frameUpdater.videoOutput = _videoOutput; - _pixelBufferSynchronizationQueue = - dispatch_queue_create("io.flutter.video_player.pixelBufferSynchronizationQueue", NULL); - frameUpdater.pixelBufferSynchronizationQueue = _pixelBufferSynchronizationQueue; - [self addObserversForItem:item player:_player]; [asset loadValuesAsynchronouslyForKeys:@[ @"tracks" ] completionHandler:assetCompletionHandler]; @@ -562,11 +574,7 @@ - (void)setPlaybackSpeed:(double)speed { } - (CVPixelBufferRef)copyPixelBuffer { - __block CVPixelBufferRef buffer = NULL; - dispatch_sync(self.pixelBufferSynchronizationQueue, ^{ - buffer = self.frameUpdater.latestPixelBuffer; - self.frameUpdater.latestPixelBuffer = NULL; - }); + CVPixelBufferRef buffer = self.frameUpdater.acquirePixelBuffer; if (self.waitingForFrame && buffer) { self.waitingForFrame = NO; From 02a124101cec6cdd9814001e681fbb263ea4ba74 Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Fri, 11 Oct 2024 18:06:00 +0200 Subject: [PATCH 03/11] avoid race between displayLinkFired and copyPixelBuffer --- .../FVPVideoPlayerPlugin.m | 84 ++++++++++++------- .../FVPDisplayLink.h | 3 + .../FVPDisplayLink.m | 4 + .../FVPDisplayLink.m | 8 ++ 4 files changed, 67 insertions(+), 32 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index d2918f2c9eb..17f244ad47a 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -5,6 +5,7 @@ #import "FVPVideoPlayerPlugin.h" #import "FVPVideoPlayerPlugin_Test.h" +#import #import #import @@ -21,10 +22,10 @@ @interface FVPFrameUpdater : NSObject @property(nonatomic, weak, readonly) NSObject *registry; // The output that this updater is managing. @property(nonatomic, weak) AVPlayerItemVideoOutput *videoOutput; -// Latest copied pixel buffer ready to display. -@property(nonatomic) CVPixelBufferRef latestPixelBuffer; -// Synchronization queue for sending and receiving pixel buffers. -@property(nonatomic) dispatch_queue_t pixelBufferSynchronizationQueue; +// The display link that drives frameUpdater. +@property(nonatomic) FVPDisplayLink *displayLink; +// The time interval between screen refresh updates. +@property(nonatomic) _Atomic CFTimeInterval duration; @end @implementation FVPFrameUpdater @@ -32,38 +33,18 @@ - (FVPFrameUpdater *)initWithRegistry:(NSObject *)regist NSAssert(self, @"super init cannot be nil"); if (self == nil) return nil; _registry = registry; - dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class(NULL, QOS_CLASS_USER_INTERACTIVE, -1); - _pixelBufferSynchronizationQueue = - dispatch_queue_create("io.flutter.video_player.pixelBufferSynchronizationQueue", attributes); return self; } - (void)displayLinkFired { - // Only report a new frame if one is actually available. - CMTime outputItemTime = [self.videoOutput itemTimeForHostTime:CACurrentMediaTime()]; - if ([self.videoOutput hasNewPixelBufferForItemTime:outputItemTime]) { - CVPixelBufferRef pixelBuffer = [self.videoOutput copyPixelBufferForItemTime:outputItemTime - itemTimeForDisplay:NULL]; - dispatch_sync(self.pixelBufferSynchronizationQueue, ^{ - CVBufferRelease(self.latestPixelBuffer); - self.latestPixelBuffer = pixelBuffer; - }); - [_registry textureFrameAvailable:_textureId]; - } -} - -- (CVPixelBufferRef)acquirePixelBuffer -{ - __block CVPixelBufferRef pixelBuffer; - dispatch_sync(self.pixelBufferSynchronizationQueue, ^{ - pixelBuffer = self.latestPixelBuffer; - self.latestPixelBuffer = NULL; - }); - return pixelBuffer; + // Display link duration is in an undefined state until displayLinkFired is called at least once + // so it should not be used directly. + atomic_store_explicit(&_duration, _displayLink.duration, memory_order_relaxed); + [_registry textureFrameAvailable:_textureId]; } -- (void)dealloc { - CVBufferRelease(_latestPixelBuffer); +- (CFTimeInterval)duration { + return atomic_load_explicit(&_duration, memory_order_relaxed); } @end @@ -111,6 +92,10 @@ @interface FVPVideoPlayer () @property(nonatomic) FVPFrameUpdater *frameUpdater; // The display link that drives frameUpdater. @property(nonatomic) FVPDisplayLink *displayLink; +// Latest copied pixel buffer. +@property(nonatomic) CVPixelBufferRef latestPixelBuffer; +// The time that represents when the next frame displays. +@property(nonatomic) CFTimeInterval targetTime; // Whether a new frame needs to be provided to the engine regardless of the current play/pause state // (e.g., after a seek while paused). If YES, the display link should continue to run until the next // frame is successfully provided. @@ -158,6 +143,7 @@ - (instancetype)initWithAsset:(NSString *)asset } - (void)dealloc { + CVBufferRelease(_latestPixelBuffer); if (!_disposed) { [self removeKeyValueObservers]; } @@ -350,6 +336,7 @@ - (instancetype)initWithPlayerItem:(AVPlayerItem *)item }; _videoOutput = [avFactory videoOutputWithPixelBufferAttributes:pixBuffAttributes]; frameUpdater.videoOutput = _videoOutput; + frameUpdater.displayLink = _displayLink; [self addObserversForItem:item player:_player]; @@ -574,7 +561,28 @@ - (void)setPlaybackSpeed:(double)speed { } - (CVPixelBufferRef)copyPixelBuffer { - CVPixelBufferRef buffer = self.frameUpdater.acquirePixelBuffer; + // Ensure video sampling at regular intervals. This function is not called at exact time intervals + // so CACurrentMediaTime returns irregular timestamps which causes missed video frames. The range + // outside which targetTime is reset should be narrow enough to make possible lag as small as + // possible and at the same time wide enough to avoid too frequent resets which would lead to + // irregular sampling. Ideally there would be a targetTimestamp of display link used by flutter + // engine (FlutterTexture can provide timestamp and duration or timestamp and targetTimestamp). + CFTimeInterval currentTime = CACurrentMediaTime(); + if (fabs(self.targetTime - currentTime) > self.frameUpdater.duration / 2) { + self.targetTime = currentTime; + } + self.targetTime += self.frameUpdater.duration; + + CVPixelBufferRef buffer = NULL; + CMTime outputItemTime = [self.videoOutput itemTimeForHostTime:self.targetTime]; + if ([self.videoOutput hasNewPixelBufferForItemTime:outputItemTime]) { + buffer = [self.videoOutput copyPixelBufferForItemTime:outputItemTime + itemTimeForDisplay:NULL]; + if (buffer) { + CVBufferRelease(self.latestPixelBuffer); + self.latestPixelBuffer = buffer; + } + } if (self.waitingForFrame && buffer) { self.waitingForFrame = NO; @@ -585,7 +593,19 @@ - (CVPixelBufferRef)copyPixelBuffer { } } - return buffer; + // Calling textureFrameAvailable only from within displayLinkFired would require a non-trivial + // solution to minimize missed video frames due to race between displayLinkFired, copyPixelBuffer + // and place where is _textureFrameAvailable reset to false in the flutter engine. Ideally + // FlutterTexture would support mode of operation where the copyPixelBuffer is called always, + // maybe with possibility to start and stop it, instead of on demand by calling textureFrameAvailable. + if (self.displayLink.running) { + dispatch_async(dispatch_get_main_queue(), ^{ + [self.frameUpdater.registry textureFrameAvailable:self.frameUpdater.textureId]; + }); + } + + // Better to avoid returning NULL as it is unspecified what should be displayed in such a case. + return CVBufferRetain(self.latestPixelBuffer); } - (void)onTextureUnregistered:(NSObject *)texture { diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/include/video_player_avfoundation/FVPDisplayLink.h b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/include/video_player_avfoundation/FVPDisplayLink.h index 80d400629b2..54e338993c4 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/include/video_player_avfoundation/FVPDisplayLink.h +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/include/video_player_avfoundation/FVPDisplayLink.h @@ -18,6 +18,9 @@ /// Defaults to NO. @property(nonatomic, assign) BOOL running; +/// The time interval between screen refresh updates. +@property(nonatomic, readonly) CFTimeInterval duration; + /// Initializes a display link that calls the given callback when fired. /// /// The display link starts paused, so must be started, by setting 'running' to YES, before the diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_ios/FVPDisplayLink.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_ios/FVPDisplayLink.m index 9bdb321ae16..5ea6ea0135d 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_ios/FVPDisplayLink.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_ios/FVPDisplayLink.m @@ -67,4 +67,8 @@ - (void)setRunning:(BOOL)running { self.displayLink.paused = !running; } +- (CFTimeInterval)duration { + return self.displayLink.duration; +} + @end diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m index cd5670fa5a3..02148d5eee7 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m @@ -81,4 +81,12 @@ - (void)setRunning:(BOOL)running { } } +- (CFTimeInterval)duration { + CVTimeStamp timestamp = {0}; + if (CVDisplayLinkGetCurrentTime(self.displayLink, ×tamp) != kCVReturnSuccess) { + return 0; + } + return 1.0 * timestamp.videoRefreshPeriod / timestamp.videoTimeScale; +} + @end From e0f85ce5211835b0394889e63eda74693b8d97ce Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Fri, 11 Oct 2024 20:16:35 +0200 Subject: [PATCH 04/11] update warning message and format --- .../video_player_avfoundation/FVPVideoPlayerPlugin.m | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index 17f244ad47a..7d745a99f9d 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -5,9 +5,9 @@ #import "FVPVideoPlayerPlugin.h" #import "FVPVideoPlayerPlugin_Test.h" -#import #import #import +#import #import "./include/video_player_avfoundation/AVAssetTrackUtils.h" #import "./include/video_player_avfoundation/FVPDisplayLink.h" @@ -247,7 +247,7 @@ - (AVMutableVideoComposition *)getVideoCompositionWithTransform:(CGAffineTransfo if (CMTIME_IS_VALID(videoTrack.minFrameDuration)) { videoComposition.frameDuration = videoTrack.minFrameDuration; } else { - NSLog(@"Warning: videoTrack.minFrameDuration for input video is invalid, please report this."); + NSLog(@"Warning: videoTrack.minFrameDuration for input video is invalid, please report this to https://github.com/flutter/flutter/issues with input video attached."); videoComposition.frameDuration = CMTimeMake(1, 120); } @@ -576,8 +576,7 @@ - (CVPixelBufferRef)copyPixelBuffer { CVPixelBufferRef buffer = NULL; CMTime outputItemTime = [self.videoOutput itemTimeForHostTime:self.targetTime]; if ([self.videoOutput hasNewPixelBufferForItemTime:outputItemTime]) { - buffer = [self.videoOutput copyPixelBufferForItemTime:outputItemTime - itemTimeForDisplay:NULL]; + buffer = [self.videoOutput copyPixelBufferForItemTime:outputItemTime itemTimeForDisplay:NULL]; if (buffer) { CVBufferRelease(self.latestPixelBuffer); self.latestPixelBuffer = buffer; @@ -597,7 +596,8 @@ - (CVPixelBufferRef)copyPixelBuffer { // solution to minimize missed video frames due to race between displayLinkFired, copyPixelBuffer // and place where is _textureFrameAvailable reset to false in the flutter engine. Ideally // FlutterTexture would support mode of operation where the copyPixelBuffer is called always, - // maybe with possibility to start and stop it, instead of on demand by calling textureFrameAvailable. + // maybe with possibility to start and stop it, instead of on demand by calling + // textureFrameAvailable. if (self.displayLink.running) { dispatch_async(dispatch_get_main_queue(), ^{ [self.frameUpdater.registry textureFrameAvailable:self.frameUpdater.textureId]; From 89b051757e63c5b6a564700a6879b200e677e0f9 Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Fri, 11 Oct 2024 20:26:57 +0200 Subject: [PATCH 05/11] fix bad merge and format --- packages/video_player/video_player_avfoundation/CHANGELOG.md | 5 ++++- .../Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m | 3 ++- packages/video_player/video_player_avfoundation/pubspec.yaml | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/CHANGELOG.md b/packages/video_player/video_player_avfoundation/CHANGELOG.md index 8194010f161..f2478710e23 100644 --- a/packages/video_player/video_player_avfoundation/CHANGELOG.md +++ b/packages/video_player/video_player_avfoundation/CHANGELOG.md @@ -1,7 +1,10 @@ -## 2.6.2 +## 2.6.3 * Adds possibility to play videos at more than 30 FPS. * Fixes playing state not updating in some paths. + +## 2.6.2 + * Updates Pigeon for non-nullable collection type support. * Updates minimum supported SDK version to Flutter 3.19/Dart 3.3. diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index 7d745a99f9d..633b8a5026a 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -247,7 +247,8 @@ - (AVMutableVideoComposition *)getVideoCompositionWithTransform:(CGAffineTransfo if (CMTIME_IS_VALID(videoTrack.minFrameDuration)) { videoComposition.frameDuration = videoTrack.minFrameDuration; } else { - NSLog(@"Warning: videoTrack.minFrameDuration for input video is invalid, please report this to https://github.com/flutter/flutter/issues with input video attached."); + NSLog(@"Warning: videoTrack.minFrameDuration for input video is invalid, please report this to " + @"https://github.com/flutter/flutter/issues with input video attached."); videoComposition.frameDuration = CMTimeMake(1, 120); } diff --git a/packages/video_player/video_player_avfoundation/pubspec.yaml b/packages/video_player/video_player_avfoundation/pubspec.yaml index d309c02258d..31d615dd886 100644 --- a/packages/video_player/video_player_avfoundation/pubspec.yaml +++ b/packages/video_player/video_player_avfoundation/pubspec.yaml @@ -2,7 +2,7 @@ name: video_player_avfoundation description: iOS and macOS implementation of the video_player plugin. repository: https://github.com/flutter/packages/tree/main/packages/video_player/video_player_avfoundation issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22 -version: 2.6.2 +version: 2.6.3 environment: sdk: ^3.3.0 From 24002603a885533831cd343624aab5df2639e933 Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Fri, 11 Oct 2024 20:42:55 +0200 Subject: [PATCH 06/11] fix comments --- .../video_player_avfoundation/FVPVideoPlayerPlugin.m | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index 633b8a5026a..8ad0dd26304 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -564,7 +564,7 @@ - (void)setPlaybackSpeed:(double)speed { - (CVPixelBufferRef)copyPixelBuffer { // Ensure video sampling at regular intervals. This function is not called at exact time intervals // so CACurrentMediaTime returns irregular timestamps which causes missed video frames. The range - // outside which targetTime is reset should be narrow enough to make possible lag as small as + // outside of which targetTime is reset should be narrow enough to make possible lag as small as // possible and at the same time wide enough to avoid too frequent resets which would lead to // irregular sampling. Ideally there would be a targetTimestamp of display link used by flutter // engine (FlutterTexture can provide timestamp and duration or timestamp and targetTimestamp). @@ -596,9 +596,8 @@ - (CVPixelBufferRef)copyPixelBuffer { // Calling textureFrameAvailable only from within displayLinkFired would require a non-trivial // solution to minimize missed video frames due to race between displayLinkFired, copyPixelBuffer // and place where is _textureFrameAvailable reset to false in the flutter engine. Ideally - // FlutterTexture would support mode of operation where the copyPixelBuffer is called always, - // maybe with possibility to start and stop it, instead of on demand by calling - // textureFrameAvailable. + // FlutterTexture would support mode of operation where the copyPixelBuffer is called always or + // some other alternative, instead of on demand by calling textureFrameAvailable. if (self.displayLink.running) { dispatch_async(dispatch_get_main_queue(), ^{ [self.frameUpdater.registry textureFrameAvailable:self.frameUpdater.textureId]; From aae683d8a7a3433554e2cd7a8e747616a78bdbde Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:48:13 +0200 Subject: [PATCH 07/11] explicitly initialize what is needed --- .../Sources/video_player_avfoundation_macos/FVPDisplayLink.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m index 02148d5eee7..fb356a1ee96 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m @@ -82,7 +82,7 @@ - (void)setRunning:(BOOL)running { } - (CFTimeInterval)duration { - CVTimeStamp timestamp = {0}; + CVTimeStamp timestamp = {.version = 0}; if (CVDisplayLinkGetCurrentTime(self.displayLink, ×tamp) != kCVReturnSuccess) { return 0; } From 016f799e258728e9078862655f01a5e4891cf4de Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Sun, 2 Feb 2025 20:07:20 +0100 Subject: [PATCH 08/11] add logic to turn off self refresh --- .../darwin/RunnerTests/VideoPlayerTests.m | 2 +- .../FVPVideoPlayerPlugin.m | 73 ++++++++++++++----- .../FVPDisplayLink.m | 2 +- 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m b/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m index eafbe99f55e..34a69ae93ef 100644 --- a/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m +++ b/packages/video_player/video_player_avfoundation/darwin/RunnerTests/VideoPlayerTests.m @@ -125,7 +125,7 @@ @interface StubFVPDisplayLinkFactory : NSObject /** This display link to return. */ @property(nonatomic, strong) FVPDisplayLink *displayLink; -@property(nonatomic) void (^fireDisplayLink)(void); +@property(nonatomic, copy) void (^fireDisplayLink)(void); - (instancetype)initWithDisplayLink:(FVPDisplayLink *)displayLink; diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index 8ad0dd26304..90e6e3281d0 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -24,8 +24,8 @@ @interface FVPFrameUpdater : NSObject @property(nonatomic, weak) AVPlayerItemVideoOutput *videoOutput; // The display link that drives frameUpdater. @property(nonatomic) FVPDisplayLink *displayLink; -// The time interval between screen refresh updates. -@property(nonatomic) _Atomic CFTimeInterval duration; +// The time interval between screen refresh updates. Display link duration is in an undefined state until displayLinkFired is called at least once so it should not be used directly. +@property(atomic) CFTimeInterval frameDuration; @end @implementation FVPFrameUpdater @@ -37,15 +37,9 @@ - (FVPFrameUpdater *)initWithRegistry:(NSObject *)regist } - (void)displayLinkFired { - // Display link duration is in an undefined state until displayLinkFired is called at least once - // so it should not be used directly. - atomic_store_explicit(&_duration, _displayLink.duration, memory_order_relaxed); + self.frameDuration = _displayLink.duration; [_registry textureFrameAvailable:_textureId]; } - -- (CFTimeInterval)duration { - return atomic_load_explicit(&_duration, memory_order_relaxed); -} @end @interface FVPDefaultAVFactory : NSObject @@ -92,10 +86,18 @@ @interface FVPVideoPlayer () @property(nonatomic) FVPFrameUpdater *frameUpdater; // The display link that drives frameUpdater. @property(nonatomic) FVPDisplayLink *displayLink; -// Latest copied pixel buffer. +// The latest buffer obtained from video output. This is stored so that it can be returned from copyPixelBuffer again if nothing new is available, since the engine has undefined behavior when returning NULL. @property(nonatomic) CVPixelBufferRef latestPixelBuffer; // The time that represents when the next frame displays. @property(nonatomic) CFTimeInterval targetTime; +// Whether to enqueue textureFrameAvailable from copyPixelBuffer. +@property(nonatomic) BOOL selfRefresh; +// The time that represents the start of average frame duration measurement. +@property(nonatomic) CFTimeInterval startTime; +// The number of frames since the start of average frame duration measurement. +@property(nonatomic) int framesCount; +// The latest frame duration since there was significant change. +@property(nonatomic) CFTimeInterval latestDuration; // Whether a new frame needs to be provided to the engine regardless of the current play/pause state // (e.g., after a seek while paused). If YES, the display link should continue to run until the next // frame is successfully provided. @@ -249,7 +251,7 @@ - (AVMutableVideoComposition *)getVideoCompositionWithTransform:(CGAffineTransfo } else { NSLog(@"Warning: videoTrack.minFrameDuration for input video is invalid, please report this to " @"https://github.com/flutter/flutter/issues with input video attached."); - videoComposition.frameDuration = CMTimeMake(1, 120); + videoComposition.frameDuration = CMTimeMake(1, 30); } return videoComposition; @@ -338,6 +340,7 @@ - (instancetype)initWithPlayerItem:(AVPlayerItem *)item _videoOutput = [avFactory videoOutputWithPixelBufferAttributes:pixBuffAttributes]; frameUpdater.videoOutput = _videoOutput; frameUpdater.displayLink = _displayLink; + _selfRefresh = true; [self addObserversForItem:item player:_player]; @@ -562,23 +565,29 @@ - (void)setPlaybackSpeed:(double)speed { } - (CVPixelBufferRef)copyPixelBuffer { + // If the difference between target time and current time is longer than this fraction of frame duration then reset target time. + const float resetThreshold = 0.5; + // Ensure video sampling at regular intervals. This function is not called at exact time intervals // so CACurrentMediaTime returns irregular timestamps which causes missed video frames. The range // outside of which targetTime is reset should be narrow enough to make possible lag as small as // possible and at the same time wide enough to avoid too frequent resets which would lead to - // irregular sampling. Ideally there would be a targetTimestamp of display link used by flutter - // engine (FlutterTexture can provide timestamp and duration or timestamp and targetTimestamp). + // irregular sampling. + // TODO: Ideally there would be a targetTimestamp of display link used by the flutter engine. + // https://github.com/flutter/flutter/issues/159087 CFTimeInterval currentTime = CACurrentMediaTime(); - if (fabs(self.targetTime - currentTime) > self.frameUpdater.duration / 2) { + CFTimeInterval duration = self.frameUpdater.frameDuration; + if (fabs(self.targetTime - currentTime) > duration * resetThreshold) { self.targetTime = currentTime; } - self.targetTime += self.frameUpdater.duration; + self.targetTime += duration; CVPixelBufferRef buffer = NULL; CMTime outputItemTime = [self.videoOutput itemTimeForHostTime:self.targetTime]; if ([self.videoOutput hasNewPixelBufferForItemTime:outputItemTime]) { buffer = [self.videoOutput copyPixelBufferForItemTime:outputItemTime itemTimeForDisplay:NULL]; if (buffer) { + // Balance the owned reference from copyPixelBufferForItemTime. CVBufferRelease(self.latestPixelBuffer); self.latestPixelBuffer = buffer; } @@ -595,16 +604,40 @@ - (CVPixelBufferRef)copyPixelBuffer { // Calling textureFrameAvailable only from within displayLinkFired would require a non-trivial // solution to minimize missed video frames due to race between displayLinkFired, copyPixelBuffer - // and place where is _textureFrameAvailable reset to false in the flutter engine. Ideally - // FlutterTexture would support mode of operation where the copyPixelBuffer is called always or - // some other alternative, instead of on demand by calling textureFrameAvailable. - if (self.displayLink.running) { + // and place where is _textureFrameAvailable reset to false in the flutter engine. + // TODO: Ideally FlutterTexture would support mode of operation where the copyPixelBuffer is called always or some other alternative, instead of on demand by calling textureFrameAvailable. + // https://github.com/flutter/flutter/issues/159162 + if (self.displayLink.running && self.selfRefresh) { + // The number of frames over which to measure average frame duration. + const int windowSize = 10; + // If duration changes by this fraction or more then reset average frame duration measurement. + const float resetFraction = 0.01; + // If measured average frame duration is shorter than this fraction of frame duration obtained from display link then rely solely on refreshes from display link. + const float durationThreshold = 0.5; + + if (fabs(duration - self.latestDuration) >= self.latestDuration * resetFraction) { + self.startTime = currentTime; + self.framesCount = 0; + self.latestDuration = duration; + } + if (self.framesCount == windowSize) { + CFTimeInterval averageDuration = (currentTime - self.startTime) / windowSize; + if (averageDuration < duration * durationThreshold) { + NSLog(@"Warning: measured average duration between frames is unexpectedly short (%f/%f), please report this to " + @"https://github.com/flutter/flutter/issues.", averageDuration, duration); + self.selfRefresh = false; + } + self.startTime = currentTime; + self.framesCount = 0; + } + self.framesCount++; + dispatch_async(dispatch_get_main_queue(), ^{ [self.frameUpdater.registry textureFrameAvailable:self.frameUpdater.textureId]; }); } - // Better to avoid returning NULL as it is unspecified what should be displayed in such a case. + // Add a retain for the engine, since the copyPixelBufferForItemTime has already been accounted for, and the engine expects an owning reference. return CVBufferRetain(self.latestPixelBuffer); } diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m index fb356a1ee96..cb9682e6011 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation_macos/FVPDisplayLink.m @@ -86,7 +86,7 @@ - (CFTimeInterval)duration { if (CVDisplayLinkGetCurrentTime(self.displayLink, ×tamp) != kCVReturnSuccess) { return 0; } - return 1.0 * timestamp.videoRefreshPeriod / timestamp.videoTimeScale; + return (CFTimeInterval)timestamp.videoRefreshPeriod / timestamp.videoTimeScale; } @end From 8fb95cb3b9545bf4f4b846bcf0f5a26c63beef22 Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Sun, 2 Feb 2025 20:18:14 +0100 Subject: [PATCH 09/11] fix format --- .../FVPVideoPlayerPlugin.m | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index 90e6e3281d0..cc8e7beaa41 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -7,7 +7,6 @@ #import #import -#import #import "./include/video_player_avfoundation/AVAssetTrackUtils.h" #import "./include/video_player_avfoundation/FVPDisplayLink.h" @@ -24,7 +23,8 @@ @interface FVPFrameUpdater : NSObject @property(nonatomic, weak) AVPlayerItemVideoOutput *videoOutput; // The display link that drives frameUpdater. @property(nonatomic) FVPDisplayLink *displayLink; -// The time interval between screen refresh updates. Display link duration is in an undefined state until displayLinkFired is called at least once so it should not be used directly. +// The time interval between screen refresh updates. Display link duration is in an undefined state +// until displayLinkFired is called at least once so it should not be used directly. @property(atomic) CFTimeInterval frameDuration; @end @@ -86,7 +86,9 @@ @interface FVPVideoPlayer () @property(nonatomic) FVPFrameUpdater *frameUpdater; // The display link that drives frameUpdater. @property(nonatomic) FVPDisplayLink *displayLink; -// The latest buffer obtained from video output. This is stored so that it can be returned from copyPixelBuffer again if nothing new is available, since the engine has undefined behavior when returning NULL. +// The latest buffer obtained from video output. This is stored so that it can be returned from +// copyPixelBuffer again if nothing new is available, since the engine has undefined behavior when +// returning NULL. @property(nonatomic) CVPixelBufferRef latestPixelBuffer; // The time that represents when the next frame displays. @property(nonatomic) CFTimeInterval targetTime; @@ -565,7 +567,8 @@ - (void)setPlaybackSpeed:(double)speed { } - (CVPixelBufferRef)copyPixelBuffer { - // If the difference between target time and current time is longer than this fraction of frame duration then reset target time. + // If the difference between target time and current time is longer than this fraction of frame + // duration then reset target time. const float resetThreshold = 0.5; // Ensure video sampling at regular intervals. This function is not called at exact time intervals @@ -605,14 +608,16 @@ - (CVPixelBufferRef)copyPixelBuffer { // Calling textureFrameAvailable only from within displayLinkFired would require a non-trivial // solution to minimize missed video frames due to race between displayLinkFired, copyPixelBuffer // and place where is _textureFrameAvailable reset to false in the flutter engine. - // TODO: Ideally FlutterTexture would support mode of operation where the copyPixelBuffer is called always or some other alternative, instead of on demand by calling textureFrameAvailable. + // TODO: Ideally FlutterTexture would support mode of operation where the copyPixelBuffer is + // called always or some other alternative, instead of on demand by calling textureFrameAvailable. // https://github.com/flutter/flutter/issues/159162 if (self.displayLink.running && self.selfRefresh) { // The number of frames over which to measure average frame duration. const int windowSize = 10; // If duration changes by this fraction or more then reset average frame duration measurement. const float resetFraction = 0.01; - // If measured average frame duration is shorter than this fraction of frame duration obtained from display link then rely solely on refreshes from display link. + // If measured average frame duration is shorter than this fraction of frame duration obtained + // from display link then rely solely on refreshes from display link. const float durationThreshold = 0.5; if (fabs(duration - self.latestDuration) >= self.latestDuration * resetFraction) { @@ -623,8 +628,10 @@ - (CVPixelBufferRef)copyPixelBuffer { if (self.framesCount == windowSize) { CFTimeInterval averageDuration = (currentTime - self.startTime) / windowSize; if (averageDuration < duration * durationThreshold) { - NSLog(@"Warning: measured average duration between frames is unexpectedly short (%f/%f), please report this to " - @"https://github.com/flutter/flutter/issues.", averageDuration, duration); + NSLog(@"Warning: measured average duration between frames is unexpectedly short (%f/%f), " + "please report this to " + @"https://github.com/flutter/flutter/issues.", + averageDuration, duration); self.selfRefresh = false; } self.startTime = currentTime; @@ -637,7 +644,8 @@ - (CVPixelBufferRef)copyPixelBuffer { }); } - // Add a retain for the engine, since the copyPixelBufferForItemTime has already been accounted for, and the engine expects an owning reference. + // Add a retain for the engine, since the copyPixelBufferForItemTime has already been accounted + // for, and the engine expects an owning reference. return CVBufferRetain(self.latestPixelBuffer); } From 836733c2773c3ae554254ab761b9a768f6865ba0 Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Sun, 2 Feb 2025 20:45:42 +0100 Subject: [PATCH 10/11] fix format --- .../Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index cc8e7beaa41..38793d2a79c 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -629,7 +629,7 @@ - (CVPixelBufferRef)copyPixelBuffer { CFTimeInterval averageDuration = (currentTime - self.startTime) / windowSize; if (averageDuration < duration * durationThreshold) { NSLog(@"Warning: measured average duration between frames is unexpectedly short (%f/%f), " - "please report this to " + @"please report this to " @"https://github.com/flutter/flutter/issues.", averageDuration, duration); self.selfRefresh = false; From e304b16fd76e9b28fc709f0923c05622c594889a Mon Sep 17 00:00:00 2001 From: misos1 <30872003+misos1@users.noreply.github.com> Date: Tue, 4 Feb 2025 17:22:03 +0100 Subject: [PATCH 11/11] change order of consts --- .../Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m index 38793d2a79c..f13a1b09b0e 100644 --- a/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m +++ b/packages/video_player/video_player_avfoundation/darwin/video_player_avfoundation/Sources/video_player_avfoundation/FVPVideoPlayerPlugin.m @@ -614,11 +614,11 @@ - (CVPixelBufferRef)copyPixelBuffer { if (self.displayLink.running && self.selfRefresh) { // The number of frames over which to measure average frame duration. const int windowSize = 10; - // If duration changes by this fraction or more then reset average frame duration measurement. - const float resetFraction = 0.01; // If measured average frame duration is shorter than this fraction of frame duration obtained // from display link then rely solely on refreshes from display link. const float durationThreshold = 0.5; + // If duration changes by this fraction or more then reset average frame duration measurement. + const float resetFraction = 0.01; if (fabs(duration - self.latestDuration) >= self.latestDuration * resetFraction) { self.startTime = currentTime;