<div dir="ltr"><div class="gmail_quote"><div dir="ltr">Thanks for the review. <div class="gmail_extra"> <div class="gmail_quote"><div><div class="h5">On Wed, Jun 22, 2016 at 1:17 PM, Christian König <<a href="mailto:deathsimple@vodafone.de" target="_blank">deathsimple@vodafone.de</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div>Am 21.06.2016 um 21:21 schrieb Nayan Deshmukh: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> use bicubic filtering as high quality scaling L1. v2: fix a typo and add a newline to code v3: -render the unscaled image on a temporary surface (Christian) -apply noise reduction and sharpness filter on unscaled surface -render the final scaled surface using bicubic interpolation v4: support high quality scaling Signed-off-by: Nayan Deshmukh <<a href="mailto:nayan26deshmukh@gmail.com" target="_blank">nayan26deshmukh@gmail.com</a>> --- src/gallium/state_trackers/vdpau/mixer.c | 109 ++++++++++++++++++++--- src/gallium/state_trackers/vdpau/query.c | 1 + src/gallium/state_trackers/vdpau/vdpau_private.h | 6 ++ 3 files changed, 102 insertions(+), 14 deletions(-) diff --git a/src/gallium/state_trackers/vdpau/mixer.c b/src/gallium/state_trackers/vdpau/mixer.c index 65c3ce2..2a67ac2 100644 --- a/src/gallium/state_trackers/vdpau/mixer.c +++ b/src/gallium/state_trackers/vdpau/mixer.c @@ -82,7 +82,6 @@ vlVdpVideoMixerCreate(VdpDevice device, switch (features[i]) { /* they are valid, but we doesn't support them */ case VDP_VIDEO_MIXER_FEATURE_DEINTERLACE_TEMPORAL_SPATIAL: - case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L2: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L3: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L4: @@ -110,6 +109,9 @@ vlVdpVideoMixerCreate(VdpDevice device, vmixer->luma_key.supported = true; break; + case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: + vmixer->bicubic.supported = true; + break; default: goto no_params; } } @@ -202,6 +204,11 @@ vlVdpVideoMixerDestroy(VdpVideoMixer mixer) vl_matrix_filter_cleanup(vmixer->sharpness.filter); FREE(vmixer->sharpness.filter); } + + if (vmixer->bicubic.filter) { + vl_bicubic_filter_cleanup(vmixer->bicubic.filter); + FREE(vmixer->bicubic.filter); + } pipe_mutex_unlock(vmixer->device->mutex); DeviceReference(&vmixer->device, NULL); @@ -230,9 +237,11 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, VdpLayer const *layers) { enum vl_compositor_deinterlace deinterlace; - struct u_rect rect, clip, *prect; + struct u_rect rect, clip, *prect, *rect_temp, dirty_area, temp; unsigned i, layer = 0; struct pipe_video_buffer *video_buffer; + struct pipe_sampler_view *sampler_view; + struct pipe_surface *surface; vlVdpVideoMixer *vmixer; vlVdpSurface *surf; @@ -324,8 +333,48 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, rect.y1 = surf->templat.height; prect = &rect; } + + if(vmixer->bicubic.filter){ + struct pipe_context *pipe; + struct pipe_resource res_tmpl, *res; + struct pipe_sampler_view sv_templ; + struct pipe_surface surf_templ; + + pipe = vmixer->device->context; + memset(&res_tmpl, 0, sizeof(res_tmpl)); + + res_tmpl.target = PIPE_TEXTURE_2D; + res_tmpl.format = surf->templat.chroma_format; </blockquote> </div></div> That is incorrect. The resource format isn't related in any way to the chroma format. Please use the format of the destination surface here. </blockquote></div></div><div>I should probably use res_tmpl = dst->sampler_view->texture->format; Right?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> + res_tmpl.width0 = surf->templat.width; + res_tmpl.height0 = surf->templat.height; + res_tmpl.depth0 = 1; + res_tmpl.array_size = 1; + res_tmpl.bind = PIPE_BIND_SAMPLER_VIEW | PIPE_BIND_RENDER_TARGET | + PIPE_BIND_LINEAR | PIPE_BIND_SHARED; </blockquote> No need for PIPE_BIND_LINEAR or PIPE_BIND_SHARED here, we are not going to share the temporary texture with anybody. <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> + res_tmpl.usage = PIPE_USAGE_DEFAULT; + + res = pipe->screen->resource_create(pipe->screen, &res_tmpl); + + vlVdpDefaultSamplerViewTemplate(&sv_templ, res); + sampler_view = pipe->create_sampler_view(pipe, res, &sv_templ); + + memset(&surf_templ, 0, sizeof(surf_templ)); + surf_templ.format = res->format; + surface = pipe->create_surface(pipe, res, &surf_templ); + + vl_compositor_reset_dirty_area(&dirty_area); + rect_temp = prect; </blockquote> You need to free the resource with pipe_resource_reference(&res, NULL); here, otherwise you will create quite a memory leak. Same thing is true for the surface and the sampler view, but you need those and can only free them later on. Easiest way to do this is probably to grab an extra reference in the else case as well. </blockquote><div>I can also check in the end if (surface != dst->surface) and then free the surface and sampler_view if it's true.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> + } + + else{ </blockquote> Coding style here should be "} else {" if I remember correctly. <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> + surface = dst->surface; + sampler_view = dst->sampler_view; + rect_temp = RectToPipe(destination_video_rect, &temp); + dirty_area = dst->dirty_area; + } + vl_compositor_set_buffer_layer(&vmixer->cstate, compositor, layer, video_buffer, prect, NULL, deinterlace); - vl_compositor_set_layer_dst_area(&vmixer->cstate, layer++, RectToPipe(destination_video_rect, &rect)); + vl_compositor_set_layer_dst_area(&vmixer->cstate, layer++, rect_temp); for (i = 0; i < layer_count; ++i) { vlVdpOutputSurface *src = vlGetDataHTAB(layers->source_surface); @@ -343,22 +392,22 @@ VdpStatus vlVdpVideoMixerRender(VdpVideoMixer mixer, ++layers; } - vl_compositor_set_dst_clip(&vmixer->cstate, RectToPipe(destination_rect, &clip)); </blockquote> That probably needs to be moved into the true case of the if and the clip handled by the bicubic filter as well. Same is true for the destination video rectangle as well. </blockquote><div> </div><div>if the bicubic scaling is off then we need to set dst_area and dst_clip in vl_compositor, but if bicubic scaling is on then </div><div>dst_area and dst_clip should be set in bicubic_filter.</div><div> </div><div>Regards,</div><div>Nayan.</div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Regards, Christian.<div><div> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> - if (!vmixer->noise_reduction.filter && !vmixer->sharpness.filter) + if (!vmixer->noise_reduction.filter && !vmixer->sharpness.filter && !vmixer->bicubic.filter) vlVdpSave4DelayedRendering(vmixer->device, destination_surface, &vmixer->cstate); else { - vl_compositor_render(&vmixer->cstate, compositor, dst->surface, &dst->dirty_area, true); + vl_compositor_render(&vmixer->cstate, compositor, surface, &dirty_area, true); - /* applying the noise reduction after scaling is actually not very - clever, but currently we should avoid to copy around the image - data once more. */ if (vmixer->noise_reduction.filter) vl_median_filter_render(vmixer->noise_reduction.filter, - dst->sampler_view, dst->surface); + sampler_view, surface); if (vmixer->sharpness.filter) vl_matrix_filter_render(vmixer->sharpness.filter, - dst->sampler_view, dst->surface); + sampler_view, surface); + + if (vmixer->bicubic.filter) + vl_bicubic_filter_render(vmixer->bicubic.filter, + sampler_view, dst->surface); } pipe_mutex_unlock(vmixer->device->mutex); @@ -461,6 +510,28 @@ vlVdpVideoMixerUpdateSharpnessFilter(vlVdpVideoMixer *vmixer) } /** + * Update the bicubic filter + */ +static void +vlVdpVideoMixerUpdateBicubicFilter(vlVdpVideoMixer *vmixer) +{ + assert(vmixer); + + /* if present remove the old filter first */ + if (vmixer->bicubic.filter) { + vl_bicubic_filter_cleanup(vmixer->bicubic.filter); + FREE(vmixer->bicubic.filter); + vmixer->bicubic.filter = NULL; + } + /* and create a new filter as needed */ + if (vmixer->bicubic.enabled) { + vmixer->bicubic.filter = MALLOC(sizeof(struct vl_bicubic_filter)); + vl_bicubic_filter_init(vmixer->bicubic.filter, vmixer->device->context, + vmixer->video_width, vmixer->video_height); + } +} + +/** * Retrieve whether features were requested at creation time. */ VdpStatus @@ -483,7 +554,6 @@ vlVdpVideoMixerGetFeatureSupport(VdpVideoMixer mixer, switch (features[i]) { /* they are valid, but we doesn't support them */ case VDP_VIDEO_MIXER_FEATURE_DEINTERLACE_TEMPORAL_SPATIAL: - case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L2: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L3: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L4: @@ -512,6 +582,10 @@ vlVdpVideoMixerGetFeatureSupport(VdpVideoMixer mixer, feature_supports[i] = vmixer->luma_key.supported; break; + case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: + feature_supports[i] = vmixer->bicubic.supported; + break; + default: return VDP_STATUS_INVALID_VIDEO_MIXER_FEATURE; } @@ -544,7 +618,6 @@ vlVdpVideoMixerSetFeatureEnables(VdpVideoMixer mixer, switch (features[i]) { /* they are valid, but we doesn't support them */ case VDP_VIDEO_MIXER_FEATURE_DEINTERLACE_TEMPORAL_SPATIAL: - case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L2: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L3: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L4: @@ -578,6 +651,11 @@ vlVdpVideoMixerSetFeatureEnables(VdpVideoMixer mixer, vmixer->luma_key.luma_min, vmixer->luma_key.luma_max); break; + case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: + vmixer->bicubic.enabled = feature_enables[i]; + vlVdpVideoMixerUpdateBicubicFilter(vmixer); + break; + default: pipe_mutex_unlock(vmixer->device->mutex); return VDP_STATUS_INVALID_VIDEO_MIXER_FEATURE; @@ -612,7 +690,6 @@ vlVdpVideoMixerGetFeatureEnables(VdpVideoMixer mixer, /* they are valid, but we doesn't support them */ case VDP_VIDEO_MIXER_FEATURE_DEINTERLACE_TEMPORAL: case VDP_VIDEO_MIXER_FEATURE_DEINTERLACE_TEMPORAL_SPATIAL: - case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L2: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L3: case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L4: @@ -636,6 +713,10 @@ vlVdpVideoMixerGetFeatureEnables(VdpVideoMixer mixer, feature_enables[i] = vmixer->luma_key.enabled; break; + case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: + feature_enables[i] = vmixer->bicubic.enabled; + break; + default: return VDP_STATUS_INVALID_VIDEO_MIXER_FEATURE; } diff --git a/src/gallium/state_trackers/vdpau/query.c b/src/gallium/state_trackers/vdpau/query.c index c3decc0..e69c9b1 100644 --- a/src/gallium/state_trackers/vdpau/query.c +++ b/src/gallium/state_trackers/vdpau/query.c @@ -471,6 +471,7 @@ vlVdpVideoMixerQueryFeatureSupport(VdpDevice device, VdpVideoMixerFeature featur case VDP_VIDEO_MIXER_FEATURE_NOISE_REDUCTION: case VDP_VIDEO_MIXER_FEATURE_DEINTERLACE_TEMPORAL: case VDP_VIDEO_MIXER_FEATURE_LUMA_KEY: + case VDP_VIDEO_MIXER_FEATURE_HIGH_QUALITY_SCALING_L1: *is_supported = VDP_TRUE; break; default: diff --git a/src/gallium/state_trackers/vdpau/vdpau_private.h b/src/gallium/state_trackers/vdpau/vdpau_private.h index 8673c6a..bcd4bb1 100644 --- a/src/gallium/state_trackers/vdpau/vdpau_private.h +++ b/src/gallium/state_trackers/vdpau/vdpau_private.h @@ -45,6 +45,7 @@ #include "os/os_thread.h" #include "vl/vl_video_buffer.h" +#include "vl/vl_bicubic_filter.h" #include "vl/vl_compositor.h" #include "vl/vl_csc.h" #include "vl/vl_deint_filter.h" @@ -373,6 +374,11 @@ typedef struct } deint; struct { + bool supported, enabled; + struct vl_bicubic_filter *filter; + } bicubic; + + struct { bool supported, enabled; unsigned level; struct vl_median_filter *filter; </blockquote> </div></div></blockquote></div></div></div> </div></div> </div> </div>