On 05.04.2018 18:43, James Xiong wrote:
On Thu, 5 Apr 2018 14:30:02 +0300
Tapani Pälli <tapani.pa...@intel.com> wrote:

On 04/05/2018 02:51 AM, James Xiong wrote:
From: "Xiong, James" <james.xi...@intel.com>

The planar_format in __DRIimage contains the original fourcc
used to create the image, if it's set, return the saved fourcc
directly; Otherwise fall back to the old way.

Also we should validate the input parameter "value" first as it
might be NULL based on the SPEC.

v2: fall back to intel_lookup_fourcc() when planar_format is NULL
    (by Dongwon & Matt Roper)

Signed-off-by: Xiong, James <james.xi...@intel.com>
---
   src/mesa/drivers/dri/i965/intel_screen.c | 15 ++++++++++++---
   1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
b/src/mesa/drivers/dri/i965/intel_screen.c index 7df8bc4..aeecef3
100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -388,10 +388,16 @@ intel_image_format_lookup(int fourcc)
      return NULL;
   }
-static boolean intel_lookup_fourcc(int dri_format, int *fourcc)
+static boolean
+intel_image_get_fourcc(__DRIimage *image, int *fourcc)
   {
+   if (image->planar_format) {
+      *fourcc = image->planar_format->fourcc;
+      return true;
+   }
+
      for (unsigned i = 0; i < ARRAY_SIZE(intel_image_formats); i++)
{
-      if (intel_image_formats[i].planes[0].dri_format ==
dri_format) {
+      if (intel_image_formats[i].planes[0].dri_format ==
image->dri_format) { *fourcc = intel_image_formats[i].fourcc;
            return true;
         }
@@ -844,6 +850,9 @@ intel_create_image_with_modifiers(__DRIscreen
*dri_screen, static GLboolean
   intel_query_image(__DRIimage *image, int attrib, int *value)
   {
+   if (value == NULL)
+      return false;
+

I would remove this check, we've been fine many years without it.
The function spec does say: "<fourcc>, <num_planes> and <modifiers> may
be NULL, in which case no value is retrieved."
it's better to stick to it and have an extra check than
segmentation fault, what do you say?

Function modified here 'intel_query_image' is part of DRI interface (dri_interface.h) with no such spec, it is used from many existing places and none of them seem to be passing NULL.

Now that I know you are using eglExportDMABUFImageMESA, I can also see that dri2_export_dma_buf_image_mesa implementation does not call this API when 'fds', 'strides' or 'offsets' is NULL, it checks this in the implementation before calling.



      switch (attrib) {
      case __DRI_IMAGE_ATTRIB_STRIDE:
         *value = image->pitch;
@@ -870,7 +879,7 @@ intel_query_image(__DRIimage *image, int
attrib, int *value) case __DRI_IMAGE_ATTRIB_FD:
         return !brw_bo_gem_export_to_prime(image->bo, value);
      case __DRI_IMAGE_ATTRIB_FOURCC:
-      return intel_lookup_fourcc(image->dri_format, value);
+      return intel_image_get_fourcc(image, value);
      case __DRI_IMAGE_ATTRIB_NUM_PLANES:
         if (isl_drm_modifier_has_aux(image->modifier)) {
            assert(!image->planar_format ||
image->planar_format->nplanes == 1);


// Tapani
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to