[MSE] Handle the case where AVStreamDataParser packages sync and non-sync samples together in a CMSampleBufferRef.

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

[MSE] Handle the case where AVStreamDataParser packages sync and non-sync samples together in a CMSampleBufferRef.

[Jer Noble]

<rdar://problem/60625209>

[Jer Noble]

Created attachment 394140 [details]
Patch

[Eric Carlson]

Comment on attachment 394140 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:342
> +    CFArrayRef attachmentsArray = CMSampleBufferGetSampleAttachmentsArray(m_sample.get(), true);

The documentation says CMSampleBufferGetSampleAttachmentsArray can return NULL, and some of our code does too (e.g. isCMSampleBufferNonDisplaying), so this should probably check.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:344
> +    if (count == 1)

<= 1

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:369
> +    CFArrayRef attachmentsArray = CMSampleBufferGetSampleAttachmentsArray(m_sample.get(), true);
> +    auto count = CFArrayGetCount(attachmentsArray);
> +    if (count == 1)

Ditto the comments above.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:377
> +

Nit: the extra blank isn't really needed.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.mm:399
> +        if (CMSampleBufferCopySampleBufferForRange(kCFAllocatorDefault, m_sample.get(), range, &rawSample) != noErr || !rawSample)
> +            return { };

It would be good to log an error and maybe ASSERT

[Jer Noble]

Created attachment 394143 [details]
Patch for landing

[EWS]

Committed r258846: <https://trac.webkit.org/changeset/258846>

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