[FFmpeg-devel] libavfilter: extending overlay filter
Stefano Sabatini
stefano.sabatini-lala
Sun Mar 13 19:38:04 CET 2011
On date Sunday 2011-03-13 15:18:04 +0000, Mark Himsley encoded:
> On 13/03/11 14:26, Stefano Sabatini wrote:
> >On date Sunday 2011-03-13 14:18:42 +0000, Mark Himsley encoded:
[...]
> --- ffmpeg/libavfilter/vf_overlay.c 2011-02-19 15:20:16.000000000 +0000
> +++ FFmbc-0.6-rc3/libavfilter/vf_overlay.c 2011-03-13 15:11:23.000000000 +0000
Please provide a patch created with git format-patch.
> @@ -60,6 +60,19 @@
> typedef struct {
> int x, y; ///< position of overlayed picture
>
> + int inout_is_rgb;
> + int inout_is_alpha;
> + int inout_r_location;
> + int inout_g_location;
> + int inout_b_location;
> + int inout_a_location;
> +
> + int blend_is_rgb;
> + int blend_r_location;
> + int blend_g_location;
> + int blend_b_location;
> + int blend_a_location;
This is best addressed by using an rgb_map.
Check libavfilter/drawutils.c:ff_fill_line_with_color()
Note: the code should be eventually factorized.
> +
> AVFilterBufferRef *overpicref;
>
> int max_plane_step[4]; ///< steps per pixel for each plane
> @@ -91,8 +104,25 @@
>
> static int query_formats(AVFilterContext *ctx)
> {
> - const enum PixelFormat inout_pix_fmts[] = { PIX_FMT_YUV420P, PIX_FMT_NONE };
> - const enum PixelFormat blend_pix_fmts[] = { PIX_FMT_YUVA420P, PIX_FMT_NONE };
> + const enum PixelFormat inout_pix_fmts[] = {
> + PIX_FMT_YUV420P,
> + PIX_FMT_BGR24,
> + PIX_FMT_RGB24,
> + PIX_FMT_ARGB,
> + PIX_FMT_RGBA,
> + PIX_FMT_ABGR,
> + PIX_FMT_BGRA,
> + PIX_FMT_NONE
> + };
> + const enum PixelFormat blend_pix_fmts[] = {
> + PIX_FMT_YUVA420P,
> + PIX_FMT_ARGB,
> + PIX_FMT_RGBA,
> + PIX_FMT_ABGR,
> + PIX_FMT_BGRA,
> + PIX_FMT_NONE
> + };
> +
> AVFilterFormats *inout_formats = avfilter_make_format_list(inout_pix_fmts);
> AVFilterFormats *blend_formats = avfilter_make_format_list(blend_pix_fmts);
Ideally we should be able to define the overlay pad accepted formats
when the main pad formats have been already negotiated. Unfortunately
I don't think this is currently possible.
Alternatively we could setup a parameter during configuration to set
the colorspace class to use (rgb/yuv).
> @@ -105,13 +135,68 @@
>
> static int config_input_main(AVFilterLink *inlink)
> {
> - OverlayContext *over = inlink->dst->priv;
> + AVFilterContext *ctx = inlink->dst;
> + OverlayContext *over = inlink->dst->priv;
> const AVPixFmtDescriptor *pix_desc = &av_pix_fmt_descriptors[inlink->format];
>
> av_image_fill_max_pixsteps(over->max_plane_step, NULL, pix_desc);
> over->hsub = pix_desc->log2_chroma_w;
> over->vsub = pix_desc->log2_chroma_h;
>
> + over->inout_is_rgb = 0;
> + switch (inlink->format)
> + {
> + case PIX_FMT_RGB24:
> + case PIX_FMT_BGR24:
> + case PIX_FMT_RGBA:
> + case PIX_FMT_BGRA:
> + case PIX_FMT_ARGB:
> + case PIX_FMT_ABGR:
> + over->inout_is_rgb = 1;
> +
> + over->inout_is_alpha = 0;
> + switch (inlink->format)
> + {
> + case PIX_FMT_RGBA:
> + case PIX_FMT_BGRA:
> + case PIX_FMT_ARGB:
> + case PIX_FMT_ABGR:
> + over->inout_is_alpha = 1;
> + av_log(ctx, AV_LOG_INFO,
> + "main has alpha\n");
> + break;
> + }
> +
> + over->inout_r_location = 0;
> + over->inout_g_location = 1;
> + over->inout_b_location = 2;
> + over->inout_a_location = 3;
> + switch (inlink->format)
> + {
> + case PIX_FMT_BGR24:
> + case PIX_FMT_BGRA:
> + over->inout_r_location = 2;
> + over->inout_b_location = 0;
> + break;
> + case PIX_FMT_ARGB:
> + over->inout_r_location = 1;
> + over->inout_g_location = 2;
> + over->inout_b_location = 3;
> + over->inout_a_location = 0;
> + break;
> + case PIX_FMT_ABGR:
> + over->inout_r_location = 3;
> + over->inout_g_location = 2;
> + over->inout_b_location = 1;
> + over->inout_a_location = 0;
> + }
> + av_log(ctx, AV_LOG_INFO,
> + "main pixel locations R:%d G:%d B:%d A:%d\n",
> + over->inout_r_location, over->inout_g_location,
> + over->inout_b_location, over->inout_a_location);
> + break;
> + }
> +
>
> return 0;
> }
>
> @@ -147,7 +232,45 @@
> NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> goto fail;
> over->x = res;
> -
> +
> + over->blend_is_rgb = 0;
> + switch (inlink->format)
> + {
> + case PIX_FMT_RGBA:
> + case PIX_FMT_BGRA:
> + case PIX_FMT_ARGB:
> + case PIX_FMT_ABGR:
> + over->blend_is_rgb = 1;
> +
> + over->blend_r_location = 0;
> + over->blend_g_location = 1;
> + over->blend_b_location = 2;
> + over->blend_a_location = 3;
> + switch (inlink->format)
> + {
> + case PIX_FMT_BGRA:
> + over->blend_r_location = 2;
> + over->blend_b_location = 0;
> + break;
> + case PIX_FMT_ARGB:
> + over->blend_r_location = 1;
> + over->blend_g_location = 2;
> + over->blend_b_location = 3;
> + over->blend_a_location = 0;
> + break;
> + case PIX_FMT_ABGR:
> + over->blend_r_location = 3;
> + over->blend_g_location = 2;
> + over->blend_b_location = 1;
> + over->blend_a_location = 0;
> + }
> + av_log(ctx, AV_LOG_INFO,
> + "overlay pixel locations R:%d G:%d B:%d A:%d\n",
> + over->blend_r_location, over->blend_g_location,
> + over->blend_b_location, over->blend_a_location);
> + break;
> + }
Already noted, this can be greatly simplified.
> +
> av_log(ctx, AV_LOG_INFO,
> "main w:%d h:%d fmt:%s overlay x:%d y:%d w:%d h:%d fmt:%s\n",
> ctx->inputs[MAIN]->w, ctx->inputs[MAIN]->h,
> @@ -167,6 +290,12 @@
> (int)var_values[VAR_MAIN_W], (int)var_values[VAR_MAIN_H]);
> return AVERROR(EINVAL);
> }
> +
> + if (over->inout_is_rgb != over->blend_is_rgb) {
> + av_log(ctx, AV_LOG_ERROR,
> + "Main and overlay are not similar formats, cannot mix YUV and RGB\n");
> + return AVERROR(EINVAL);
> + }
> return 0;
Possibly I would like to avoid this kind of configuration failures, or
we're going to have lots of reports from users failing to interpret
the error message. A better approach is defining a configuration
parameter for setting the colorspace class, a yet better solution
would be to extend the framework to manage efficiently such
situations.
>
> fail:
> @@ -187,7 +316,7 @@
> av_gcd((int64_t)tb1.num * tb2.den,
> (int64_t)tb2.num * tb1.den),
> (int64_t)tb1.den * tb2.den, INT_MAX);
> - av_log(ctx, AV_LOG_INFO,
> + av_log(ctx, AV_LOG_DEBUG,
spurios hunk
> "main_tb:%d/%d overlay_tb:%d/%d -> tb:%d/%d exact:%d\n",
> tb1.num, tb1.den, tb2.num, tb2.den, tb->num, tb->den, exact);
> if (!exact)
> @@ -250,26 +379,77 @@
> int overlay_end_y = y+h;
> int slice_end_y = slice_y+slice_h;
> int end_y, start_y;
> -
> + uint8_t m;
Uhm, please meaningful name.
--
FFmpeg = Fierce Frightening Meaningless Puristic EnGine
More information about the ffmpeg-devel
mailing list