[webkit-reviews] review granted: [Bug 196772] iOS 12.2 Drawing portrait video to canvas is sideways : [Attachment 372372] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 196772: iOS 12.2 Drawing portrait video to canvas is sideways
https://bugs.webkit.org/show_bug.cgi?id=196772

Attachment 372372: Patch

https://bugs.webkit.org/attachment.cgi?id=372372&action=review




--- Comment #19 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 372372
  --> https://bugs.webkit.org/attachment.cgi?id=372372
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:428
> +    if (m_imageRotationSession)

Might be worth adding ASSERT(m_imageRotationSession) here to catch this in
debug builds.

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:1863
> +    if (!finalTransform.isIdentity())
> +	   m_imageRotationSession =
std::make_unique<ImageRotationSessionVT>(WTFMove(finalTransform),
expandedIntSize(m_cachedPresentationSize), kCVPixelFormatType_32BGRA,
ImageRotationSessionVT::IsCGImageCompatible::Yes);
> +    else
> +	   m_imageRotationSession = nullptr;

Tracks change so fequently with HLS that it might be worth doing this only when
m_cachedPresentationSize changes.

Also, shouldn't we also do this when
MediaPlayerPrivateAVFoundationObjC::sizeChanged is called?

> Source/WebCore/platform/graphics/cv/ImageRotationSessionVT.mm:92
> +    CVPixelBufferPoolCreate(kCFAllocatorDefault, nullptr, (__bridge
CFDictionaryRef)pixelAttributes, &rawPool);

Even if it is safe to assume it is OK to ignore this failing, we should log an
error when it does.