SourceBufferPrivateAVFObjC should not report an error to the web page when the video playback is interrupted
<rdar://70118480>
Created attachment 428214 [details] Patch
Comment on attachment 428214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428214&action=review > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:971 > +#if PLATFORM(IOS_FAMILY) > + if ([layer status] == AVQueuedSampleBufferRenderingStatusFailed && [[error domain] isEqualToString:@"AVFoundationErrorDomain"] && [error code] == AVErrorOperationInterrupted) { > + m_displayLayerWasInterrupted = true; > + return; > + } > +#endif Why isn’t this needed on Mac? Why are we compiling all the code to handle m_displayLayerWasInterrupted on Mac, but then not using it there?
Comment on attachment 428214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428214&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:971 >> +#endif > > Why isn’t this needed on Mac? Why are we compiling all the code to handle m_displayLayerWasInterrupted on Mac, but then not using it there? The bug that this patch is fixing only happens on iPads. Based on my test results, AVFoundation only reports the error (AVErrorOperationInterrupted) on iPads. Therefore, I enabled this code section for iOS only. Potentially, other code sections related to "m_displayLayerWasInterrupted" are useful for Mac, so I did not wrap them with the macro. But it seems a good idea to compile them for iOS only because they won't be used on Mac for now.
Created attachment 428293 [details] Make all changes for iOS only
Comment on attachment 428293 [details] Make all changes for iOS only View in context: https://bugs.webkit.org/attachment.cgi?id=428293&action=review > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:1206 > +#if PLATFORM(IOS_FAMILY) > + if (m_displayLayerWasInterrupted) > + return false; > +#endif This will block appending samples to the audio renderer as well as the display layer. Consider moving this inside the `if (trackID == m_enabledVideoTrackID)` block below. > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:1260 > +#if PLATFORM(IOS_FAMILY) > + if (m_displayLayerWasInterrupted) > + return; > +#endif Ditto, and consider adding a call to `[m_displayLayer stopRequestingMediaData];` before the return.
Comment on attachment 428293 [details] Make all changes for iOS only View in context: https://bugs.webkit.org/attachment.cgi?id=428293&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:1206 >> +#endif > > This will block appending samples to the audio renderer as well as the display layer. Consider moving this inside the `if (trackID == m_enabledVideoTrackID)` block below. Good point! But looks like it won't impact the function, because we will want to stop appending audio samples when video is interrupted. I will fix it anyway. >> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:1260 >> +#endif > > Ditto, and consider adding a call to `[m_displayLayer stopRequestingMediaData];` before the return. Hmm, probably we can just remove this change. Let me have a try.
Created attachment 428421 [details] Patch for landing
Comment on attachment 428293 [details] Make all changes for iOS only View in context: https://bugs.webkit.org/attachment.cgi?id=428293&action=review >>> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:1260 >>> +#endif >> >> Ditto, and consider adding a call to `[m_displayLayer stopRequestingMediaData];` before the return. > > Hmm, probably we can just remove this change. Let me have a try. Confirmed that this change is not needed. Removed.
Committed r277424 (237672@main): <https://commits.webkit.org/237672@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428421 [details].