From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t 07/10] lib: Use igt_matrix for ycbcr<->rgb conversion
Date: Fri, 18 May 2018 15:41:49 +0300 [thread overview]
Message-ID: <20180518124149.GT23723@intel.com> (raw)
In-Reply-To: <152659377239.4210.2233447133518897636@mail.alporthouse.com>
On Thu, May 17, 2018 at 10:49:32PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-05-17 20:51:45)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Probably horribly inefficient (not that the original code tried to be
> > particularly efficient), but at least this is now pretty generic so
> > it'll be super easy to add other color encodings and whatnot.
> >
> > v2: Rebase
> > v3: Deal with the new color_encoding/range enums
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > lib/igt_fb.c | 134 +++++++++++++++++++++++++++++------------------------------
> > 1 file changed, 65 insertions(+), 69 deletions(-)
> >
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index af8ba70b0a2a..8aea5b52c8a9 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -31,8 +31,10 @@
> >
> > #include "drmtest.h"
> > #include "igt_aux.h"
> > +#include "igt_color_encoding.h"
> > #include "igt_fb.h"
> > #include "igt_kms.h"
> > +#include "igt_matrix.h"
> > #include "igt_x86.h"
> > #include "ioctl_wrappers.h"
> > #include "intel_batchbuffer.h"
> > @@ -1377,6 +1379,8 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> > uint8_t *rgb24 = blit->rgb24.map;
> > unsigned rgb24_stride = blit->rgb24.stride, planar_stride = blit->linear.stride;
> > uint8_t *buf = malloc(blit->linear.size);
> > + struct igt_mat4 m = igt_ycbcr_to_rgb_matrix(IGT_COLOR_YCBCR_BT601,
> > + IGT_COLOR_YCBCR_LIMITED_RANGE);
> >
> > /*
> > * Reading from the BO is awfully slow because of lack of read caching,
> > @@ -1387,30 +1391,29 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> > y = &buf[blit->linear.offsets[0]];
> > uv = &buf[blit->linear.offsets[1]];
> >
> > - /* Convert from limited color range BT.601 */
> > for (i = 0; i < fb->height / 2; i++) {
> > for (j = 0; j < fb->width; j++) {
> > - float r_, g_, b_, y0, y1, cb, cr;
> > /* Convert 1x2 pixel blocks */
> > + struct igt_vec4 yuv0, yuv1;
> > + struct igt_vec4 rgb0, rgb1;
> >
> > - y0 = 1.164f * (y[j] - 16.f);
> > - y1 = 1.164f * (y[j + planar_stride] - 16.f);
> > + yuv0.d[0] = y[j];
> > + yuv1.d[0] = y[j + planar_stride];
> > + yuv0.d[1] = yuv1.d[1] = uv[j & ~1];
> > + yuv0.d[2] = yuv1.d[2] = uv[j | 1];
> > + yuv0.d[3] = yuv1.d[3] = 1.0f;
> >
> > - cb = uv[j & ~1] - 128.f;
> > - cr = uv[j | 1] - 128.f;
> > + rgb0 = igt_matrix_transform(&m, &yuv0);
> > + rgb1 = igt_matrix_transform(&m, &yuv1);
>
> Ok.
>
> > - r_ = 0.000f * cb + 1.596f * cr;
> > - g_ = -0.392f * cb + -0.813f * cr;
> > - b_ = 2.017f * cb + 0.000f * cr;
> > + rgb24[j * 4 + 2] = clamprgb(rgb0.d[0]);
> > + rgb24[j * 4 + 2 + rgb24_stride] = clamprgb(rgb1.d[0]);
> >
> > - rgb24[j * 4 + 2] = clamprgb(y0 + r_);
> > - rgb24[j * 4 + 2 + rgb24_stride] = clamprgb(y1 + r_);
> > + rgb24[j * 4 + 1] = clamprgb(rgb0.d[1]);
> > + rgb24[j * 4 + 1 + rgb24_stride] = clamprgb(rgb1.d[1]);
> >
> > - rgb24[j * 4 + 1] = clamprgb(y0 + g_);
> > - rgb24[j * 4 + 1 + rgb24_stride] = clamprgb(y1 + g_);
> > -
> > - rgb24[j * 4] = clamprgb(y0 + b_);
> > - rgb24[j * 4 + rgb24_stride] = clamprgb(y1 + b_);
> > + rgb24[j * 4 + 0] = clamprgb(rgb0.d[2]);
> > + rgb24[j * 4 + 0 + rgb24_stride] = clamprgb(rgb1.d[2]);
>
> Ok.
>
> > }
> >
> > rgb24 += 2 * rgb24_stride;
> > @@ -1421,20 +1424,20 @@ static void convert_nv12_to_rgb24(struct igt_fb *fb, struct fb_convert_blit_uplo
> > if (fb->height & 1) {
> > /* Convert last row */
> > for (j = 0; j < fb->width; j++) {
> > - float r_, g_, b_, y0, cb, cr;
> > /* Convert single pixel */
> > + struct igt_vec4 yuv;
> > + struct igt_vec4 rgb;
> >
> > - cb = uv[j & ~1] - 128.f;
> > - cr = uv[j | 1] - 128.f;
> > + yuv.d[0] = y[j];
> > + yuv.d[1] = uv[j & ~1];
> > + yuv.d[2] = uv[j | 1];
> > + yuv.d[3] = 1.0f;
> >
> > - y0 = 1.164f * (y[j] - 16.f);
> > - r_ = 0.000f * cb + 1.596f * cr;
> > - g_ = -0.392f * cb + -0.813f * cr;
> > - b_ = 2.017f * cb + 0.000f * cr;
> > + rgb = igt_matrix_transform(&m, &yuv);
> >
> > - rgb24[j * 4 + 2] = clamprgb(y0 + r_);
> > - rgb24[j * 4 + 1] = clamprgb(y0 + g_);
> > - rgb24[j * 4] = clamprgb(y0 + b_);
> > + rgb24[j * 4 + 2] = clamprgb(rgb.d[0]);
> > + rgb24[j * 4 + 1] = clamprgb(rgb.d[1]);
> > + rgb24[j * 4] = clamprgb(rgb.d[2]);
>
> rgb24[j * 4 + 2] = clamprgb(rgb.d[0]);
> rgb24[j * 4 + 1] = clamprgb(rgb.d[1]);
> rgb24[j * 4 + 0] = clamprgb(rgb.d[2]);
> ?
Sure. Looks less messy that way.
>
> > }
> > }
> >
> > @@ -1449,65 +1452,58 @@ static void convert_rgb24_to_nv12(struct igt_fb *fb, struct fb_convert_blit_uplo
> > const uint8_t *rgb24 = blit->rgb24.map;
> > unsigned rgb24_stride = blit->rgb24.stride;
> > unsigned planar_stride = blit->linear.stride;
> > + struct igt_mat4 m = igt_rgb_to_ycbcr_matrix(IGT_COLOR_YCBCR_BT601,
> > + IGT_COLOR_YCBCR_LIMITED_RANGE);
> >
> > igt_assert_f(fb->drm_format == DRM_FORMAT_NV12,
> > "Conversion not implemented for !NV12 planar formats\n");
> >
> > - for (i = 0; i < fb->plane_height[0]; i++) {
> > - /* Use limited color range BT.601 */
> > + for (i = 0; i < fb->height / 2; i++) {
> > + for (j = 0; j < fb->width; j++) {
> > + struct igt_vec4 rgb0, rgb1;
> > + struct igt_vec4 yuv0, yuv1;
> >
> > - for (j = 0; j < fb->plane_width[0]; j++) {
> > - float yf = 0.257f * rgb24[j * 4 + 2] +
> > - 0.504f * rgb24[j * 4 + 1] +
> > - 0.098f * rgb24[j * 4] + 16;
> > + rgb0.d[0] = rgb24[j * 4 + 2];
> > + rgb0.d[1] = rgb24[j * 4 + 1];
> > + rgb0.d[2] = rgb24[j * 4 + 0];
> > + rgb0.d[3] = 1.0f;
> >
> > - y[j] = (uint8_t)yf;
> > - }
> > + rgb1.d[0] = rgb24[j * 4 + 2 + rgb24_stride];
> > + rgb1.d[1] = rgb24[j * 4 + 1 + rgb24_stride];
> > + rgb1.d[2] = rgb24[j * 4 + 0 + rgb24_stride];
> > + rgb1.d[3] = 1.0f;
> >
> > - rgb24 += rgb24_stride;
> > - y += planar_stride;
> > - }
> > + yuv0 = igt_matrix_transform(&m, &rgb0);
> > + yuv1 = igt_matrix_transform(&m, &rgb1);
> >
> > - rgb24 = blit->rgb24.map;
> > + y[j] = yuv0.d[0];
> > + y[j + planar_stride] = yuv1.d[0];
> >
> > - for (i = 0; i < fb->height / 2; i++) {
> > - for (j = 0; j < fb->plane_width[1]; j++) {
> > - /*
> > - * Pixel center for Cb'Cr' is between the left top and
> > - * bottom pixel in a 2x2 block, so take the average.
> > - */
> > - float uf = -0.148f/2 * rgb24[j * 8 + 2] +
> > - -0.148f/2 * rgb24[j * 8 + 2 + rgb24_stride] +
> > - -0.291f/2 * rgb24[j * 8 + 1] +
> > - -0.291f/2 * rgb24[j * 8 + 1 + rgb24_stride] +
> > - 0.439f/2 * rgb24[j * 8] +
> > - 0.439f/2 * rgb24[j * 8 + rgb24_stride] + 128;
> > - float vf = 0.439f/2 * rgb24[j * 8 + 2] +
> > - 0.439f/2 * rgb24[j * 8 + 2 + rgb24_stride] +
> > - -0.368f/2 * rgb24[j * 8 + 1] +
> > - -0.368f/2 * rgb24[j * 8 + 1 + rgb24_stride] +
> > - -0.071f/2 * rgb24[j * 8] +
> > - -0.071f/2 * rgb24[j * 8 + rgb24_stride] + 128;
> > - uv[j * 2] = (uint8_t)uf;
> > - uv[j * 2 + 1] = (uint8_t)vf;
> > + uv[j * 2 + 0] = (yuv0.d[1] + yuv1.d[1]) / 2.0f;
> > + uv[j * 2 + 1] = (yuv0.d[2] + yuv1.d[2]) / 2.0f;
>
> Keep the comment about averaging tl+br. Why only 2 points btw?
Lazy I guess. Hmm. Somehow this doesn't even look right to me now. I
think I'll have to actually figure out what I was doing here, and test
it on actual hardware.
>
> > }
> >
> > rgb24 += 2 * rgb24_stride;
> > + y += 2 * planar_stride;
> > uv += planar_stride;
> > }
> >
> > /* Last row cannot be interpolated between 2 pixels, take the single value */
> > - if (i < fb->plane_height[1]) {
> > - for (j = 0; j < fb->plane_width[1]; j++) {
> > - float uf = -0.148f * rgb24[j * 8 + 2] +
> > - -0.291f * rgb24[j * 8 + 1] +
> > - 0.439f * rgb24[j * 8] + 128;
> > - float vf = 0.439f * rgb24[j * 8 + 2] +
> > - -0.368f * rgb24[j * 8 + 1] +
> > - -0.071f * rgb24[j * 8] + 128;
> > -
> > - uv[j * 2] = (uint8_t)uf;
> > - uv[j * 2 + 1] = (uint8_t)vf;
> > + if (fb->height & 1) {
> > + for (j = 0; j < fb->width; j++) {
> > + struct igt_vec4 rgb;
> > + struct igt_vec4 yuv;
> > +
> > + rgb.d[0] = rgb24[j * 4 + 2];
> > + rgb.d[1] = rgb24[j * 4 + 1];
> > + rgb.d[2] = rgb24[j * 4 + 0];
> > + rgb.d[3] = 1.0f;
> > +
> > + yuv = igt_matrix_transform(&m, &rgb);
> > +
> > + y[j] = yuv.d[0];
> > + uv[j * 2 + 0] = yuv.d[1];
> > + uv[j * 2 + 1] = yuv.d[2];
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
--
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
next prev parent reply other threads:[~2018-05-18 12:41 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-17 19:51 [igt-dev] [PATCH i-g-t 01/10] lib: Add clamp() to igt_aux and use it for ycbcr<->rgb conversion Ville Syrjala
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 02/10] lib: Clear packed YUV formats to black Ville Syrjala
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 03/10] lib: Don't use dumb buffers for YCbCr Ville Syrjala
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 04/10] lib: Clean up format_desc Ville Syrjala
2018-05-17 20:13 ` Chris Wilson
2018-05-18 12:42 ` Ville Syrjälä
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 05/10] lib: Add igt_matrix Ville Syrjala
2018-05-17 20:16 ` Chris Wilson
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 06/10] lib: Add igt_color_encoding Ville Syrjala
2018-05-17 20:48 ` Chris Wilson
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 07/10] lib: Use igt_matrix for ycbcr<->rgb conversion Ville Syrjala
2018-05-17 21:49 ` Chris Wilson
2018-05-18 12:41 ` Ville Syrjälä [this message]
2018-05-18 19:56 ` [igt-dev] [PATCH i-g-t v4 " Ville Syrjala
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 08/10] lib: Add support for rendering into packed YCbCr framebuffers Ville Syrjala
2018-05-17 22:41 ` Chris Wilson
2018-05-18 13:00 ` Ville Syrjälä
2018-05-18 13:17 ` Chris Wilson
2018-05-18 19:57 ` [igt-dev] [PATCH i-g-t v5 " Ville Syrjala
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 09/10] lib/fb: Add color_encoding/color_range to igt_fb Ville Syrjala
2018-05-17 21:42 ` Chris Wilson
2018-05-17 19:51 ` [igt-dev] [PATCH i-g-t 10/10] lib/kms: Respect fb color_encoding/color_range Ville Syrjala
2018-05-17 20:09 ` [igt-dev] [PATCH i-g-t 01/10] lib: Add clamp() to igt_aux and use it for ycbcr<->rgb conversion Chris Wilson
2018-05-17 20:29 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,01/10] " Patchwork
2018-05-17 23:38 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
2018-05-18 20:30 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,01/10] lib: Add clamp() to igt_aux and use it for ycbcr<->rgb conversion (rev3) Patchwork
2018-05-19 6:30 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180518124149.GT23723@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=igt-dev@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox