Skip to content

HTML audio element currentTime property too high when setting srcObject to MediaStream (macOS and iOS)#38633

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
youennf:eng/HTML-audio-element-currentTime-property-too-high-when-setting-srcObject-to-MediaStream-macOS-and-iOS
Jan 7, 2025
Merged

HTML audio element currentTime property too high when setting srcObject to MediaStream (macOS and iOS)#38633
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
youennf:eng/HTML-audio-element-currentTime-property-too-high-when-setting-srcObject-to-MediaStream-macOS-and-iOS

Conversation

@youennf
Copy link
Copy Markdown
Contributor

@youennf youennf commented Jan 7, 2025

8194ca4

HTML audio element currentTime property too high when setting srcObject to MediaStream (macOS and iOS)
https://bugs.webkit.org/show_bug.cgi?id=285391
rdar://142465970

Reviewed by Jean-Yves Avenard.

We were not initializing m_startTime to an invalid value since MediaPlayerPrivateMediaStreamAVFObjC::play is checking for whether it is valid or not.
We initialize m_startTime to an invalid value so that it can be correctly initialized in MediaPlayerPrivateMediaStreamAVFObjC::play.

Covered by updated test.

* LayoutTests/fast/mediastream/media-element-current-time.html:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaStreamAVFObjC::MediaPlayerPrivateMediaStreamAVFObjC):
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer):

Canonical link: https://commits.webkit.org/288519@main

0769bcf

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@youennf youennf self-assigned this Jan 7, 2025
@youennf youennf added the Media Bugs related to the HTML 5 Media elements. label Jan 7, 2025
Copy link
Copy Markdown
Member

@jyavenard jyavenard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with comment addressed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
, m_startTime(0, 0, 0)
, m_startTime(MediaTime::invalidTime())

@youennf youennf force-pushed the eng/HTML-audio-element-currentTime-property-too-high-when-setting-srcObject-to-MediaStream-macOS-and-iOS branch from 7d5f92e to 0769bcf Compare January 7, 2025 10:21
@youennf youennf requested a review from philn as a code owner January 7, 2025 10:21
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 7, 2025
@youennf youennf added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Jan 7, 2025
…ct to MediaStream (macOS and iOS)

https://bugs.webkit.org/show_bug.cgi?id=285391
rdar://142465970

Reviewed by Jean-Yves Avenard.

We were not initializing m_startTime to an invalid value since MediaPlayerPrivateMediaStreamAVFObjC::play is checking for whether it is valid or not.
We initialize m_startTime to an invalid value so that it can be correctly initialized in MediaPlayerPrivateMediaStreamAVFObjC::play.

Covered by updated test.

* LayoutTests/fast/mediastream/media-element-current-time.html:
* Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaStreamAVFObjC::MediaPlayerPrivateMediaStreamAVFObjC):
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::MediaPlayerPrivateGStreamer):

Canonical link: https://commits.webkit.org/288519@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/HTML-audio-element-currentTime-property-too-high-when-setting-srcObject-to-MediaStream-macOS-and-iOS branch from 0769bcf to 8194ca4 Compare January 7, 2025 12:59
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 288519@main (8194ca4): https://commits.webkit.org/288519@main

Reviewed commits have been landed. Closing PR #38633 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 8194ca4 into WebKit:main Jan 7, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Media Bugs related to the HTML 5 Media elements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants