Bug 237472

<rdar://problem/89810969>

Created attachment 453889 [details]
Patch

Created attachment 453890 [details]
Combined patch for EWS tests

Created attachment 453891 [details]
Combined patch for EWS tests

fix compilation TV and Watch

Created attachment 453899 [details]
Combined patch for EWS tests

Created attachment 453927 [details]
Patch for review

Created attachment 453928 [details]
Combined Patch for EWS

Comment on attachment 453927 [details]
Patch for review

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

> Source/WebCore/platform/graphics/cocoa/CMUtilities.mm:217
> +    if (!format)
> +        return nullptr;

It is probably worth logging an error to help when this, presumably rare, failure happens.

> Source/WebCore/platform/graphics/cocoa/CMUtilities.mm:225
> +        if (err != kCMBlockBufferNoErr || !rawBlockBuffer)
> +            return nullptr;

Ditto

> Source/WebCore/platform/graphics/cocoa/CMUtilities.mm:239
> +        if (!blockBuffer)
> +            return nullptr;

Ditto

> Source/WebCore/platform/graphics/cocoa/CMUtilities.mm:246
> +            if (err != kCMBlockBufferNoErr)
> +                return nullptr;

Ditto

> Source/WebCore/platform/graphics/cocoa/CMUtilities.mm:254
> +    if (auto err = PAL::CMSampleBufferCreateReady(kCFAllocatorDefault, completeBlockBuffers.get(), format.get(), packetSizes.size(), packetTimings.size(), packetTimings.data(), packetSizes.size(), packetSizes.data(), &rawSampleBuffer))
> +        return nullptr;

Ditto

> Source/WebCore/platform/graphics/cocoa/CMUtilities.mm:258
> +        ASSERT(attachmentsArray);

Is this failure more serious than those above? If not, should those assert too?

> Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:566
> +    if (!m_parser)
> +        return -1;

Yuck. Maybe have this return an ExceptionOr<int> instead?

> Tools/ChangeLog:26
> +        * TestWebKitAPI/Tests/WebCore/SampleMap.cpp: Update following base class definition change.

Looks like this is in the wrong place?

Comment on attachment 453927 [details]
Patch for review

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

>> Source/WebCore/platform/graphics/cocoa/CMUtilities.mm:217
>> +        return nullptr;
> 
> It is probably worth logging an error to help when this, presumably rare, failure happens.

This is a utility method. AFAIK, there’s no logging possible here. The error is logged on return

>> Source/WebCore/platform/graphics/cocoa/CMUtilities.mm:258
>> +        ASSERT(attachmentsArray);
> 
> Is this failure more serious than those above? If not, should those assert too?

This is carrying on the old code. It can’t happen unless there’s a change in behaviour in the CM method. 

We create the CM Sample and give it an array of timings etc. So there will always be an arrray there. 

The other errors indicates an OOM

>> Source/WebCore/platform/graphics/cocoa/SourceBufferParserWebM.cpp:566
>> +        return -1;
> 
> Yuck. Maybe have this return an ExceptionOr<int> instead?

Could return a new error code. 

Otherwise , crap in, crap out. This indicates the demuxer was called after being torn down. Which can’t happen currently

Comment on attachment 453927 [details]
Patch for review

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

>> Tools/ChangeLog:26
>> +        * TestWebKitAPI/Tests/WebCore/SampleMap.cpp: Update following base class definition change.
> 
> Looks like this is in the wrong place?

what do you mean? this method is modified and that template was generated by webkit-patch

Created attachment 453950 [details]
Patch for review

Propage error detail

Created attachment 453951 [details]
Patch for review

Comment on attachment 453927 [details]
Patch for review

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

>>> Tools/ChangeLog:26
>>> +        * TestWebKitAPI/Tests/WebCore/SampleMap.cpp: Update following base class definition change.
>> 
>> Looks like this is in the wrong place?
> 
> what do you mean? this method is modified and that template was generated by webkit-patch

I meant it is in the wrong comment block.

Committed r291025 (248199@main): <https://commits.webkit.org/248199@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453951 [details].