GNOME Bugzilla – Bug 698054
Port gstreamer-vaapi to GStreamer API 1.2.x
Last modified: 2013-12-19 12:14:47 UTC
* Port gstreamer-vaapi to use GstContext instead of the -already removed- GstVideoContext * Port gstreamer-vaapi to use GstVideoGLTextureUploadMeta instead of the -already removed- GstSurfaceConverter
Those APIs are removed from GStreamer git master (towards 1.2.x), not from 1.0.x where they will be deprecated instead. It would be fine to indeed port gstreamer-vaapi to GStreamer git master and use the new APIs. Then, if that works good enough, backport the necessary support to 1.0.x. However, meanwhile, and since OSVs are already using GStreamer 1.0.x as is, I have just implemented support GstSurfaceMeta. Tested with a slightly patched clutter-gst 1.9.92 only.
Created attachment 241576 [details] [review] [clutter-gst] Add HW decoder support back Patch against clutter-gst 1.9.92 so that to test GstSurfaceMeta API + gstreamer-vaapi.
Created attachment 244557 [details] [review] intial port to gstreamer 1.2 api I'm starting to work on this. The current patch only enables the compilation of gstreamer-vaapi with gstreamer HEAD. The next step is connect the dots for the context and the texture upload meta. Another stage would be the use of caps features. __gb__ what do you thing about this approach?
And yet another stage would be to implement a vaapi specific GstAllocator/GstMemory, and deferring the jobs of the vaapi GstMeta to those for conceptional cleanness and flexibility :)
(In reply to comment #3) > Created an attachment (id=244557) [details] [review] > intial port to gstreamer 1.2 api > > I'm starting to work on this. The current patch only enables the compilation of > gstreamer-vaapi with gstreamer HEAD. > > The next step is connect the dots for the context and the texture upload meta. > > Another stage would be the use of caps features. I have implemented the capsfeature for testing this: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=a8d1b4549184f4aec88132b3650fa2b8fe1ca970 I will provide the proper patch once I back from vacation :) > > __gb__ what do you thing about this approach?
Created attachment 244905 [details] [review] Initial port to GStreamer 1.2
This new patch is still a WIP. It makes a simple playbin, with X11 display, works.
Hi Victor, (In reply to comment #3) > Created an attachment (id=244557) [details] [review] > intial port to gstreamer 1.2 api > > I'm starting to work on this. The current patch only enables the compilation of > gstreamer-vaapi with gstreamer HEAD. > > The next step is connect the dots for the context and the texture upload meta. > > Another stage would be the use of caps features. > > __gb__ what do you thing about this approach? That looks like a good plan.
(In reply to comment #6) > Created an attachment (id=244905) [details] [review] > Initial port to GStreamer 1.2 Does !USE_BASEVIDEO with GStreamer 1.0.x have any meaningful use? If not, why don't we just check against Gst 1.2.x? Or do we plan to backport some of those new APIs to 1.0.x at least in non-upstream spaces? i.e. we probably need to just check for gst >= 1.2, then >= 1.0, then the fallback is 0.10.
Hi Gwenole, (In reply to comment #9) > (In reply to comment #6) > > Created an attachment (id=244905) [details] [review] [details] [review] > > Initial port to GStreamer 1.2 > > Does !USE_BASEVIDEO with GStreamer 1.0.x have any meaningful use? If not, why > don't we just check against Gst 1.2.x? Or do we plan to backport some of those > new APIs to 1.0.x at least in non-upstream spaces? > > i.e. we probably need to just check for gst >= 1.2, then >= 1.0, then the > fallback is 0.10. Yes! I'm working in a new patch that use GST_CHECK_VERSION(1,1,0) instead of USE_BASEVIDEO Also I'm exploring a new approach: as gstcompat.h, I'm trying to see if is possible a gstvaapivideocontext adapting the current GstContext to the old GstVideoContext.
(In reply to comment #10) > Hi Gwenole, > > (In reply to comment #9) > > (In reply to comment #6) > > > Created an attachment (id=244905) [details] [review] [details] [review] [details] [review] > > > Initial port to GStreamer 1.2 > > > > Does !USE_BASEVIDEO with GStreamer 1.0.x have any meaningful use? If not, why > > don't we just check against Gst 1.2.x? Or do we plan to backport some of those > > new APIs to 1.0.x at least in non-upstream spaces? > > > > i.e. we probably need to just check for gst >= 1.2, then >= 1.0, then the > > fallback is 0.10. > > Yes! I'm working in a new patch that use GST_CHECK_VERSION(1,1,0) instead of > USE_BASEVIDEO > > Also I'm exploring a new approach: as gstcompat.h, I'm trying to see if is > possible a gstvaapivideocontext adapting the current GstContext to the old > GstVideoContext. Don't spend too much time on it if you can't get this to work simply. The gstcompat or glibcompat thing can only work well because the replacement functions are rather small. Though, if you can get this to work, that's fine too. :) The idea is indeed to use as many new APIs as possible by default to ease transition and integration, with reasonable fallbacks to older APIs. e.g. no serious performance regression. That's because most of the "real" customers out there will stick to 0.10 or 1.0 for now. They would only upgrade if there is a compelling reason for this. e.g. for 0.10 to 1.0 transition, my metrics show that overall the 1.0 based gstreamer-vaapi is ~10% faster than 0.10 based. As in, a full video playback will tend to use 10% less instructions overall to perform the same workload than before. That's an interesting gain.
Created attachment 245176 [details] [review] initial port to GStreamer 1.2 This new patch uses GST_CHECK_VERSION macro, and depends on --with-gstreamer-api=1.2 configuration flag TODO: + fix the context sharing + support GstVideoGLTextureUploadMeta + add caps features + use a x-raw caps relying on GstMemory ???
Created attachment 245982 [details] [review] initial port to GStreamer 1.2
Created attachment 245983 [details] [review] pluginutil: add gst_vaapi_create_display()
Created attachment 245984 [details] [review] pluginutil: add locks This seems to be healthy.
Created attachment 245985 [details] [review] display: add gstcontext utilities declared an use a boxed type for display
Created attachment 245986 [details] [review] libs: add gstvaapivideocontext This is thin compat layer for the deprecated GstVideoContext.
Created attachment 245987 [details] [review] postproc: set context
Created attachment 245988 [details] [review] decode: set context
Created attachment 245989 [details] [review] decode: query context
Created attachment 245990 [details] [review] sink: set context
Created attachment 245991 [details] [review] sink: handle events
Created attachment 245992 [details] [review] sink: handle context query
Created attachment 246164 [details] [review] pluginsutil: GLX first
Created attachment 246165 [details] [review] videobuffer: add GstVideoGLTextureUploadMeta
https://bugzilla.gnome.org/show_bug.cgi?id=687182 is also a dependency.
Created attachment 246629 [details] [review] initial port to GStreamer 1.2
Created attachment 246630 [details] [review] pluginutil: add gst_vaapi_create_display()
Created attachment 246631 [details] [review] pluginutil: add locks This seems to be healthy.
Created attachment 246632 [details] [review] display: add gstcontext utilities declared an use a boxed type for display
Created attachment 246633 [details] [review] libs: add gstvaapivideocontext This is thin compat layer for the deprecated GstVideoContext.
Created attachment 246634 [details] [review] postproc: set context
Created attachment 246635 [details] [review] decode: set context
Created attachment 246636 [details] [review] decode: query context
Created attachment 246637 [details] [review] sink: set context
Created attachment 246638 [details] [review] sink: handle events
Created attachment 246639 [details] [review] sink: handle context query
Created attachment 246640 [details] [review] videobuffer: add GstVideoGLTextureUploadMeta
Created attachment 246725 [details] [review] Add capsfeature support This patch is adding capsfeature support. But for now I have added GST_VIDEO_FORMATS_ALL for the raw video format support.
Hm, it seems that the code is already messing up with lots of VERSION_CHECK macros !!
Hi Ceyusa, Can you please add the similar stuffs to vaapipostproc also ??
Aha, just noticed that you have a patch to vaapipostproc also. Is that working fine for you?
(In reply to comment #42) > Aha, just noticed that you have a patch to vaapipostproc also. Is that working > fine for you? Sorry, I haven't tried postproc at all :/
Review of attachment 246725 [details] [review]: The problem with this patch is that no format negotiation is done, NV12 is hard-coded.
(In reply to comment #44) > Review of attachment 246725 [details] [review]: > > The problem with this patch is that no format negotiation is done, NV12 is > hard-coded. I didn't do anything to handle the format nego with this patch. Just kept the master branch as it is. Yes, we need better ways to handle negotiations. We will get the exact format only after the decoding of first buffer.: Reference: https://bugzilla.gnome.org/show_bug.cgi?id=687183
(In reply to comment #43) > (In reply to comment #42) > > Aha, just noticed that you have a patch to vaapipostproc also. Is that working > > fine for you? > > Sorry, I haven't tried postproc at all :/ It is not working ;) Failing somewhere in context query handling. Didn't debug it but you can reproduce it with any file i guess..
This bug is big and complex, so I will split it, and handle this bug as a meta bug.
Closing this bug since this concerned the initial GStreamer 1.2 support work, and all dependencies are now closed. :)