GNOME Bugzilla – Bug 724352
Plugin should not register if underlaying library not available
Last modified: 2016-07-27 13:46:30 UTC
If the libva-intel-driver package is not installed a autoplugged pipeline will just fail with the following: Could not initialize supporting library. gstvideoencoder.c(1479): gst_video_encoder_change_state (): /GstPipeline:pipeline0/GstEncodeBin:encodebin0/GstVaapiEncodeH264:vaapiencodeh264-0: Failed to open encoder Would be good if we somehow could avoid plugging the vaapi plugin if the needed libva-intel-driver is not installed as it creates a bad user experience for applications.
For the record, as we discussed earlier on IRC, early check for VA capabilities would increase the start-up time. Should this be bearable to other developers/users, then that's fine with me too. Another issue, which does not exist yet, would be that we wouldn't know beforehand what precise display/device to pick. So, we would always pick the default one. This could be an issue if we come to support multiple devices. Anyway, the first step would be to split the macro vaapidecode element first, as referenced by bug #734093
It would be interesting to experiment with GstDeviceProvider[1] to create the vaapielements "in-runtime", depending on the available display/device. 1. http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gstreamer-GstDeviceProvider.html
Moving to Product:GStreamer, Component:gstreamer-vaapi
Last hackfest I talked with ndufresne about the registry cache and he told me about some dependency handling for the cache. Just today I look for it and found this beauty: gst_plugin_add_dependency () https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstPlugin.html#gst-plugin-add-dependency With it we can invalidate the registry cache if a file, a directory or a envvar changes. Therefore, we could monitor the diretories /dev, /usr/lib/dri, and the LIBVA_* envvars to invalidate the registry cache if the change, thus we could register what libva reports as available, and not registering all as we currently do. In this way we could reduce the number bugs raised because of the limitation of the hardware/driver of the user, or not register anything if the introspection fails.
Note though, if you can limit /dev deps as much as possible, it's nicer, because /dev are created at boot time. If not, let it be, I guess with all those PCI / USB stuff, you can never be sure and need to probe once per boot.
Created attachment 330560 [details] [review] vaapi: declare external dependencies There are two main external dependencies that define the feature set of this plugin: a) the kernel and b) the VA driver This patch tracks both dependencies, if any of them change, GStreamer will re-inspect the plugin. The kernel is tracked through the device files /dev/dri/card* The VA driver is tracked through the files VA_DRIVERS_PATH/*_drv_video.so, where VA_DRIVERS_PATH is the one defined in libva package configuration. Also, the environment variables LIBVA_DRIVERS_PATH and LIBVA_DRIVER_NAME are tracked since they modify the driver lookup. Additionally, the environment variable GST_VAAPI_ALL_DRIVERS is tracked too.
Created attachment 330561 [details] [review] vaapi: register according to a test VA display In order to register only the available elements, this patch creates a test VA display, using the currently used back-end (X11, Wayland, DRM, …) on the used display device, it is queried and given the response the available feature set in plugin are registered. This patch depends on a patch in bug 734093 (vaapidecode: allow for per-codec element split) which completes the codec split.
Created attachment 330562 [details] [review] plugins: add gst_vaapi_driver_is_whitelisted() Move some of the logic in gst_vaapi_plugin_base_driver_is_whitelisted() to a new function gst_vaapi_driver_is_whitelisted(), in this way, it can be used when registering the plugin's feature set with the test VA display.
Created attachment 330563 [details] [review] vaapi: don't register if VA driver is unsupported Using the test VA display, the driver name is queried, and if it is not white-listed, the plugin rejects to register any element.
I have done some testing with these patches: Pros: 1\ running gst-inspect the test display is created only once and the registry cache remains until the dependency changes. 1.1\ Using /dev/dri/card* as a dependency, every reboot will invalidate the registry cache. 2\ vaapidecodebin can be simplified since we already know if the driver supports postprocessing 3\ The user knows what can they use without asking for a vainfo report Cons: 1\ Every time a pipeline is created, the plugin features are checked, hence a VA display is instantiated twice: one for this plugin initialization and later for the pipeline usage. I don't like this and I didn't expect it.
Created attachment 331474 [details] [review] vaapi: declare external dependencies There are two main external dependencies that define the feature set of this plugin: a) the kernel and b) the VA driver This patch tracks both dependencies, if any of them change, GStreamer will re-inspect the plugin. The kernel is tracked through the device files /dev/dri/card* The VA driver is tracked through the files VA_DRIVERS_PATH/*_drv_video.so, where VA_DRIVERS_PATH is the one defined in libva package configuration. Also, the environment variables LIBVA_DRIVERS_PATH and LIBVA_DRIVER_NAME are tracked since they modify the driver lookup. Additionally, the environment variable GST_VAAPI_ALL_DRIVERS is tracked too.
Created attachment 331475 [details] [review] vaapi: instantiate a VA display when registering This patch tries to instantiate a GstVaapiDisplay when registering the plugin features, if it fails, no gstreamer-vaapi element is registering. The purpose of this patch is to avoid a situation where the user has gstreamer-vaapi installed but their VA-API setup is not functional, which may lead to unexpected behavior.
Created attachment 331476 [details] [review] plugins: add gst_vaapi_driver_is_whitelisted() Move some of the logic in gst_vaapi_plugin_base_driver_is_whitelisted() to a new function gst_vaapi_driver_is_whitelisted(), in this way, it can be used when registering the plugin's feature set with the test VA display.
Created attachment 331477 [details] [review] vaapi: don't register if VA driver is unsupported Using the test VA display, the driver name is queried, and if it is not white-listed, the plugin rejects to register any element.
Created attachment 331478 [details] [review] vaapi: register vaapipostproc only if supported Query the GstVaapiDisplay to know if the driver supports video postprocessing. If does, then register vaapipostproc and vaapidecodebin elements. This patch will simplify the design of vaapidecodebin.
Created attachment 331486 [details] [review] vaapi: register only the available encoders In order to register only the available encoders, this patch queries the created test VA display, which uses the currently used back-end (X11, Wayland, DRM, …) on the used display device.
Created attachment 331487 [details] [review] vaapidecode: register only the available decoders In order to register only the available decoders, this patch queries the created test VA display, which uses the currently used back-end (X11, Wayland, DRM, …) on the used display device.
You may want to add a call to gst_plugin_add_dependency_simple() with the path where the vaapi plugin are installed, to make GStreamer re-try if a new one is installed.
(In reply to Olivier Crête from comment #18) > You may want to add a call to gst_plugin_add_dependency_simple() with the > path where the vaapi plugin are installed, to make GStreamer re-try if a new > one is installed. Good catch! I didn't realized that va_openDrive iterates over a driver_dir string separated by ':'
Created attachment 331601 [details] [review] vaapi: declare external dependencies There are two main external dependencies that define the feature set of this plugin: a) the kernel and b) the VA driver This patch tracks both dependencies, if any of them change, GStreamer will re-inspect the plugin. The kernel is tracked through the device files /dev/dri/card* The VA driver is tracked through the files VA_DRIVERS_PATH/*_drv_video.so, where VA_DRIVERS_PATH is the one defined in libva package configuration. Also, the environment variables LIBVA_DRIVERS_PATH and LIBVA_DRIVER_NAME are tracked since they modify the driver lookup. Additionally, the environment variable GST_VAAPI_ALL_DRIVERS is tracked too.
Ptches lgtm.. :)
Created attachment 331758 [details] [review] vaapi: instantiate a VA display when registering This patch tries to instantiate a GstVaapiDisplay when registering the plugin features, if it fails, no gstreamer-vaapi element is registering. The purpose of this patch is to avoid a situation where the user has gstreamer-vaapi installed but their VA-API setup is not functional, which may lead to unexpected behavior.
Created attachment 331759 [details] [review] vaapi: register only the available encoders In order to register only the available encoders, this patch queries the created test VA display, which uses the currently used back-end (X11, Wayland, DRM, …) on the used display device.
Created attachment 331760 [details] [review] vaapidecode: register only the available decoders In order to register only the available decoders, this patch queries the created test VA display, which uses the currently used back-end (X11, Wayland, DRM, …) on the used display device.
Attachment 331476 [details] pushed as a9e7eac - plugins: add gst_vaapi_driver_is_whitelisted() Attachment 331477 [details] pushed as 90b0ba7 - vaapi: don't register if VA driver is unsupported Attachment 331478 [details] pushed as c60312c - vaapi: register vaapipostproc only if supported Attachment 331601 [details] pushed as 1cba2f3 - vaapi: declare external dependencies Attachment 331758 [details] pushed as e070d3e - vaapi: instantiate a VA display when registering Attachment 331759 [details] pushed as eb621c9 - vaapi: register only the available encoders Attachment 331760 [details] pushed as 5beccf5 - vaapidecode: register only the available decoders