Skip to content

REGRESSION(290813@main) [Cocoa] Videos on bleacherreport.com are muted.#42860

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
jernoble:eng/REGRESSION-290813-main-Cocoa-Videos-on-bleacherreport-com-are-muted
Mar 25, 2025
Merged

REGRESSION(290813@main) [Cocoa] Videos on bleacherreport.com are muted.#42860
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
jernoble:eng/REGRESSION-290813-main-Cocoa-Videos-on-bleacherreport-com-are-muted

Conversation

@jernoble
Copy link
Contributor

@jernoble jernoble commented Mar 22, 2025

39c8c28

REGRESSION(290813@main) [Cocoa] Videos on bleacherreport.com are muted.
rdar://147271449
https://bugs.webkit.org/show_bug.cgi?id=290225

Reviewed by Eric Carlson.

In 290813@main, an AVAssetTrack was always added to AVTrackPrivateAVFObjCImpl when
that object was created with an AVMediaSelectionGroup/Option. But because a number
of the functions in AVTrackPrivateAVFObjCImpl check first for the presence of an
AVAssetTrack, this caused a change in behavior where those AudioTracks created with
an AVMediaSelectionGroup/Option no longer had a language.

Reverse the if() check to check for a mediaSelectionOption before an assetTrack.

* LayoutTests/http/tests/media/hls/hls-audio-tracks-language-expected.txt: Added.
* LayoutTests/http/tests/media/hls/hls-audio-tracks-language.html: Added.
* Source/WebCore/platform/graphics/avfoundation/AVTrackPrivateAVFObjCImpl.mm:
(WebCore::AVTrackPrivateAVFObjCImpl::audioKind const):
(WebCore::AVTrackPrivateAVFObjCImpl::videoKind const):
(WebCore::AVTrackPrivateAVFObjCImpl::textKind const):
(WebCore::AVTrackPrivateAVFObjCImpl::index const):
(WebCore::AVTrackPrivateAVFObjCImpl::id const):
(WebCore::AVTrackPrivateAVFObjCImpl::label const):
(WebCore::AVTrackPrivateAVFObjCImpl::language const):

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

5dfe1db

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@jernoble jernoble self-assigned this Mar 22, 2025
@jernoble jernoble added the Media Bugs related to the HTML 5 Media elements. label Mar 22, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 22, 2025
@jernoble jernoble removed the merging-blocked Applied to prevent a change from being merged label Mar 24, 2025
@jernoble jernoble force-pushed the eng/REGRESSION-290813-main-Cocoa-Videos-on-bleacherreport-com-are-muted branch from 20c4cf8 to b172313 Compare March 24, 2025 15:34
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 24, 2025
@jernoble jernoble removed the merging-blocked Applied to prevent a change from being merged label Mar 24, 2025
@jernoble jernoble force-pushed the eng/REGRESSION-290813-main-Cocoa-Videos-on-bleacherreport-com-are-muted branch from b172313 to 432ae45 Compare March 24, 2025 20:52
Copy link
Contributor

Choose a reason for hiding this comment

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

This will assert if a track or media selection option has an empty or unknown language

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I'll remove the assert.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 25, 2025
@jernoble jernoble removed the merging-blocked Applied to prevent a change from being merged label Mar 25, 2025
@jernoble jernoble force-pushed the eng/REGRESSION-290813-main-Cocoa-Videos-on-bleacherreport-com-are-muted branch from 432ae45 to 5dfe1db Compare March 25, 2025 02:53
@jernoble jernoble added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Mar 25, 2025
@webkit-ews-buildbot
Copy link
Collaborator

Failed mac-AS-debug-wk2, api-gtk checks. Please resolve failures and re-apply safe-merge-queue label.

Rejecting #42860 from merge queue.

@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Mar 25, 2025
@webkit-ews-buildbot
Copy link
Collaborator

Safe-Merge-Queue: Build #52489.

@jernoble jernoble added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Mar 25, 2025
rdar://147271449
https://bugs.webkit.org/show_bug.cgi?id=290225

Reviewed by Eric Carlson.

In 290813@main, an AVAssetTrack was always added to AVTrackPrivateAVFObjCImpl when
that object was created with an AVMediaSelectionGroup/Option. But because a number
of the functions in AVTrackPrivateAVFObjCImpl check first for the presence of an
AVAssetTrack, this caused a change in behavior where those AudioTracks created with
an AVMediaSelectionGroup/Option no longer had a language.

Reverse the if() check to check for a mediaSelectionOption before an assetTrack.

* LayoutTests/http/tests/media/hls/hls-audio-tracks-language-expected.txt: Added.
* LayoutTests/http/tests/media/hls/hls-audio-tracks-language.html: Added.
* Source/WebCore/platform/graphics/avfoundation/AVTrackPrivateAVFObjCImpl.mm:
(WebCore::AVTrackPrivateAVFObjCImpl::audioKind const):
(WebCore::AVTrackPrivateAVFObjCImpl::videoKind const):
(WebCore::AVTrackPrivateAVFObjCImpl::textKind const):
(WebCore::AVTrackPrivateAVFObjCImpl::index const):
(WebCore::AVTrackPrivateAVFObjCImpl::id const):
(WebCore::AVTrackPrivateAVFObjCImpl::label const):
(WebCore::AVTrackPrivateAVFObjCImpl::language const):

Canonical link: https://commits.webkit.org/292648@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/REGRESSION-290813-main-Cocoa-Videos-on-bleacherreport-com-are-muted branch from 5dfe1db to 39c8c28 Compare March 25, 2025 15:53
@webkit-commit-queue
Copy link
Collaborator

Committed 292648@main (39c8c28): https://commits.webkit.org/292648@main

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

@webkit-commit-queue webkit-commit-queue merged commit 39c8c28 into WebKit:main Mar 25, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 25, 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.

5 participants