[Mesa-dev] [PATCH 1/4] vl: Apply luma key filter before CSC conversion
Christian König
deathsimple at vodafone.de
Tue Jun 7 11:49:59 UTC 2016
Am 07.06.2016 um 13:28 schrieb Nayan Deshmukh:
> Apply the luma key filter to the YCbCr values during the CSC conversion
> in video buffer shader. The initial values of max and min luma are set
> to opposite values to disable the filter initially and will be set when
> enabling it.
>
> Signed-off-by: Nayan Deshmukh <nayan26deshmukh at gmail.com>
Really nice that you got that working.
First of all squash patches #1,#2 and #3 together into one. A single
patch should never break the build of other components.
A few more notes on the code below:
> ---
> src/gallium/auxiliary/vl/vl_compositor.c | 40 ++++++++++++++++++++------------
> src/gallium/auxiliary/vl/vl_compositor.h | 2 +-
> 2 files changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/src/gallium/auxiliary/vl/vl_compositor.c b/src/gallium/auxiliary/vl/vl_compositor.c
> index acb2f4f..05c2d5a 100644
> --- a/src/gallium/auxiliary/vl/vl_compositor.c
> +++ b/src/gallium/auxiliary/vl/vl_compositor.c
> @@ -130,8 +130,9 @@ create_frag_shader_video_buffer(struct vl_compositor *c)
> {
> struct ureg_program *shader;
> struct ureg_src tc;
> - struct ureg_src csc[3];
> + struct ureg_src csc[4];
Better give the luma key parameters its own variable, cause it's not
really part of the CSC matrix. Something like:
+ struct ureg_src lumakey;
> struct ureg_src sampler[3];
> + struct ureg_dst t_tex[3];
Maybe call that temp or something like this.
> struct ureg_dst texel;
> struct ureg_dst fragment;
> unsigned i;
> @@ -142,9 +143,13 @@ create_frag_shader_video_buffer(struct vl_compositor *c)
>
> tc = ureg_DECL_fs_input(shader, TGSI_SEMANTIC_GENERIC, VS_O_VTEX, TGSI_INTERPOLATE_LINEAR);
> for (i = 0; i < 3; ++i) {
> - csc[i] = ureg_DECL_constant(shader, i);
> sampler[i] = ureg_DECL_sampler(shader, i);
> + t_tex[i] = ureg_DECL_temporary(shader);
> }
> +
> + for (i = 0; i < 4; ++i)
> + csc[i] = ureg_DECL_constant(shader, i);
> +
Then this change becomes:
lumakey = uref_DECL_constant(shader, 3);
> texel = ureg_DECL_temporary(shader);
> fragment = ureg_DECL_output(shader, TGSI_SEMANTIC_COLOR, 0);
>
> @@ -156,12 +161,16 @@ create_frag_shader_video_buffer(struct vl_compositor *c)
> ureg_TEX(shader, ureg_writemask(texel, TGSI_WRITEMASK_X << i), TGSI_TEXTURE_2D_ARRAY, tc, sampler[i]);
>
> ureg_MOV(shader, ureg_writemask(texel, TGSI_WRITEMASK_W), ureg_imm1f(shader, 1.0f));
> + ureg_DP4(shader, ureg_writemask(t_tex[0], TGSI_WRITEMASK_W), ureg_src(texel), ureg_imm4f(shader, 0,0,1,0));
> + ureg_DP4(shader, ureg_writemask(t_tex[1], TGSI_WRITEMASK_W), csc[3], ureg_imm4f(shader, 1,0,0,0));
> + ureg_DP4(shader, ureg_writemask(t_tex[2], TGSI_WRITEMASK_W), csc[3], ureg_imm4f(shader, 0,1,0,0));
Well that made me smile :) What you are missing here is the function
ureg_swizzle().
This function can move the different channels around without the use of
a DP4 instruction.
> + ureg_SLE(shader, t_tex[1], ureg_src(t_tex[0]), ureg_src(t_tex[1]));
> + ureg_SGT(shader, t_tex[2], ureg_src(t_tex[0]), ureg_src(t_tex[2]));
> + ureg_MAX(shader, ureg_writemask(fragment, TGSI_WRITEMASK_W), ureg_src(t_tex[1]), ureg_src(t_tex[2]));
>
> for (i = 0; i < 3; ++i)
> ureg_DP4(shader, ureg_writemask(fragment, TGSI_WRITEMASK_X << i), csc[i], ureg_src(texel));
>
> - ureg_MOV(shader, ureg_writemask(fragment, TGSI_WRITEMASK_W), ureg_imm1f(shader, 1.0f));
> -
Better move calculating the W component here where we have set it
previously.
And you should release your temporary here again after using it.
Cheers,
Christian.
> ureg_release_temporary(shader, texel);
> ureg_END(shader);
>
> @@ -852,20 +861,21 @@ vl_compositor_cleanup(struct vl_compositor *c)
> }
>
> void
> -vl_compositor_set_csc_matrix(struct vl_compositor_state *s, vl_csc_matrix const *matrix)
> +vl_compositor_set_csc_matrix(struct vl_compositor_state *s, vl_csc_matrix const *matrix, float luma_min, float luma_max)
> {
> struct pipe_transfer *buf_transfer;
>
> assert(s);
>
> - memcpy
> - (
> - pipe_buffer_map(s->pipe, s->csc_matrix,
> - PIPE_TRANSFER_WRITE | PIPE_TRANSFER_DISCARD_RANGE,
> - &buf_transfer),
> - matrix,
> - sizeof(vl_csc_matrix)
> - );
> + float *ptr = pipe_buffer_map(s->pipe, s->csc_matrix,
> + PIPE_TRANSFER_WRITE | PIPE_TRANSFER_DISCARD_RANGE,
> + &buf_transfer);
> +
> + memcpy(ptr, matrix, sizeof(vl_csc_matrix));
> +
> + ptr += sizeof(vl_csc_matrix)/sizeof(float);
> + ptr[0] = luma_min;
> + ptr[1] = luma_max;
>
> pipe_buffer_unmap(s->pipe, buf_transfer);
> }
> @@ -1142,13 +1152,13 @@ vl_compositor_init_state(struct vl_compositor_state *s, struct pipe_context *pip
> pipe->screen,
> PIPE_BIND_CONSTANT_BUFFER,
> PIPE_USAGE_DEFAULT,
> - sizeof(csc_matrix)
> + sizeof(csc_matrix) + 2*sizeof(float)
> );
>
> vl_compositor_clear_layers(s);
>
> vl_csc_get_matrix(VL_CSC_COLOR_STANDARD_IDENTITY, NULL, true, &csc_matrix);
> - vl_compositor_set_csc_matrix(s, (const vl_csc_matrix *)&csc_matrix);
> + vl_compositor_set_csc_matrix(s, (const vl_csc_matrix *)&csc_matrix, 1.f, 0.f);
>
> return true;
> }
> diff --git a/src/gallium/auxiliary/vl/vl_compositor.h b/src/gallium/auxiliary/vl/vl_compositor.h
> index 934b634..23cab6c 100644
> --- a/src/gallium/auxiliary/vl/vl_compositor.h
> +++ b/src/gallium/auxiliary/vl/vl_compositor.h
> @@ -138,7 +138,7 @@ vl_compositor_init_state(struct vl_compositor_state *state, struct pipe_context
> * set yuv -> rgba conversion matrix
> */
> void
> -vl_compositor_set_csc_matrix(struct vl_compositor_state *settings, const vl_csc_matrix *matrix);
> +vl_compositor_set_csc_matrix(struct vl_compositor_state *settings, const vl_csc_matrix *matrix, float luma_min, float luma_max);
>
> /**
> * reset dirty area, so it's cleared with the clear colour
More information about the mesa-dev
mailing list