Bug 219857 - [Media in GPU Process][MSE] WebM source buffer parser is not enabled
Summary: [Media in GPU Process][MSE] WebM source buffer parser is not enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Peng Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-12-14 09:29 PST by Peng Liu
Modified: 2020-12-14 15:05 PST (History)
13 users (show)

See Also:

Attachments
Patch (23.03 KB, patch)
2020-12-14 11:05 PST, Peng Liu 		eric.carlson: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Rebase the patch and update it based on Eric's comments (25.95 KB, patch)
2020-12-14 13:06 PST, Peng Liu 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Peng Liu 2020-12-14 09:29:47 PST 
Function isWebmParserAvailable() in SourceBufferParserWebM always returns false in GPU Process because it uses a RuntimeEnabledFeature flag.
Comment 1 Radar WebKit Bug Importer 2020-12-14 09:30:35 PST 
<rdar://problem/72300659>
Comment 2 Peng Liu 2020-12-14 11:05:48 PST 
Created attachment 416175 [details]
Patch
Comment 3 Eric Carlson 2020-12-14 11:27:12 PST 
Comment on attachment 416175 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416175&action=review

> Source/WebCore/platform/graphics/MediaSourcePrivate.h:57
> +    virtual AddStatus addSourceBuffer(const ContentType&, bool webMParserEnabled, RefPtr<SourceBufferPrivate>&) = 0;

Nit: the parameter name isn't necessary.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSourcePrivateAVFObjC.h:69
> +    AddStatus addSourceBuffer(const ContentType&, bool webMParserEnabled, RefPtr<SourceBufferPrivate>&) override;

Ditto.

> Source/WebCore/platform/graphics/cocoa/SourceBufferParser.h:49
> +    static RefPtr<SourceBufferParser> create(const ContentType&, bool webMParserEnabled);

Ditto

> Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp:67
> +MediaSourcePrivate::AddStatus MockMediaSourcePrivate::addSourceBuffer(const ContentType& contentType, bool webMParserEnabled, RefPtr<SourceBufferPrivate>& outPrivate)
>  {
> +    UNUSED_PARAM(webMParserEnabled);

Ditto, and you don't need the UNUSED_PARAM without it.

> Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.h:82
> +    AddStatus addSourceBuffer(const ContentType&, bool webMParserEnabled, RefPtr<SourceBufferPrivate>&) override;

Ditto.

> Source/WebKit/GPUProcess/media/RemoteMediaSourceProxy.h:60
> +    RemoteMediaSourceProxy(GPUConnectionToWebProcess&, RemoteMediaSourceIdentifier, bool webMParserEnabled, RemoteMediaPlayerProxy&);

Ditto.

> Source/WebKit/WebProcess/GPU/media/MediaSourcePrivateRemote.h:65
> +    AddStatus addSourceBuffer(const WebCore::ContentType&, bool webMParserEnabled, RefPtr<WebCore::SourceBufferPrivate>&) final;

Ditto.
Comment 4 Peng Liu 2020-12-14 13:06:51 PST 
Created attachment 416189 [details]
Rebase the patch and update it based on Eric's comments
Comment 5 EWS 2020-12-14 14:55:51 PST 
Committed r270805: <https://trac.webkit.org/changeset/270805>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416189 [details].
Comment 6 Peng Liu 2020-12-14 15:05:33 PST 
Comment on attachment 416175 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416175&action=review

>> Source/WebCore/platform/graphics/MediaSourcePrivate.h:57
>> +    virtual AddStatus addSourceBuffer(const ContentType&, bool webMParserEnabled, RefPtr<SourceBufferPrivate>&) = 0;
> 
> Nit: the parameter name isn't necessary.

Right, but I would like to keep it because a bool argument's purpose is not obvious in this case for a reader.

>> Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp:67
>> +    UNUSED_PARAM(webMParserEnabled);
> 
> Ditto, and you don't need the UNUSED_PARAM without it.

Right, fixed.