Bug 154544

Summary: Refactor CoreVideo API access into their own classes so code can be re-used.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, eric.carlson, ossy
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=169931
Bug Depends on:    
Bug Blocks: 125157    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
eric.carlson: review+
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing
none
Patch for landing none

Description Jer Noble 2016-02-22 11:17:23 PST
Refactor CoreVideo API access into their own classes so code can be re-used.
Comment 1 Jer Noble 2016-02-22 11:21:07 PST
Created attachment 271936 [details]
Patch
Comment 2 Jer Noble 2016-02-22 13:00:54 PST
Created attachment 271944 [details]
Patch
Comment 3 Jer Noble 2016-02-22 13:10:38 PST
Created attachment 271946 [details]
Patch
Comment 4 Jer Noble 2016-02-22 13:21:09 PST
Created attachment 271949 [details]
Patch
Comment 5 Eric Carlson 2016-02-22 13:39:17 PST
Comment on attachment 271949 [details]
Patch

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

> Source/WebCore/platform/graphics/cv/PixelBufferConformerCV.cpp:42
> +    VTPixelBufferConformerRef conformer = 0;

Nit: nullptr?

> Source/WebCore/platform/graphics/cv/PixelBufferConformerCV.cpp:44
> +    m_pixelConformer = adoptCF(conformer);

Nit: I'll bet the various VTPixelBufferConformer calls aren't NULL safe, so an ASSERT(conformer) and/or the OSStatus returned by VTPixelBufferConformerCreateWithAttributes here might be useful.

> Source/WebCore/platform/graphics/cv/PixelBufferConformerCV.h:41
> +    PixelBufferConformerCV(CFDictionaryRef attributes);

Nit: the parameter name isn't necessary.
Comment 6 Jer Noble 2016-02-22 22:50:00 PST
Created attachment 271991 [details]
Patch for landing
Comment 7 Jer Noble 2016-02-29 13:15:45 PST
Created attachment 272512 [details]
Patch for landing
Comment 8 Jer Noble 2016-02-29 13:52:04 PST
Created attachment 272515 [details]
Patch for landing
Comment 9 Jer Noble 2016-02-29 14:07:26 PST
Created attachment 272517 [details]
Patch for landing
Comment 10 Jer Noble 2016-02-29 14:39:24 PST
Created attachment 272519 [details]
Patch for landing
Comment 11 Jer Noble 2016-02-29 14:55:17 PST
Created attachment 272520 [details]
Patch for landing
Comment 12 Jer Noble 2016-02-29 15:10:13 PST
Created attachment 272523 [details]
Patch for landing
Comment 13 Jer Noble 2016-02-29 16:09:40 PST
Created attachment 272527 [details]
Patch for landing
Comment 14 Jer Noble 2016-02-29 16:44:54 PST
Committed r197375: <http://trac.webkit.org/changeset/197375>
Comment 15 Csaba Osztrogonác 2016-03-01 00:21:18 PST
(In reply to comment #14)
> Committed r197375: <http://trac.webkit.org/changeset/197375>

It broke the Apple Mac cmake build:
/Volumes/Data/slave/elcapitan-cmake-debug/build/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:56:9: fatal error: 'PixelBufferConformerCV.h' file not found
#import "PixelBufferConformerCV.h"
        ^
Comment 16 Csaba Osztrogonác 2016-03-01 03:54:55 PST
fixed by https://trac.webkit.org/changeset/197397
( But the build is still broken because of bug154518 )
Comment 17 Csaba Osztrogonác 2016-03-01 03:59:12 PST
One more fix landed in http://trac.webkit.org/changeset/197398