On 10 November 2016 at 07:13, Tomasz Figa <[email protected]> wrote: > On Thu, Nov 10, 2016 at 1:50 PM, Tomasz Figa <[email protected]> wrote: >> On Thu, Nov 10, 2016 at 5:14 AM, Emil Velikov <[email protected]> >> wrote: >>> 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 >> >> There are also few lines above: >> >> * Stride describes the distance in bytes from the first value of one row of >> * the image to the first value of the next row. It includes the width of >> the >> * image plus padding. >> >>> >>> 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. >> >> Yeah, that's sounds right. >> >>> >>> 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. > > Actually, do we really have any format where Cb and Cr planes are > different? The formats I've worked with always had symmetric Cb and Cr > planes, i.e. cstride == cbstride == crstride. This holds true even for > interleaved formats, since the distance between first pixels of two > subsequent lines is always the same, regardless of whether we refer to > the Cr or Cb byte. > Grr right - the chroma stride is always the same. Thanks
Emil _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
