Our video sink now needs to support the NV12 format to get the GLTextureUploadMeta support working.
Created attachment 215376 [details] patch
Attachment 215376 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp', u'Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp']" exit_code: 1 Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:47: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:48: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:49: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:50: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:51: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:53: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:54: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:55: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:56: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:57: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:59: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:60: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:61: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:62: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:63: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:64: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:67: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:68: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:69: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:70: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:71: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Total errors found: 21 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 215377 [details] patch
Sebastian, does this change makes sense to you? :)
Comment on attachment 215377 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=215377&action=review > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:355 > +#if GST_CHECK_VERSION(1, 3, 0) Why 1.3.0? All this should be supported since GStreamer 1.2.0. > Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:64 > #define WEBKIT_VIDEO_SINK_PAD_CAPS GST_FEATURED_CAPS GST_VIDEO_CAPS_MAKE(GST_CAPS_FORMAT) This now means that you also have video/x-raw,format=NV12 here (i.e. without the caps feature). However without the GL texture upload meta NV12 is not supported by the sink.
Comment on attachment 215377 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=215377&action=review >> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:355 >> +#if GST_CHECK_VERSION(1, 3, 0) > > Why 1.3.0? All this should be supported since GStreamer 1.2.0. But there's no release of gst-vaapi with that support yet and they moved all their code-checking to 1.3 already... So I thought I'd do the same :) >> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:64 >> #define WEBKIT_VIDEO_SINK_PAD_CAPS GST_FEATURED_CAPS GST_VIDEO_CAPS_MAKE(GST_CAPS_FORMAT) > > This now means that you also have video/x-raw,format=NV12 here (i.e. without the caps feature). However without the GL texture upload meta NV12 is not supported by the sink. Hmm, right... Ok I'll revise the patch. Thanks for looking!
Created attachment 216010 [details] Patch
When using the NV12 caps, are there two textures created from the GstGLVideoTextureUploadMeta? Or only a single one that behaves like (A)RGB?
(In reply to comment #8) > When using the NV12 caps, are there two textures created from the GstGLVideoTextureUploadMeta? Or only a single one that behaves like (A)RGB? Only one texture in GL_RGBA format. The code is in gst/vaapi/gstvaapivideometa_texture.c In the end this patch only fixes caps negotiation :)
(In reply to comment #9) > (In reply to comment #8) > > When using the NV12 caps, are there two textures created from the GstGLVideoTextureUploadMeta? Or only a single one that behaves like (A)RGB? > > Only one texture in GL_RGBA format. The code is in gst/vaapi/gstvaapivideometa_texture.c > > In the end this patch only fixes caps negotiation :) That's a bug on their side then, it should use RGBA in the caps instead of NV12.
Sree thinks otherwise in https://bugzilla.gnome.org/show_bug.cgi?id=687183 It'd be nice to reach a consensus about this at some point.
Comment on attachment 216010 [details] Patch Pulling this out of review since it's a gst-vaapi bug