GNOME Bugzilla – Bug 732691
vaapivideomemory: Fix allocator's surfacepool format caculation
Last modified: 2014-07-23 16:50:53 UTC
Created attachment 279844 [details] [review] vaapivideomemory: Fix allocator's surfacepool format caculation While creating the vaapi video allocator, make sure that the associated surface pool has correct format instead of defaulting to NV12 video format even though there is no direct rendering support.
We could create a GstvaapiImage with desired format(format of the surface), then get the image data from the surface with surface_get_image, and then update the allocator->surface_info from this new image by gst_video_info_update_from_image(). This could be the right way since the surface might have created with a different plane/stride offset by the driver. right?
Review of attachment 279844 [details] [review]: Could you please indicate how is the fixed intended to be tested? :) Thanks. I would like QA to add a test for it. ::: gst/vaapi/gstvaapivideomemory.c @@ +528,3 @@ + gst_video_info_set_format(&allocator->surface_info, format, + width, height); + } I think we could just move up guint width, height; and make them = GST_VIDEO_INFO_{WIDTH,HEIGHT}(vip).
It is difficult to test this ;) This code path will be active only in the following scenario, -- try to create a new surface with a specific videoformat. -- driver failing to create surface in this format choosing a different format in same chromatype. (in this case surface_format will be always == GST_VIDEO_FORMAT_ENCODED) -- gst_vaapi_surface_derive_image() should be successful. -- but it won't support direct rendering because of different i/p and o/p formats. I think there is a better way to fix this. I will add the patch But yes, since it won't occur in usual cases, itz up to you to commit or not :) This just came to my mind while reading the code.
Created attachment 279850 [details] [review] vaapivideomemory: Fix allocator's surfacepool format caculation
Review of attachment 279850 [details] [review]: ::: gst/vaapi/gstvaapivideomemory.c @@ +513,3 @@ &allocator->surface_info, image); + if (GST_VAAPI_IMAGE_FORMAT(image) != GST_VIDEO_INFO_FORMAT(vip)) + allocator->has_direct_rendering = FALSE; has_direct_rendering = update_from_image() && image-format == video-format looks more intuitive.
commit 9cb3acc813320aad072672296b5af168badf82a0 Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com> Date: Thu Jul 3 18:41:11 2014 +0300 vaapivideomemory: fix determination of the surface pool format. While creating the vaapi video allocator, make sure the associated surface pool has correct format instead of defaulting to NV12 video format even though there is no direct rendering support. https://bugzilla.gnome.org/show_bug.cgi?id=732691