On Thu, 5 Apr 2018 20:57:46 +0300
Tapani Pälli <tapani.pa...@intel.com> wrote:
> 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.
>
You are right it's already been checked in the upper level. I will
remove it.
>
> >>
> >>> 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
