Created attachment 431302 [details] Video showing poster disappearing prematurely Overview: In the latests versions of Safari on iOS the poster disappears before the video itself is ready to play. The background is then showing through for a while, causing a flash when starting videos. The video element should switch seamlessly from poster to video. Steps to reproduce: 1. Go to test page https://codepen.io/johannesodland/pen/XWMNvdW 2. Press play Actual results: Poster disappears prematurely once user presses play, leaving a transparent element with the background showing through. Video appears later once it has enough data to play. Expected results: Poster is visible until video is ready to play, switching seamlessly to video Build and hardware: Safari on iOS 14.6 iPhone 11 Pro
<rdar://problem/79315114>
The issue is still present in Safari 15.
Created attachment 454331 [details] Video of the issue on twitter This issue makes every video on twitter and other sites flash on play. Any hope you will look into it?
Created attachment 454615 [details] Patch
Created attachment 454626 [details] Patch
Comment on attachment 454626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454626&action=review r=me with nits: > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:543 > + bool hasAvailableVideoFrame = this->hasAvailableVideoFrame(); > + if (!m_haveReportedFirstVideoFrame && m_cachedHasVideo && hasAvailableVideoFrame) { > + haveFirstVideoFrameChanged = true; > + m_haveReportedFirstVideoFrame = true; > + } else if (!hasAvailableVideoFrame) > + m_haveReportedFirstVideoFrame = false; This section of code confused me. What about this instead?: if (!hasAvailableVideoFrame()) m_haveReportedFirstVideoFrame = false else if (!m_haveReportedFirstVideoFrame && m_cachedHasVideo) { m_haveReportedFirstVideoFrame = true; haveFirstVideoFrameChanged = true; } > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:561 > - if (m_readyState != MediaPlayer::ReadyState::HaveEnoughData && maxTimeLoaded() > currentMediaTime()) > + if (m_readyState != MediaPlayer::ReadyState::HaveEnoughData && (!m_cachedHasVideo || m_haveReportedFirstVideoFrame) && maxTimeLoaded() > currentMediaTime()) I wonder if we should have a `shouldBlockAdvancingToFutureData` variable (initialized to the same as above) here. > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:582 > - if (!m_haveReportedFirstVideoFrame && m_cachedHasVideo && hasAvailableVideoFrame()) { > + if (haveFirstVideoFrameChanged) { `haveFirstVideoFrameChanged` is slightly weird (because it's really saying the media player has a video frame available for the first time). Maybe `videoFrameBecomeAvailable`?
Comment on attachment 454626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454626&action=review >> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:543 >> + m_haveReportedFirstVideoFrame = false; > > This section of code confused me. What about this instead?: > > if (!hasAvailableVideoFrame()) > m_haveReportedFirstVideoFrame = false > else if (!m_haveReportedFirstVideoFrame && m_cachedHasVideo) { > m_haveReportedFirstVideoFrame = true; > haveFirstVideoFrameChanged = true; > } Good idea, changed. >> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:561 >> + if (m_readyState != MediaPlayer::ReadyState::HaveEnoughData && (!m_cachedHasVideo || m_haveReportedFirstVideoFrame) && maxTimeLoaded() > currentMediaTime()) > > I wonder if we should have a `shouldBlockAdvancingToFutureData` variable (initialized to the same as above) here. I think it makes more sense to have the logic here so I'll leave this. >> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:582 >> + if (haveFirstVideoFrameChanged) { > > `haveFirstVideoFrameChanged` is slightly weird (because it's really saying the media player has a video frame available for the first time). Maybe `videoFrameBecomeAvailable`? I changed it to `firstVideoFrameBecomeAvailable`.
Created attachment 454642 [details] Patch for landing
Created attachment 454710 [details] Patch for landing
Created attachment 454713 [details] Patch for landing
Created attachment 454720 [details] Patch for landing
Created attachment 454771 [details] Patch for landing
The Windows bot test failure, streams/readable-stream-lock-after-worker-terminates-crash.html, is unrelated.
Committed r291326 (248465@main): <https://commits.webkit.org/248465@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454771 [details].
Which Safari version should this patch be included in? Current Safari Technology Preview (Release 142 (Safari 15.4, WebKit 17614.1.5.16)) seems to not include those changes. At least codepen test from comment #0 still reproduces for me. (Asking, because I experience similar bug with my own page)
Created attachment 459526 [details] Video of the bug on Safari 15.5 Tested on Safari 15.5, and the bug is still there. Should we register a new issue, or should this issue be reopened?
See https://bugs.webkit.org/show_bug.cgi?id=234743