[webkit-reviews] review granted: [Bug 177119] Creating a second AVPlayerItemVideoOutput causes flakey failures : [Attachment 328452] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 177119: Creating a second AVPlayerItemVideoOutput causes flakey failures
https://bugs.webkit.org/show_bug.cgi?id=177119

Attachment 328452: Patch

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




--- Comment #33 from Dean Jackson <dino at apple.com> ---
Comment on attachment 328452
  --> https://bugs.webkit.org/attachment.cgi?id=328452
Patch

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

> Source/WebCore/ChangeLog:39
> +	   uploading BGRA pixel formatted data to a texture; work aronud this
bug for now.

Typo: around

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:2333
> -    if (!pixelBuffer)
> +    if (!updateLastPixelBuffer() && (m_lastImage || !m_lastPixelBuffer))

I don't quite understand the logic change here. The !pixelBuffer and
!updateLastPixelBuffer() are effectively the same thing... but why did you add
&& (m_lastImage || !m_lastPixelBuffer)?

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:329
> +    fragmentShaderSource.appendLiteral("uniform int u_swapColors;\n");

naming: flipColorChannels? swapColorChannels?

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:452
> -	   "	yuv.gb = TEXTUREFUNC(u_uvTexture, v_uvTextureCoordinate).rg -
vec2(0.5, 0.5);\n"
> -	   "	gl_FragColor = vec4(yuv * u_colorMatrix, 1);\n"
> +	   "	yuv.gb = TEXTUREFUNC(u_uvTexture, v_uvTextureCoordinate).rg;\n"

Why aren't you subtracting 0.5, 0.5 any more?

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:-723
> -VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver(GraphicsContext3D&
context)
> -    : m_context(context)

Booohooooo.... this had so much promise :)