Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow MSE with mp4 should AVStreamDataParser be non-functional #8439

Conversation

jyavenard
Copy link
Member

@jyavenard jyavenard commented Jan 10, 2023

1f86058

Disallow MSE with mp4 should AVStreamDataParser be non-functional
https://bugs.webkit.org/show_bug.cgi?id=250367
rdar://104065947

Reviewed by Youenn Fablet.

On iPhone, MSE is disabled by default.
It can be enabled using an experimental setting, however it is mostly non-functional
as the SourceBufferParserAVFObjC relies on AVStreamDataParser class which itself is non-functional.
So testing for the availability of this class isn't sufficient,
we need to test that the init method itself doesn't return nil.

No test, as currently all MSE tests are disabled on iOS, and enabling those is out of scope for this change.i
On other platforms, covered by existing tests.

* Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferParserAVFObjC.mm:
(WebCore::SourceBufferParserAVFObjC::isContentTypeSupported):

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

8e5eecd

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk   πŸ›  wincairo
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2   πŸ§ͺ api-mac   πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1   πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@jyavenard jyavenard self-assigned this Jan 10, 2023
@jyavenard jyavenard added the Media Bugs related to the HTML 5 Media elements. label Jan 10, 2023
@@ -208,6 +208,13 @@ AtomString codec() const override

MediaPlayerEnums::SupportsType SourceBufferParserAVFObjC::isContentTypeSupported(const ContentType& type)
{
// Check that AVStreamDataParser is in a functional state.
if (!PAL::getAVStreamDataParserClass()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!PAL::getAVStreamDataParserClass()) {
if (PAL::getAVStreamDataParserClass()) {

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, I had actually fixed that but didn't commit it, again a last minute change trying to change the code flow :(

@jyavenard jyavenard force-pushed the eng/Disallow-MSE-with-mp4-should-AVStreamDataParser-be-non-functional branch from 17d1039 to 8e5eecd Compare January 10, 2023 11:13
@jyavenard jyavenard added the merge-queue Applied to send a pull request to merge-queue label Jan 10, 2023
https://bugs.webkit.org/show_bug.cgi?id=250367
rdar://104065947

Reviewed by Youenn Fablet.

On iPhone, MSE is disabled by default.
It can be enabled using an experimental setting, however it is mostly non-functional
as the SourceBufferParserAVFObjC relies on AVStreamDataParser class which itself is non-functional.
So testing for the availability of this class isn't sufficient,
we need to test that the init method itself doesn't return nil.

No test, as currently all MSE tests are disabled on iOS, and enabling those is out of scope for this change.i
On other platforms, covered by existing tests.

* Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferParserAVFObjC.mm:
(WebCore::SourceBufferParserAVFObjC::isContentTypeSupported):

Canonical link: https://commits.webkit.org/258716@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Disallow-MSE-with-mp4-should-AVStreamDataParser-be-non-functional branch from 8e5eecd to 1f86058 Compare January 10, 2023 13:08
@webkit-commit-queue
Copy link
Collaborator

Committed 258716@main (1f86058): https://commits.webkit.org/258716@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit 1f86058 into WebKit:main Jan 10, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 10, 2023
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
4 participants