| Message ID | 1540790282-29764-2-git-send-email-ruiling.song@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
> -----Original Message----- > From: Song, Ruiling > Sent: Monday, October 29, 2018 1:18 PM > To: ffmpeg-devel@ffmpeg.org > Cc: Song, Ruiling <ruiling.song@intel.com> > Subject: [PATCH 2/4] lavfi/opencl: Handle overlay input formats correctly. > > The main input may have alpha channel, we just ignore it. > Also add some checks for incompatible input formats. > > Signed-off-by: Ruiling Song <ruiling.song@intel.com> > --- > libavfilter/vf_overlay_opencl.c | 58 ++++++++++++++++++++++++++++++++----- > ---- > 1 file changed, 46 insertions(+), 12 deletions(-) > > diff --git a/libavfilter/vf_overlay_opencl.c b/libavfilter/vf_overlay_opencl.c > index e9c8532..320c1a5 100644 > --- a/libavfilter/vf_overlay_opencl.c > +++ b/libavfilter/vf_overlay_opencl.c > @@ -37,7 +37,7 @@ typedef struct OverlayOpenCLContext { > > FFFrameSync fs; > > - int nb_planes; > + int nb_color_planes; > int x_subsample; > int y_subsample; > int alpha_separate; > @@ -46,6 +46,22 @@ typedef struct OverlayOpenCLContext { > int y_position; > } OverlayOpenCLContext; > > +static int has_planar_alpha(const AVPixFmtDescriptor *fmt) { > + int nb_components; > + int has_alpha = !!(fmt->flags & AV_PIX_FMT_FLAG_ALPHA); > + if (!has_alpha) return 0; > + > + nb_components = fmt->nb_components; > + // PAL8 > + if (nb_components < 2) return 0; > + > + if (fmt->comp[nb_components - 1].plane > > + fmt->comp[nb_components - 2].plane) > + return 1; > + else > + return 0; > +} > + > static int overlay_opencl_load(AVFilterContext *avctx, > enum AVPixelFormat main_format, > enum AVPixelFormat overlay_format) > @@ -55,10 +71,13 @@ static int overlay_opencl_load(AVFilterContext *avctx, > const char *source = ff_opencl_source_overlay; > const char *kernel; > const AVPixFmtDescriptor *main_desc, *overlay_desc; > - int err, i, main_planes, overlay_planes; > + int err, i, main_planes, overlay_planes, overlay_alpha, > + main_planar_alpha, overlay_planar_alpha; > > main_desc = av_pix_fmt_desc_get(main_format); > overlay_desc = av_pix_fmt_desc_get(overlay_format); > + overlay_alpha = !!(overlay_desc->flags & AV_PIX_FMT_FLAG_ALPHA); > + main_planar_alpha = has_planar_alpha(main_desc); > > main_planes = overlay_planes = 0; > for (i = 0; i < main_desc->nb_components; i++) > @@ -68,7 +87,7 @@ static int overlay_opencl_load(AVFilterContext *avctx, > overlay_planes = FFMAX(overlay_planes, > overlay_desc->comp[i].plane + 1); > > - ctx->nb_planes = main_planes; > + ctx->nb_color_planes = main_planar_alpha ? (main_planes - 1) : main_planes; > ctx->x_subsample = 1 << main_desc->log2_chroma_w; > ctx->y_subsample = 1 << main_desc->log2_chroma_h; > > @@ -80,15 +99,30 @@ static int overlay_opencl_load(AVFilterContext *avctx, > ctx->x_subsample, ctx->y_subsample); > } > > - if (main_planes == overlay_planes) { > - if (main_desc->nb_components == overlay_desc->nb_components) > - kernel = "overlay_no_alpha"; > - else > - kernel = "overlay_internal_alpha"; > + if ((main_desc->flags & AV_PIX_FMT_FLAG_RGB) != > + (overlay_desc->flags & AV_PIX_FMT_FLAG_RGB)) { > + av_log(avctx, AV_LOG_ERROR, "mixed YUV/RGB input formats.\n"); > + return AVERROR(EINVAL); > + } > + > + if (main_desc->log2_chroma_w != overlay_desc->log2_chroma_w || > + main_desc->log2_chroma_h != overlay_desc->log2_chroma_h) { > + av_log(avctx, AV_LOG_ERROR, "incompatible chroma sub-sampling.\n"); > + return AVERROR(EINVAL); > + } > + > + if (!overlay_alpha) { > ctx->alpha_separate = 0; > + kernel = "overlay_no_alpha"; > } else { > - kernel = "overlay_external_alpha"; > - ctx->alpha_separate = 1; > + overlay_planar_alpha = has_planar_alpha(overlay_desc); > + if (overlay_planar_alpha) { > + ctx->alpha_separate = 1; > + kernel = "overlay_external_alpha"; > + } else { > + ctx->alpha_separate = 0; > + kernel = "overlay_internal_alpha"; > + } > } > > av_log(avctx, AV_LOG_DEBUG, "Using kernel %s.\n", kernel); > @@ -155,7 +189,7 @@ static int overlay_opencl_blend(FFFrameSync *fs) > goto fail; > } > > - for (plane = 0; plane < ctx->nb_planes; plane++) { > + for (plane = 0; plane < ctx->nb_color_planes; plane++) { > kernel_arg = 0; > > mem = (cl_mem)output->data[plane]; > @@ -171,7 +205,7 @@ static int overlay_opencl_blend(FFFrameSync *fs) > kernel_arg++; > > if (ctx->alpha_separate) { > - mem = (cl_mem)input_overlay->data[ctx->nb_planes]; > + mem = (cl_mem)input_overlay->data[ctx->nb_color_planes]; > CL_SET_KERNEL_ARG(ctx->kernel, kernel_arg, cl_mem, &mem); > kernel_arg++; > } > -- Any comments on this patch? > 2.7.4
> > -----Original Message----- > > From: Song, Ruiling > > Sent: Monday, October 29, 2018 1:18 PM > > To: ffmpeg-devel@ffmpeg.org > > Cc: Song, Ruiling <ruiling.song@intel.com> > > Subject: [PATCH 2/4] lavfi/opencl: Handle overlay input formats correctly. > > > > The main input may have alpha channel, we just ignore it. > > Also add some checks for incompatible input formats. > > > > Signed-off-by: Ruiling Song <ruiling.song@intel.com> LGTM. BTW, could the main input with alpha case be supported?
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of > Li, Zhong > Sent: Wednesday, November 7, 2018 2:58 PM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 2/4] lavfi/opencl: Handle overlay input > formats correctly. > > > > -----Original Message----- > > > From: Song, Ruiling > > > Sent: Monday, October 29, 2018 1:18 PM > > > To: ffmpeg-devel@ffmpeg.org > > > Cc: Song, Ruiling <ruiling.song@intel.com> > > > Subject: [PATCH 2/4] lavfi/opencl: Handle overlay input formats correctly. > > > > > > The main input may have alpha channel, we just ignore it. > > > Also add some checks for incompatible input formats. > > > > > > Signed-off-by: Ruiling Song <ruiling.song@intel.com> > LGTM. > BTW, could the main input with alpha case be supported? I am not sure what kind of support do you mean? I simply ignore the alpha channel of the main input, and do the alpha blending using the overlay alpha. Before this patch, the filter will do it wrong if the main input has alpha channel. Now it works with this patch. Thanks! Ruiling > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > > -----Original Message----- > > > > From: Song, Ruiling > > > > Sent: Monday, October 29, 2018 1:18 PM > > > > To: ffmpeg-devel@ffmpeg.org > > > > Cc: Song, Ruiling <ruiling.song@intel.com> > > > > Subject: [PATCH 2/4] lavfi/opencl: Handle overlay input formats > correctly. > > > > > > > > The main input may have alpha channel, we just ignore it. > > > > Also add some checks for incompatible input formats. > > > > > > > > Signed-off-by: Ruiling Song <ruiling.song@intel.com> > > LGTM. > > BTW, could the main input with alpha case be supported? > > I am not sure what kind of support do you mean? > I simply ignore the alpha channel of the main input, and do the alpha > blending using the overlay alpha. > Before this patch, the filter will do it wrong if the main input has alpha > channel. Now it works with this patch. > > Thanks! > Ruiling I mean support alpha blending with alpha channel of main input when no overlay alpha.
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of > Li, Zhong > Sent: Wednesday, November 7, 2018 4:37 PM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 2/4] lavfi/opencl: Handle overlay input > formats correctly. > > > > > > -----Original Message----- > > > > > From: Song, Ruiling > > > > > Sent: Monday, October 29, 2018 1:18 PM > > > > > To: ffmpeg-devel@ffmpeg.org > > > > > Cc: Song, Ruiling <ruiling.song@intel.com> > > > > > Subject: [PATCH 2/4] lavfi/opencl: Handle overlay input formats > > correctly. > > > > > > > > > > The main input may have alpha channel, we just ignore it. > > > > > Also add some checks for incompatible input formats. > > > > > > > > > > Signed-off-by: Ruiling Song <ruiling.song@intel.com> > > > LGTM. > > > BTW, could the main input with alpha case be supported? > > > > I am not sure what kind of support do you mean? > > I simply ignore the alpha channel of the main input, and do the alpha > > blending using the overlay alpha. > > Before this patch, the filter will do it wrong if the main input has alpha > > channel. Now it works with this patch. > > > > Thanks! > > Ruiling > > I mean support alpha blending with alpha channel of main input when no overlay > alpha. I think I got your idea, this patch aims to fix the issues reported by Gyan. If people really want it (blending using alpha of main input) be supported, we can add it later. I am not sure whether this sounds ok? Thanks! Ruiling > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > -----Original Message----- > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > Behalf > > Of Li, Zhong > > Sent: Wednesday, November 7, 2018 4:37 PM > > To: FFmpeg development discussions and patches > > <ffmpeg-devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH 2/4] lavfi/opencl: Handle overlay > > input formats correctly. > > > > > > > > -----Original Message----- > > > > > > From: Song, Ruiling > > > > > > Sent: Monday, October 29, 2018 1:18 PM > > > > > > To: ffmpeg-devel@ffmpeg.org > > > > > > Cc: Song, Ruiling <ruiling.song@intel.com> > > > > > > Subject: [PATCH 2/4] lavfi/opencl: Handle overlay input > > > > > > formats > > > correctly. > > > > > > > > > > > > The main input may have alpha channel, we just ignore it. > > > > > > Also add some checks for incompatible input formats. > > > > > > > > > > > > Signed-off-by: Ruiling Song <ruiling.song@intel.com> > > > > LGTM. > > > > BTW, could the main input with alpha case be supported? > > > > > > I am not sure what kind of support do you mean? > > > I simply ignore the alpha channel of the main input, and do the > > > alpha blending using the overlay alpha. > > > Before this patch, the filter will do it wrong if the main input has > > > alpha channel. Now it works with this patch. > > > > > > Thanks! > > > Ruiling > > > > I mean support alpha blending with alpha channel of main input when no > > overlay alpha. > I think I got your idea, this patch aims to fix the issues reported by Gyan. > If people really want it (blending using alpha of main input) be supported, we > can add it later. > I am not sure whether this sounds ok? > > Thanks! > Ruiling Sure, sound good.
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of > Li, Zhong > Sent: Thursday, November 8, 2018 11:39 AM > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 2/4] lavfi/opencl: Handle overlay input > formats correctly. > > > > -----Original Message----- > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On > > Behalf > > > Of Li, Zhong > > > Sent: Wednesday, November 7, 2018 4:37 PM > > > To: FFmpeg development discussions and patches > > > <ffmpeg-devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH 2/4] lavfi/opencl: Handle overlay > > > input formats correctly. > > > > > > > > > > -----Original Message----- > > > > > > > From: Song, Ruiling > > > > > > > Sent: Monday, October 29, 2018 1:18 PM > > > > > > > To: ffmpeg-devel@ffmpeg.org > > > > > > > Cc: Song, Ruiling <ruiling.song@intel.com> > > > > > > > Subject: [PATCH 2/4] lavfi/opencl: Handle overlay input > > > > > > > formats > > > > correctly. > > > > > > > > > > > > > > The main input may have alpha channel, we just ignore it. > > > > > > > Also add some checks for incompatible input formats. > > > > > > > > > > > > > > Signed-off-by: Ruiling Song <ruiling.song@intel.com> > > > > > LGTM. > > > > > BTW, could the main input with alpha case be supported? > > > > > > > > I am not sure what kind of support do you mean? > > > > I simply ignore the alpha channel of the main input, and do the > > > > alpha blending using the overlay alpha. > > > > Before this patch, the filter will do it wrong if the main input has > > > > alpha channel. Now it works with this patch. > > > > > > > > Thanks! > > > > Ruiling > > > > > > I mean support alpha blending with alpha channel of main input when no > > > overlay alpha. > > I think I got your idea, this patch aims to fix the issues reported by Gyan. > > If people really want it (blending using alpha of main input) be supported, we > > can add it later. > > I am not sure whether this sounds ok? > > > > Thanks! > > Ruiling > > Sure, sound good. Can we merge this patch? Any objection or concern? Ruiling > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > > > > > > The main input may have alpha channel, we just ignore it. > > > > > > > > Also add some checks for incompatible input formats. > > > > > > > > > > > > > > > > Signed-off-by: Ruiling Song <ruiling.song@intel.com> > > > > > > LGTM. > > > > > > BTW, could the main input with alpha case be supported? > > > > > > > > > > I am not sure what kind of support do you mean? > > > > > I simply ignore the alpha channel of the main input, and do the > > > > > alpha blending using the overlay alpha. > > > > > Before this patch, the filter will do it wrong if the main input > > > > > has alpha channel. Now it works with this patch. > > > > > > > > > > Thanks! > > > > > Ruiling > > > > > > > > I mean support alpha blending with alpha channel of main input > > > > when no overlay alpha. > > > I think I got your idea, this patch aims to fix the issues reported by Gyan. > > > If people really want it (blending using alpha of main input) be > > > supported, we can add it later. > > > I am not sure whether this sounds ok? > > > > > > Thanks! > > > Ruiling > > > > Sure, sound good. > > Can we merge this patch? Any objection or concern? > > Ruiling Could you please provide the use case (with an alpha channel main input) to verify it? If no body against, I prefer to merge it after verification during next a few days.
diff --git a/libavfilter/vf_overlay_opencl.c b/libavfilter/vf_overlay_opencl.c index e9c8532..320c1a5 100644 --- a/libavfilter/vf_overlay_opencl.c +++ b/libavfilter/vf_overlay_opencl.c @@ -37,7 +37,7 @@ typedef struct OverlayOpenCLContext { FFFrameSync fs; - int nb_planes; + int nb_color_planes; int x_subsample; int y_subsample; int alpha_separate; @@ -46,6 +46,22 @@ typedef struct OverlayOpenCLContext { int y_position; } OverlayOpenCLContext; +static int has_planar_alpha(const AVPixFmtDescriptor *fmt) { + int nb_components; + int has_alpha = !!(fmt->flags & AV_PIX_FMT_FLAG_ALPHA); + if (!has_alpha) return 0; + + nb_components = fmt->nb_components; + // PAL8 + if (nb_components < 2) return 0; + + if (fmt->comp[nb_components - 1].plane > + fmt->comp[nb_components - 2].plane) + return 1; + else + return 0; +} + static int overlay_opencl_load(AVFilterContext *avctx, enum AVPixelFormat main_format, enum AVPixelFormat overlay_format) @@ -55,10 +71,13 @@ static int overlay_opencl_load(AVFilterContext *avctx, const char *source = ff_opencl_source_overlay; const char *kernel; const AVPixFmtDescriptor *main_desc, *overlay_desc; - int err, i, main_planes, overlay_planes; + int err, i, main_planes, overlay_planes, overlay_alpha, + main_planar_alpha, overlay_planar_alpha; main_desc = av_pix_fmt_desc_get(main_format); overlay_desc = av_pix_fmt_desc_get(overlay_format); + overlay_alpha = !!(overlay_desc->flags & AV_PIX_FMT_FLAG_ALPHA); + main_planar_alpha = has_planar_alpha(main_desc); main_planes = overlay_planes = 0; for (i = 0; i < main_desc->nb_components; i++) @@ -68,7 +87,7 @@ static int overlay_opencl_load(AVFilterContext *avctx, overlay_planes = FFMAX(overlay_planes, overlay_desc->comp[i].plane + 1); - ctx->nb_planes = main_planes; + ctx->nb_color_planes = main_planar_alpha ? (main_planes - 1) : main_planes; ctx->x_subsample = 1 << main_desc->log2_chroma_w; ctx->y_subsample = 1 << main_desc->log2_chroma_h; @@ -80,15 +99,30 @@ static int overlay_opencl_load(AVFilterContext *avctx, ctx->x_subsample, ctx->y_subsample); } - if (main_planes == overlay_planes) { - if (main_desc->nb_components == overlay_desc->nb_components) - kernel = "overlay_no_alpha"; - else - kernel = "overlay_internal_alpha"; + if ((main_desc->flags & AV_PIX_FMT_FLAG_RGB) != + (overlay_desc->flags & AV_PIX_FMT_FLAG_RGB)) { + av_log(avctx, AV_LOG_ERROR, "mixed YUV/RGB input formats.\n"); + return AVERROR(EINVAL); + } + + if (main_desc->log2_chroma_w != overlay_desc->log2_chroma_w || + main_desc->log2_chroma_h != overlay_desc->log2_chroma_h) { + av_log(avctx, AV_LOG_ERROR, "incompatible chroma sub-sampling.\n"); + return AVERROR(EINVAL); + } + + if (!overlay_alpha) { ctx->alpha_separate = 0; + kernel = "overlay_no_alpha"; } else { - kernel = "overlay_external_alpha"; - ctx->alpha_separate = 1; + overlay_planar_alpha = has_planar_alpha(overlay_desc); + if (overlay_planar_alpha) { + ctx->alpha_separate = 1; + kernel = "overlay_external_alpha"; + } else { + ctx->alpha_separate = 0; + kernel = "overlay_internal_alpha"; + } } av_log(avctx, AV_LOG_DEBUG, "Using kernel %s.\n", kernel); @@ -155,7 +189,7 @@ static int overlay_opencl_blend(FFFrameSync *fs) goto fail; } - for (plane = 0; plane < ctx->nb_planes; plane++) { + for (plane = 0; plane < ctx->nb_color_planes; plane++) { kernel_arg = 0; mem = (cl_mem)output->data[plane]; @@ -171,7 +205,7 @@ static int overlay_opencl_blend(FFFrameSync *fs) kernel_arg++; if (ctx->alpha_separate) { - mem = (cl_mem)input_overlay->data[ctx->nb_planes]; + mem = (cl_mem)input_overlay->data[ctx->nb_color_planes]; CL_SET_KERNEL_ARG(ctx->kernel, kernel_arg, cl_mem, &mem); kernel_arg++; }
The main input may have alpha channel, we just ignore it. Also add some checks for incompatible input formats. Signed-off-by: Ruiling Song <ruiling.song@intel.com> --- libavfilter/vf_overlay_opencl.c | 58 ++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 12 deletions(-)