Bug 207622

<rdar://problem/59427591>

I can reproduce this on macOS Mojave with STP 99. I cannot reproduce with shipping Safari/WebKit, so this looks like a regression. I cannot reproduce on macOS Catalina.

MediaSource.isTypeSupported("video/mp4;codecs=\"avc3.42C015\"")

Created attachment 390889 [details]
Patch

Comment on attachment 390889 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/AVAssetMIMETypeCache.mm:35
>  #import "ContentType.h"
> +#import <pal/spi/mac/AVFoundationSPI.h>
> +
>  #import <pal/cf/CoreMediaSoftLink.h>
>  #import <pal/cocoa/AVFoundationSoftLink.h>

These should be sorted in a single block.

> Source/WebCore/platform/graphics/avfoundation/objc/AVAssetMIMETypeCache.mm:71
> +    outputCodecs = [PAL::getAVStreamDataParserClass() outputMIMECodecParameterForInputMIMECodecParameter:outputCodecs];

This needs to be indented.

Comment on attachment 390889 [details]
Patch

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

>> Source/WebCore/platform/graphics/avfoundation/objc/AVAssetMIMETypeCache.mm:71
>> +    if ([PAL::getAVStreamDataParserClass() respondsToSelector:@selector(outputMIMECodecParameterForInputMIMECodecParameter:)])
>> +    outputCodecs = [PAL::getAVStreamDataParserClass() outputMIMECodecParameterForInputMIMECodecParameter:outputCodecs];
> 
> This needs to be indented.

We don't want to change the behavior of video.canPlayType, so I would put this into MediaPlayerPrivateMediaSourceAVFObjC::supportsType.

Comment on attachment 390889 [details]
Patch

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

>> Source/WebCore/platform/graphics/avfoundation/objc/AVAssetMIMETypeCache.mm:35
>>  #import <pal/cocoa/AVFoundationSoftLink.h>
> 
> These should be sorted in a single block.

Soft-link headers need to be included after other headers. The style checking errors will go away after we land the patch for http://webkit.org/b/207834.

>>> Source/WebCore/platform/graphics/avfoundation/objc/AVAssetMIMETypeCache.mm:71
>>> +    outputCodecs = [PAL::getAVStreamDataParserClass() outputMIMECodecParameterForInputMIMECodecParameter:outputCodecs];
>> 
>> This needs to be indented.
> 
> We don't want to change the behavior of video.canPlayType, so I would put this into MediaPlayerPrivateMediaSourceAVFObjC::supportsType.

I see. Do we need to remove the similar code section in AVStreamDataParserMIMETypeCache::canDecodeExtendedType() after we add it into MediaPlayerPrivateMediaSourceAVFObjC::supportsType()? I suppose yes.

Created attachment 390982 [details]
Patch

Comment on attachment 390982 [details]
Patch

Clearing flags on attachment: 390982

Committed r256804: <https://trac.webkit.org/changeset/256804>

All reviewed patches have been landed.  Closing bug.

Looks like the changes in https://trac.webkit.org/changeset/256804/webkit caused 50+ crashes on macOS debug.

Results: https://build.webkit.org/results/Apple-Catalina-Debug-WK2-Tests/r256824%20(2379)/results.html  


Build: https://build.webkit.org/builders/Apple-Catalina-Debug-WK1-Tests/builds/2532

ASSERTION FAILED: !outputCodecs.isEmpty()

This is found in the changes that were made in the revision.

Reverted r256804 for reason:

This broke 50+ media tests on mac debug

Committed r256828: <https://trac.webkit.org/changeset/256828>

Created attachment 391071 [details]
Fix layout test failures on the Mac debug build

The commit-queue encountered the following flaky tests while processing attachment 391071 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.

Comment on attachment 391071 [details]
Fix layout test failures on the Mac debug build

Clearing flags on attachment: 391071

Committed r256856: <https://trac.webkit.org/changeset/256856>

All reviewed patches have been landed.  Closing bug.