On 9 November 2016 at 08:33, Tomasz Figa <[email protected]> wrote: > There is an interface that can be used to query YUV buffers for their > internal format. Specifically, if gralloc:lock_ycbcr() is given no SW > usage flags, it's supposed to return plane offsets instead of pointers. > Let's use this interface to implement support for YUV formats in Android > EGL backend. > gralloc:lock_ycbcr seems to be missing from gbm_gralloc. Seems trivial to implement - one can even apply the drm_gralloc commit a43d9523c050b3ddb40190bb8de7920b7521b6ca directly ;-)
> Signed-off-by: Tomasz Figa <[email protected]> > --- > src/egl/drivers/dri2/platform_android.c | 146 > ++++++++++++++++++++++++++------ > 1 file changed, 120 insertions(+), 26 deletions(-) > > diff --git a/src/egl/drivers/dri2/platform_android.c > b/src/egl/drivers/dri2/platform_android.c > index 0b59724..696a22f 100644 > --- a/src/egl/drivers/dri2/platform_android.c > +++ b/src/egl/drivers/dri2/platform_android.c > @@ -43,6 +43,49 @@ > > #define ALIGN(val, align) (((val) + (align) - 1) & ~((align) - 1)) > > +struct droid_yuv_format { > + /* Lookup keys */ > + int native; > + int is_ycrcb; > + int chroma_step; > + > + /* Result */ > + int fourcc; > +}; > + > +static const struct droid_yuv_format droid_yuv_formats[] = { > + { HAL_PIXEL_FORMAT_YCbCr_420_888, 0, 2, __DRI_IMAGE_FOURCC_NV12 }, > + { HAL_PIXEL_FORMAT_YCbCr_420_888, 0, 1, __DRI_IMAGE_FOURCC_YUV420 }, > + { HAL_PIXEL_FORMAT_YCbCr_420_888, 1, 1, __DRI_IMAGE_FOURCC_YVU420 }, > + { HAL_PIXEL_FORMAT_YV12, 1, 1, __DRI_IMAGE_FOURCC_YVU420 }, > +}; > + This table here feels a bit magical. Please add a comment explaining things. > +static int > +get_fourcc_yuv(int native, int is_ycrcb, int chroma_step) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(droid_yuv_formats); ++i) > + if (droid_yuv_formats[i].native == native && > + droid_yuv_formats[i].is_ycrcb == is_ycrcb && > + droid_yuv_formats[i].chroma_step == chroma_step) > + return droid_yuv_formats[i].fourcc; > + > + return 0; get_fourcc() uses -1 for error/not-found. Can we have both functions behave identically - don't mind each way. > +} > + > +static int We haven't used it too often - yet "bool" would be nice. > +is_yuv(int native) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(droid_yuv_formats); ++i) > + if (droid_yuv_formats[i].native == native) > + return 1; > + > + return 0; > +} > + > static int > get_format_bpp(int native) > { > @@ -57,9 +100,6 @@ get_format_bpp(int native) > case HAL_PIXEL_FORMAT_RGB_565: > bpp = 2; > break; > - case HAL_PIXEL_FORMAT_YV12: > - bpp = 1; > - break; > default: > bpp = 0; > break; > @@ -76,7 +116,6 @@ static int get_fourcc(int native) > case HAL_PIXEL_FORMAT_BGRA_8888: return __DRI_IMAGE_FOURCC_ARGB8888; > case HAL_PIXEL_FORMAT_RGBA_8888: return __DRI_IMAGE_FOURCC_ABGR8888; > case HAL_PIXEL_FORMAT_RGBX_8888: return __DRI_IMAGE_FOURCC_XBGR8888; > - case HAL_PIXEL_FORMAT_YV12: return __DRI_IMAGE_FOURCC_YVU420; > default: > _eglLog(_EGL_WARNING, "unsupported native buffer format 0x%x", native); > } > @@ -479,35 +518,68 @@ droid_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, > _EGLSurface *draw) > } > > static _EGLImage * > -droid_create_image_from_prime_fd(_EGLDisplay *disp, _EGLContext *ctx, > - struct ANativeWindowBuffer *buf, int fd) > +droid_create_image_from_prime_fd_yuv(_EGLDisplay *disp, _EGLContext *ctx, > + struct ANativeWindowBuffer *buf, int fd) > { > - unsigned int offsets[3] = { 0, 0, 0 }; > - unsigned int pitches[3] = { 0, 0, 0 }; > + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); > + struct android_ycbcr ycbcr; > + size_t offsets[3]; > + size_t pitches[3]; > + int is_ycrcb; > + int fourcc; > + int ret; > > - const int fourcc = get_fourcc(buf->format); > - if (fourcc == -1) { > - _eglError(EGL_BAD_PARAMETER, "eglCreateEGLImageKHR"); > + if (!dri2_dpy->gralloc->lock_ycbcr) { > + _eglLog(_EGL_WARNING, "Gralloc does not support lock_ycbcr"); > return NULL; > } > > - pitches[0] = buf->stride * get_format_bpp(buf->format); > - if (pitches[0] == 0) { > - _eglError(EGL_BAD_PARAMETER, "eglCreateEGLImageKHR"); > + memset(&ycbcr, 0, sizeof(ycbcr)); > + ret = dri2_dpy->gralloc->lock_ycbcr(dri2_dpy->gralloc, buf->handle, > + 0, 0, 0, 0, 0, &ycbcr); > + if (ret) { > + _eglLog(_EGL_WARNING, "gralloc->lock_ycbcr failed: %d", ret); > return NULL; > } > + dri2_dpy->gralloc->unlock(dri2_dpy->gralloc, buf->handle); > + > + offsets[0] = (size_t)ycbcr.y; > + is_ycrcb = (size_t)ycbcr.cb < (size_t)ycbcr.cr; > + if (is_ycrcb) { > + offsets[1] = (size_t)ycbcr.cr; > + offsets[2] = (size_t)ycbcr.cb; > + } else { > + offsets[1] = (size_t)ycbcr.cb; > + offsets[2] = (size_t)ycbcr.cr; > + } > > - switch (buf->format) { > - case HAL_PIXEL_FORMAT_YV12: > - /* Y plane is assumed to be at offset 0. */ > - /* Cr plane is located after Y plane */ > - offsets[1] = offsets[0] + pitches[0] * buf->height; > - pitches[1] = ALIGN(pitches[0] / 2, 16); > - /* Cb plane is located after Cr plane */ > - offsets[2] = offsets[1] + pitches[1] * buf->height / 2; > - pitches[2] = pitches[1]; > + pitches[0] = ycbcr.ystride; > + pitches[1] = pitches[2] = ycbcr.cstride; > Going through the system/graphics.h description @cstride is the stride of the chroma planes I'd imagine that one should interpret it as: @cstride is a) the total stride of the chroma planes when interleaved or b) the individual stride of either chroma plane when planar. Doesn't Android support planar YUV 4:2:2/YUV 4:1:1 formats ? If it only does HAL_PIXEL_FORMAT_YV12 where the Cr Cb strides are the same that would explain "either chroma plane" above. Can we add a comment about what cstride is and how it works in this case ? Mildly related: while skimming through hardware/gralloc.h I've noticed a duplicate hardware/hardware.h include ;-) Considering I understood things correctly - with the above trivial suggestions the patch is Reviewed-by: Emil Velikov <[email protected]> Emil _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
