| Message ID | 20211209093654.3149267-3-jianhua.wu@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/4] avfilter: add a transpose_vulkan filter | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
9 Dec 2021, 10:36 by jianhua.wu@intel.com: > The input and output are arrays of images, so it's better to use the plural > to align them to the variable name of VkDescriptorImageInfo. > > Signed-off-by: Wu Jianhua <jianhua.wu@intel.com> > --- > libavfilter/vf_gblur_vulkan.c | 60 +++++++++++++++++------------------ > 1 file changed, 30 insertions(+), 30 deletions(-) > > diff --git a/libavfilter/vf_gblur_vulkan.c b/libavfilter/vf_gblur_vulkan.c > index a2e33d1c90..2dbbbd0965 100644 > --- a/libavfilter/vf_gblur_vulkan.c > +++ b/libavfilter/vf_gblur_vulkan.c > @@ -50,31 +50,31 @@ typedef struct GBlurVulkanContext { > } GBlurVulkanContext; > > static const char gblur_horizontal[] = { > - C(0, void gblur(const ivec2 pos, const int index) ) > - C(0, { ) > - C(1, vec4 sum = texture(input_image[index], pos) * kernel[0]; ) > - C(0, ) > - C(1, for(int i = 1; i < kernel.length(); i++) { ) > - C(2, sum += texture(input_image[index], pos + vec2(i, 0.0)) * kernel[i]; ) > - C(2, sum += texture(input_image[index], pos - vec2(i, 0.0)) * kernel[i]; ) > - C(1, } ) > - C(0, ) > - C(1, imageStore(output_image[index], pos, sum); ) > - C(0, } ) > + C(0, void gblur(const ivec2 pos, const int index) ) > + C(0, { ) > + C(1, vec4 sum = texture(input_images[index], pos) * kernel[0]; ) > + C(0, ) > + C(1, for (int i = 1; i < kernel.length(); i++) { ) > + C(2, sum += texture(input_images[index], pos + vec2(i, 0.0)) * kernel[i]; ) > + C(2, sum += texture(input_images[index], pos - vec2(i, 0.0)) * kernel[i]; ) > + C(1, } ) > + C(0, ) > + C(1, imageStore(output_images[index], pos, sum); ) > + C(0, } ) > }; > > static const char gblur_vertical[] = { > - C(0, void gblur(const ivec2 pos, const int index) ) > - C(0, { ) > - C(1, vec4 sum = texture(input_image[index], pos) * kernel[0]; ) > - C(0, ) > - C(1, for(int i = 1; i < kernel.length(); i++) { ) > - C(2, sum += texture(input_image[index], pos + vec2(0.0, i)) * kernel[i]; ) > - C(2, sum += texture(input_image[index], pos - vec2(0.0, i)) * kernel[i]; ) > - C(1, } ) > - C(0, ) > - C(1, imageStore(output_image[index], pos, sum); ) > - C(0, } ) > + C(0, void gblur(const ivec2 pos, const int index) ) > + C(0, { ) > + C(1, vec4 sum = texture(input_images[index], pos) * kernel[0]; ) > + C(0, ) > + C(1, for (int i = 1; i < kernel.length(); i++) { ) > + C(2, sum += texture(input_images[index], pos + vec2(0.0, i)) * kernel[i]; ) > + C(2, sum += texture(input_images[index], pos - vec2(0.0, i)) * kernel[i]; ) > + C(1, } ) > + C(0, ) > + C(1, imageStore(output_images[index], pos, sum); ) > + C(0, } ) > }; > > static inline float gaussian(float sigma, float x) > @@ -133,14 +133,14 @@ static av_cold int init_filter(AVFilterContext *ctx, AVFrame *in) > > FFVulkanDescriptorSetBinding image_descs[] = { > { > - .name = "input_image", > + .name = "input_images", > .type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, > .dimensions = 2, > .elems = planes, > .stages = VK_SHADER_STAGE_COMPUTE_BIT, > }, > { > - .name = "output_image", > + .name = "output_images", > .type = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, > .mem_layout = ff_vk_shader_rep_fmt(s->vkctx.output_format), > .mem_quali = "writeonly", > @@ -202,13 +202,13 @@ static av_cold int init_filter(AVFilterContext *ctx, AVFrame *in) > GLSLC(1, const ivec2 pos = ivec2(gl_GlobalInvocationID.xy); ); > for (int i = 0; i < planes; i++) { > GLSLC(0, ); > - GLSLF(1, size = imageSize(output_image[%i]); ,i); > + GLSLF(1, size = imageSize(output_images[%i]); ,i); > GLSLC(1, if (IS_WITHIN(pos, size)) { ); > if (s->planes & (1 << i)) { > GLSLF(2, gblur(pos, %i); ,i); > } else { > - GLSLF(2, vec4 res = texture(input_image[%i], pos); ,i); > - GLSLF(2, imageStore(output_image[%i], pos, res); ,i); > + GLSLF(2, vec4 res = texture(input_images[%i], pos); ,i); > + GLSLF(2, imageStore(output_images[%i], pos, res); ,i); > } > GLSLC(1, } ); > } > @@ -261,13 +261,13 @@ static av_cold int init_filter(AVFilterContext *ctx, AVFrame *in) > GLSLC(1, const ivec2 pos = ivec2(gl_GlobalInvocationID.xy); ); > for (int i = 0; i < planes; i++) { > GLSLC(0, ); > - GLSLF(1, size = imageSize(output_image[%i]); ,i); > + GLSLF(1, size = imageSize(output_images[%i]); ,i); > GLSLC(1, if (IS_WITHIN(pos, size)) { ); > if (s->planes & (1 << i)) { > GLSLF(2, gblur(pos, %i); ,i); > } else { > - GLSLF(2, vec4 res = texture(input_image[%i], pos); ,i); > - GLSLF(2, imageStore(output_image[%i], pos, res); ,i); > + GLSLF(2, vec4 res = texture(input_images[%i], pos); ,i); > + GLSLF(2, imageStore(output_images[%i], pos, res); ,i); > } > GLSLC(1, } ); > } > Not going to apply either patch, image[<image_index>] looks perfectly fine to me, and you didn't need to reindent the entire shader kernel either.
Lynne<mailto:dev@lynne.ee>: Sent: 2021年12月9日 19:26 To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] [PATCH v2 3/4] avfilter/gblur_vulkan: fix incorrect semantics 9 Dec 2021, 10:36 by jianhua.wu@intel.com: >> The input and output are arrays of images, so it's better to use the plural >> to align them to the variable name of VkDescriptorImageInfo. >> >> Signed-off-by: Wu Jianhua <jianhua.wu@intel.com> >> --- >> libavfilter/vf_gblur_vulkan.c | 60 +++++++++++++++++------------------ >> 1 file changed, 30 insertions(+), 30 deletions(-) >> >> diff --git a/libavfilter/vf_gblur_vulkan.c b/libavfilter/vf_gblur_vulkan.c >> index a2e33d1c90..2dbbbd0965 100644 >> --- a/libavfilter/vf_gblur_vulkan.c >> +++ b/libavfilter/vf_gblur_vulkan.c >> @@ -50,31 +50,31 @@ typedef struct GBlurVulkanContext { >> } GBlurVulkanContext; >> > > Not going to apply either patch, image[<image_index>] looks > perfectly fine to me, and you didn't need to reindent the entire > shader kernel either. > Alright. I’m really like using the plural for arrays and indentation to make them more clear and concise, but it’s okay to me to drop them if you really don’t like them.
diff --git a/libavfilter/vf_gblur_vulkan.c b/libavfilter/vf_gblur_vulkan.c index a2e33d1c90..2dbbbd0965 100644 --- a/libavfilter/vf_gblur_vulkan.c +++ b/libavfilter/vf_gblur_vulkan.c @@ -50,31 +50,31 @@ typedef struct GBlurVulkanContext { } GBlurVulkanContext; static const char gblur_horizontal[] = { - C(0, void gblur(const ivec2 pos, const int index) ) - C(0, { ) - C(1, vec4 sum = texture(input_image[index], pos) * kernel[0]; ) - C(0, ) - C(1, for(int i = 1; i < kernel.length(); i++) { ) - C(2, sum += texture(input_image[index], pos + vec2(i, 0.0)) * kernel[i]; ) - C(2, sum += texture(input_image[index], pos - vec2(i, 0.0)) * kernel[i]; ) - C(1, } ) - C(0, ) - C(1, imageStore(output_image[index], pos, sum); ) - C(0, } ) + C(0, void gblur(const ivec2 pos, const int index) ) + C(0, { ) + C(1, vec4 sum = texture(input_images[index], pos) * kernel[0]; ) + C(0, ) + C(1, for (int i = 1; i < kernel.length(); i++) { ) + C(2, sum += texture(input_images[index], pos + vec2(i, 0.0)) * kernel[i]; ) + C(2, sum += texture(input_images[index], pos - vec2(i, 0.0)) * kernel[i]; ) + C(1, } ) + C(0, ) + C(1, imageStore(output_images[index], pos, sum); ) + C(0, } ) }; static const char gblur_vertical[] = { - C(0, void gblur(const ivec2 pos, const int index) ) - C(0, { ) - C(1, vec4 sum = texture(input_image[index], pos) * kernel[0]; ) - C(0, ) - C(1, for(int i = 1; i < kernel.length(); i++) { ) - C(2, sum += texture(input_image[index], pos + vec2(0.0, i)) * kernel[i]; ) - C(2, sum += texture(input_image[index], pos - vec2(0.0, i)) * kernel[i]; ) - C(1, } ) - C(0, ) - C(1, imageStore(output_image[index], pos, sum); ) - C(0, } ) + C(0, void gblur(const ivec2 pos, const int index) ) + C(0, { ) + C(1, vec4 sum = texture(input_images[index], pos) * kernel[0]; ) + C(0, ) + C(1, for (int i = 1; i < kernel.length(); i++) { ) + C(2, sum += texture(input_images[index], pos + vec2(0.0, i)) * kernel[i]; ) + C(2, sum += texture(input_images[index], pos - vec2(0.0, i)) * kernel[i]; ) + C(1, } ) + C(0, ) + C(1, imageStore(output_images[index], pos, sum); ) + C(0, } ) }; static inline float gaussian(float sigma, float x) @@ -133,14 +133,14 @@ static av_cold int init_filter(AVFilterContext *ctx, AVFrame *in) FFVulkanDescriptorSetBinding image_descs[] = { { - .name = "input_image", + .name = "input_images", .type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, .dimensions = 2, .elems = planes, .stages = VK_SHADER_STAGE_COMPUTE_BIT, }, { - .name = "output_image", + .name = "output_images", .type = VK_DESCRIPTOR_TYPE_STORAGE_IMAGE, .mem_layout = ff_vk_shader_rep_fmt(s->vkctx.output_format), .mem_quali = "writeonly", @@ -202,13 +202,13 @@ static av_cold int init_filter(AVFilterContext *ctx, AVFrame *in) GLSLC(1, const ivec2 pos = ivec2(gl_GlobalInvocationID.xy); ); for (int i = 0; i < planes; i++) { GLSLC(0, ); - GLSLF(1, size = imageSize(output_image[%i]); ,i); + GLSLF(1, size = imageSize(output_images[%i]); ,i); GLSLC(1, if (IS_WITHIN(pos, size)) { ); if (s->planes & (1 << i)) { GLSLF(2, gblur(pos, %i); ,i); } else { - GLSLF(2, vec4 res = texture(input_image[%i], pos); ,i); - GLSLF(2, imageStore(output_image[%i], pos, res); ,i); + GLSLF(2, vec4 res = texture(input_images[%i], pos); ,i); + GLSLF(2, imageStore(output_images[%i], pos, res); ,i); } GLSLC(1, } ); } @@ -261,13 +261,13 @@ static av_cold int init_filter(AVFilterContext *ctx, AVFrame *in) GLSLC(1, const ivec2 pos = ivec2(gl_GlobalInvocationID.xy); ); for (int i = 0; i < planes; i++) { GLSLC(0, ); - GLSLF(1, size = imageSize(output_image[%i]); ,i); + GLSLF(1, size = imageSize(output_images[%i]); ,i); GLSLC(1, if (IS_WITHIN(pos, size)) { ); if (s->planes & (1 << i)) { GLSLF(2, gblur(pos, %i); ,i); } else { - GLSLF(2, vec4 res = texture(input_image[%i], pos); ,i); - GLSLF(2, imageStore(output_image[%i], pos, res); ,i); + GLSLF(2, vec4 res = texture(input_images[%i], pos); ,i); + GLSLF(2, imageStore(output_images[%i], pos, res); ,i); } GLSLC(1, } ); }
The input and output are arrays of images, so it's better to use the plural to align them to the variable name of VkDescriptorImageInfo. Signed-off-by: Wu Jianhua <jianhua.wu@intel.com> --- libavfilter/vf_gblur_vulkan.c | 60 +++++++++++++++++------------------ 1 file changed, 30 insertions(+), 30 deletions(-)