Bug 181445 – WebGL video texture black in Safari 11.0.2 and wrong colored in Safari Preview 11.1

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Klaus Reinfeld]

Created attachment 330841 [details]
screenshot

Hi, since Safari 11.0.2 WebGL video texture usage is broken in some cases - only black pixels will be rendered from the video texture.

The behavior itself is very strange and not logical to reproduce - in some examples with basically the same WebGL rendering code and the same video the video textures are working and in some other examples not...

Here's a not working example:
https://krpano.com/krpanocloud/video/airpano/

Older Safari versions and all others browsers are fine!

The next strange thing - in Safari Technology Preview (Release 46, 11.1) the same example is working again, but now with wrong colors (green-tinted)...

Attached a screenshot for comparison:
- Safari 11.02 - black texture
- Safari Preview 11.1 - wrong colored texture
- Chrome - as it should

[Radar WebKit Bug Importer]

<rdar://problem/36383183>

[Jer Noble]

It looks like this video is encoded in '420f' pixel format, meaning YCbCr, full-range. We may have a bug in our YUV->RGB conversion for full-range video. As a workaround, you may want to try re-encoding your source video in video-range '420v' pixel format and see if that corrects the incorrect color issue.

[Jer Noble]

(In reply to Jer Noble from comment #2)
> It looks like this video is encoded in '420f' pixel format, meaning YCbCr,
> full-range. We may have a bug in our YUV->RGB conversion for full-range
> video. As a workaround, you may want to try re-encoding your source video in
> video-range '420v' pixel format and see if that corrects the incorrect color
> issue.

Actually, if I force the interpretation of the pixel format to '420v', the colors all look correct again. It may just be the case that the video is mis-tagged as full-range.

[Daniel Rossi]

Wow are you saying you get black textures for Mp4 not just HLS ? Mp4 was never a problem.

And even further regressions. Do you now see inverted colours for Mp4 not just HLS  ?

Here is the fix I worked out for IOS 10 HLS streaming. It inverts the colour channel on the fragment shader. 

https://bugs.webkit.org/show_bug.cgi?id=163866#c16

I have a VM of high sierra but it won't let me play HLS for some reason so I can't even check if things are fixed in it or get preview versions of Safari. Pretty stuck to even continue testing and reporting.

[Klaus Reinfeld]

(In reply to Jer Noble from comment #2)
> It looks like this video is encoded in '420f' pixel format, meaning YCbCr,
> full-range. We may have a bug in our YUV->RGB conversion for full-range
> video. As a workaround, you may want to try re-encoding your source video in
> video-range '420v' pixel format and see if that corrects the incorrect color
> issue.

Yes, the video is (and should be) in full-range format.
It was encoded using ffmpeg.

I have made now here a new example that allows switching between videos with several pixel formats (all encoded using ffmpeg):

https:///krpano.com/video/pixelformattest/

(click the gear-wheel icon in the right-bottom and then pixelformat to change the video)

Here the results:

4:2:0 16-235 (yuv420p) 
- shows only the first frame in Safari 11.0.2
- works in Safari Preview 11.1 but the colors are slightly different to Chrome (not sure which ones are correct)

4:2:2 16-235 (yuv422p) - doesn't load at all
4:4:4 16-235 (yuv444p) - doesn't load at all

4:2:0 0-255 (yuvj420p)
- shows only the first frame in Safari 11.0.2
- wrong (green-tinted) colors in Safari Preview 11.1

4:2:2 0-255 (yuvj422p) - doesn't load at all
4:4:4 0-255 (yuvj444p) - doesn't load at all

The bigger problem is still Safari 11.0.2 - after recoding the video there is now the first frame there instead of a black frame, but the video textures itself still don't render...

[Klaus Reinfeld]

Created attachment 330886 [details]
screenshot 4:2:0 videorange

[Klaus Reinfeld]

Created attachment 330887 [details]
screenshot 4:2:0 fullrange

[Klaus Reinfeld]

(In reply to Daniel Rossi from comment #4)
> Wow are you saying you get black textures for Mp4 not just HLS ? Mp4 was
> never a problem.

Yes, that doesn't seem to be not related to HLS and also not to the flipY bug.


> And even further regressions. Do you now see inverted colours for Mp4 not
> just HLS  ?
> 
> Here is the fix I worked out for IOS 10 HLS streaming. It inverts the colour
> channel on the fragment shader. 
> 
> https://bugs.webkit.org/show_bug.cgi?id=163866#c16
> 
> I have a VM of high sierra but it won't let me play HLS for some reason so I
> can't even check if things are fixed in it or get preview versions of
> Safari. Pretty stuck to even continue testing and reporting.

OT: I think VM are never good for real tests...
Why not simply using a new partition or new disk for HighSierra?
That works nicely with OSX - e.g. I have two MacMini systems for testing and each has 3-4 partitions with different OSX versions...

[Daniel Rossi]

My previous mac was very faulty and died as in constant firmware resets failed to get it to boot.

I'm on windows now with specific hardware but there is mac GTX drivers now which might work. I have a VR workstation. 

Could possibly try installing on my non apple hardware workstation on a third SSD. It does not warrant hardware to report and test bugs. 

I tried a remote service running mac mini but was too limited in keeping up to date to track if the webkit updates fix the issues. 

This sounds like it might be similar to the IOS 11 bug where disabling FlipY is not relevant anymore and black rendering but for HLS. 

https://bugs.webkit.org/show_bug.cgi?id=179417

[Daniel Rossi]

These two tests I made for the IOS11 bug are working in 11.0.2, 10.13.2. FlipY and all. From the VM also until I can work out another test environment. 

If Chrome is disabling WebGL for older Macbook's with Intel HD graphics. Could it be graphics card related ? 


threejs -http://dev.electroteque.org/webgl/three-mp4.html
raw webgl - http://dev.electroteque.org/webgl/webgl-mp4.html

[Daniel Rossi]

Your source file which I had to capture in the network logs. 

Is working with Safari  in this example code

http://dev.electroteque.org/webgl/three-mp4.html

Your player is not in Safari.

[Daniel Rossi]

It's also working for me in this example code

http://dev.electroteque.org/webgl/webgl-mp4.html

[Daniel Rossi]

Confirming the colour inversion with the technology preview with your mp4 file but not with my example video. 

I tried to do this as a work around but Safari doesn't support it , webgl program breaks. 

gl_FragColor = texture2D( texture, vUV ).bgra;

[Daniel Rossi]

The first example here works with the Safari Preview in 10.13. No black frame . no colour inversion. yuv420p

https://krpano.com/krpano.html?xml=video/pixelformattest/videopano.xml

So is there going to be a requirement for the future Safari to re-encode problem files to work around this bug ? 

What is the ffmpeg command ?

[Jer Noble]

(In reply to Klaus Reinfeld from comment #5)
> (In reply to Jer Noble from comment #2)
> > It looks like this video is encoded in '420f' pixel format, meaning YCbCr,
> > full-range. We may have a bug in our YUV->RGB conversion for full-range
> > video. As a workaround, you may want to try re-encoding your source video in
> > video-range '420v' pixel format and see if that corrects the incorrect color
> > issue.
> 
> Yes, the video is (and should be) in full-range format.
> It was encoded using ffmpeg.
> 
> I have made now here a new example that allows switching between videos with
> several pixel formats (all encoded using ffmpeg):

Thanks for making these test cases, this will help with debugging quite a bit!  It'd also be helpful to post the command line you passed to ffmpeg to generate these samples so we can reproduce the outputs with other test media.

Since WebKit on Mac and iOS use AVFoundation to do video decoding, one way to test for basic decoding support would be to load these .mp4s in QuickTime Player (which also uses AVFoundation). Here's what I see on my High Sierra machine:

> https:///krpano.com/video/pixelformattest/
> 
> (click the gear-wheel icon in the right-bottom and then pixelformat to
> change the video)
> 
> Here the results:
> 
> 4:2:0 16-235 (yuv420p) 
> - shows only the first frame in Safari 11.0.2

Loads in QTP, and colors look correct.

> - works in Safari Preview 11.1 but the colors are slightly different to
> Chrome (not sure which ones are correct)

Are you explicitly specifying color primaries and a transfer function? WebKit and Blink may be choosing different defaults in the absence of explicit color metadata.

> 4:2:2 16-235 (yuv422p) - doesn't load at all

Fails to load in QTP

> 4:4:4 16-235 (yuv444p) - doesn't load at all

Fails to load in QTP

> 4:2:0 0-255 (yuvj420p)
> - shows only the first frame in Safari 11.0.2
> - wrong (green-tinted) colors in Safari Preview 11.1

Loads in QTP, colors look the same as yuv420p.

> 4:2:2 0-255 (yuvj422p) - doesn't load at all

Fails to load in QTP.

> 4:4:4 0-255 (yuvj444p) - doesn't load at all

Fails to load in QTP.

I suspect that the 422 and 444 formats aren't supported by AVFoundation's AVC decoder. This piece of the issue needs to have a Radar filed against it; it's a platform bug and not a WebKit one.

[Klaus Reinfeld]

(In reply to Jer Noble from comment #15)
> Thanks for making these test cases, this will help with debugging quite a
> bit!  It'd also be helpful to post the command line you passed to ffmpeg to
> generate these samples so we can reproduce the outputs with other test media.

That's the basic ffmpeg command line: 

ffmpeg -i input.mp4 -vf format=yuv420p -c:a copy output.mp4

with these parameter values as format:

yuv420p - for video range (16-235)
yuv422p
yuv444p
yuvj420p - with 'j' is for full range (0-255)
yuvj422p
yuvj444p


> I suspect that the 422 and 444 formats aren't supported by AVFoundation's
> AVC decoder. This piece of the issue needs to have a Radar filed against it;
> it's a platform bug and not a WebKit one.

The not working/supported 422 and 444 formats are not that big problem (even 444 support would be nice), but the BLACK video-texture in Safari 11.0.2 and green-tinted texture in Safari Preview with the fullrange video are a big problems!

[Jer Noble]

(In reply to Klaus Reinfeld from comment #16)
> (In reply to Jer Noble from comment #15)
> > Thanks for making these test cases, this will help with debugging quite a
> > bit!  It'd also be helpful to post the command line you passed to ffmpeg to
> > generate these samples so we can reproduce the outputs with other test media.
> 
> That's the basic ffmpeg command line: 
> 
> ffmpeg -i input.mp4 -vf format=yuv420p -c:a copy output.mp4
> 
> with these parameter values as format:
> 
> yuv420p - for video range (16-235)
> yuv422p
> yuv444p
> yuvj420p - with 'j' is for full range (0-255)
> yuvj422p
> yuvj444p

Okay, I'm able to reproduce the problem with a simple SMPTE color bars example (from an image sequence). The following ffmpeg command line creates a file with the green tint problem:

ffmpeg -r 30 -f image2 -s 1920x1080 -i SMPTE_Color_Bars_16x9/%d.png -vcodec libx264 -crf 25 -pix_fmt yuvj420p test-colorspace-DEFAULT-420p.mp4

However, if you add the ffmpeg flags to make the colorspace, primaries, transfer function, and range explicit (rather than implicit from the pixel format), the color comes out correctly in a sample WebGL test:

ffmpeg -r 30 -f image2 -s 1920x1080 -i SMPTE_Color_Bars_16x9/%d.png -vcodec libx264 -crf 25 -pix_fmt yuvj420p -color_primaries bt709  -color_trc bt709 -colorspace bt709 -color_range pc test-colorspace-bt709-420f.mp4

You can verify that these metadata flags are added to the output file with mediainfo:

> mediainfo test-colorspace-bt709-420f.mp4         
...snip...
Color range                              : Full
Color primaries                          : BT.709
Transfer characteristics                 : BT.709
Matrix coefficients                      : BT.709

So you probably just need to make your colorspaces explicit, so that UAs can apply the correct transforms when generating RGB output.

[Klaus Reinfeld]

(In reply to Jer Noble from comment #17)
> However, if you add the ffmpeg flags to make the colorspace, primaries,
> transfer function, and range explicit (rather than implicit from the pixel
> format), the color comes out correctly in a sample WebGL test:
> 
> ffmpeg -r 30 -f image2 -s 1920x1080 -i SMPTE_Color_Bars_16x9/%d.png -vcodec
> libx264 -crf 25 -pix_fmt yuvj420p -color_primaries bt709  -color_trc bt709
> -colorspace bt709 -color_range pc test-colorspace-bt709-420f.mp4
> 

I have added an additional video ('yuvj420p-bt709') encoded with these settings to the test case:

https://krpano.com/video/pixelformattest/

and yes, with these settings the colors are correct.



> You can verify that these metadata flags are added to the output file with
> mediainfo:
> 
> > mediainfo test-colorspace-bt709-420f.mp4         
> ...snip...
> Color range                              : Full
> Color primaries                          : BT.709
> Transfer characteristics                 : BT.709
> Matrix coefficients                      : BT.709

Sorry, I don't know about 'mediainfo' but here the video files from the above example for downloading (~30mb per file):

https://krpano.com/video/pixelformattest/yuv420p.mp4
https://krpano.com/video/pixelformattest/yuv422p.mp4
https://krpano.com/video/pixelformattest/yuv444p.mp4
https://krpano.com/video/pixelformattest/yuvj420p.mp4
https://krpano.com/video/pixelformattest/yuvj420p-bt709.mp4
https://krpano.com/video/pixelformattest/yuvj422p.mp4
https://krpano.com/video/pixelformattest/yuvj424p.mp4



> So you probably just need to make your colorspaces explicit, so that UAs can
> apply the correct transforms when generating RGB output.

Okay, but that can't be the solution, or?

All other videoplayers (also the Quicktime player) are showing the color correct also without that settings.

So maybe a kind of 'default behavior' when these settings are missing is wrong in the Safari WebGL video texture color conversion?

---

Btw - about the BLACK texture problem in Safari 11.0.2 with that example:
https://krpano.com/krpanocloud/video/airpano/

Could that be related to this bug?:
https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp?rev=222198

A broken OpenGL state could explain the behavior why some codes/examples are working and some others not...

[Jer Noble]

(In reply to Klaus Reinfeld from comment #18)
> (In reply to Jer Noble from comment #17)
> > However, if you add the ffmpeg flags to make the colorspace, primaries,
> > transfer function, and range explicit (rather than implicit from the pixel
> > format), the color comes out correctly in a sample WebGL test:
> > 
> > ffmpeg -r 30 -f image2 -s 1920x1080 -i SMPTE_Color_Bars_16x9/%d.png -vcodec
> > libx264 -crf 25 -pix_fmt yuvj420p -color_primaries bt709  -color_trc bt709
> > -colorspace bt709 -color_range pc test-colorspace-bt709-420f.mp4
> > 
> 
> I have added an additional video ('yuvj420p-bt709') encoded with these
> settings to the test case:
> 
> https://krpano.com/video/pixelformattest/
> 
> and yes, with these settings the colors are correct.
> 
> 
> 
> > You can verify that these metadata flags are added to the output file with
> > mediainfo:
> > 
> > > mediainfo test-colorspace-bt709-420f.mp4         
> > ...snip...
> > Color range                              : Full
> > Color primaries                          : BT.709
> > Transfer characteristics                 : BT.709
> > Matrix coefficients                      : BT.709
> 
> Sorry, I don't know about 'mediainfo' 

mediainfo is a command-line (or GUI) tool that pulls metadata out of media files. <https://github.com/MediaArea>

It's not important in and of itself except that it verifies that color metadata is present in the file.

> but here the video files from the
> above example for downloading (~30mb per file):
> 
> https://krpano.com/video/pixelformattest/yuv420p.mp4
> https://krpano.com/video/pixelformattest/yuv422p.mp4
> https://krpano.com/video/pixelformattest/yuv444p.mp4
> https://krpano.com/video/pixelformattest/yuvj420p.mp4
> https://krpano.com/video/pixelformattest/yuvj420p-bt709.mp4
> https://krpano.com/video/pixelformattest/yuvj422p.mp4
> https://krpano.com/video/pixelformattest/yuvj424p.mp4
> 
> 
> 
> > So you probably just need to make your colorspaces explicit, so that UAs can
> > apply the correct transforms when generating RGB output.
> 
> Okay, but that can't be the solution, or?

If you want correct colors, it has to be. The matrices to generate RGB data from the media's YCbCr data are different depending on what those color metadata values are.

Tagging media files with color metadata is a best-practices thing, and not including it means you're going to have a bad time.

That said...:

> All other videoplayers (also the Quicktime player) are showing the color
> correct also without that settings.
> 
> So maybe a kind of 'default behavior' when these settings are missing is
> wrong in the Safari WebGL video texture color conversion?

That's true. And I do need to figure out what's causing the green cast. But note that the different video players and UAs are showing "correct" colors, but not necessarily the same colors. Hence the point above.

> ---
> 
> Btw - about the BLACK texture problem in Safari 11.0.2 with that example:
> https://krpano.com/krpanocloud/video/airpano/
> 
> Could that be related to this bug?:
> https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/
> graphics/cv/VideoTextureCopierCV.cpp?rev=222198
> 
> A broken OpenGL state could explain the behavior why some codes/examples are
> working and some others not...

That's also true. I never really determined what specific piece of state was getting trampled on, so I don't have an easy answer for you about how to work around this behavior.  It might be useful to use the OpenGL Profiler on a simplified test case to see the GL commands (both from WebGL and from the TextureCopierCV) during execution and try to spot what state may have been overwritten.

[Jer Noble]

(In reply to Jer Noble from comment #19)
> (In reply to Klaus Reinfeld from comment #18)
> > So maybe a kind of 'default behavior' when these settings are missing is
> > wrong in the Safari WebGL video texture color conversion?
> 
> That's true. And I do need to figure out what's causing the green cast.

Looks like I made a math error in calculating the [1,4] value of the ITU_R_601_4 * FullRange color matrix. So indeed the problem is with r601-full pixel formats.

[Jer Noble]

Created attachment 331148 [details]
Patch

[Klaus Reinfeld]

(In reply to Jer Noble from comment #21)
> Created attachment 331148 [details]
> Patch

Nice and clear solution!
Thanks!

[Dean Jackson]

Comment on attachment 331148 [details]
Patch

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

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:141
> +    static constexpr GLfloat abs(GLfloat value)
> +    {
> +        return value >= 0 ? value : -value;
> +    }

Any reason this isn't just a file static function? Is a constexpr on the struct better?

> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:183
> +    GLfloat cgCoefficient = 1 - cbCoefficient - crCoefficient;

Maybe this should go after the comments?

[Dean Jackson]

Comment on attachment 331148 [details]
Patch

Can we have a test?

[Jer Noble]

(In reply to Dean Jackson from comment #23)
> Comment on attachment 331148 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=331148&action=review
> 
> > Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:141
> > +    static constexpr GLfloat abs(GLfloat value)
> > +    {
> > +        return value >= 0 ? value : -value;
> > +    }
> 
> Any reason this isn't just a file static function? Is a constexpr on the
> struct better?

I didn't want to put it at the WebCore namespace scope. Unified builds mean that things at namespace scope can leak between source files. The other option would be to put this all in a new nested namespace.

> > Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:183
> > +    GLfloat cgCoefficient = 1 - cbCoefficient - crCoefficient;
> 
> Maybe this should go after the comments?

Sure.

> Can we have a test?

There's a bunch of tests inline that verify the correctness of the various matrices. I thought this was a better approach because it doesn't depend on the implementation details of the platform. That said, we should probably have a bunch of SMPTE bar tests that verify correctness end-to-end. Seems like a web-platform-test kind of thing.

[Jer Noble]

Created attachment 331210 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 331210 [details]
Patch for landing

Clearing flags on attachment: 331210

Committed r226898: <https://trac.webkit.org/changeset/226898>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.