[webkit-reviews] review granted: [Bug 176825] [Cocoa] Add an ImageDecoder subclass backed by AVFoundation : [Attachment 320903] 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 176825: [Cocoa] Add an ImageDecoder subclass backed by AVFoundation
https://bugs.webkit.org/show_bug.cgi?id=176825

Attachment 320903: Patch

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




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

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

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:66
> +SOFT_LINK_FRAMEWORK(AVFoundation)
> +SOFT_LINK_CLASS(AVFoundation, AVURLAsset)
> +SOFT_LINK_CLASS(AVFoundation, AVSampleBufferGenerator)
> +SOFT_LINK_CLASS(AVFoundation, AVSampleBufferRequest)
> +SOFT_LINK_CONSTANT(AVFoundation, AVMediaCharacteristicVisual,
AVMediaCharacteristic)
> +SOFT_LINK_CONSTANT(AVFoundation, AVURLAssetReferenceRestrictionsKey,
NSString *)

These should be SOFT_LINK_OPTIONAL_* because AVFoundation is not available on a
recovery partition.

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:427
> +    return m_sampleData.isEmpty() ? EncodedDataStatus::Unknown :
EncodedDataStatus::Complete;

Nit: I believe early return is the preferred style for a short method like
this:

if (!m_sampleData.isEmpty())
    return EncodedDataStatus::Complete;

return EncodedDataStatus::Unknown;

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:432
> +    return m_size ? m_size.value() : IntSize();

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:460
> +    return m_size ? m_size.value() : IntSize();

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:465
> +    return index < m_sampleData.size() ? m_sampleData[index].image : false;

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:475
> +    return index < m_sampleData.size() ? m_sampleData[index].duration : 0;

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/ImageDecoderAVFObjC.mm:480
> +    return index < m_sampleData.size() ? m_sampleData[index].hasAlpha :
false;

Ditto.