[FFmpeg-devel] [PATCH V6] libavfilter: add transpose_vaapi filter
Mark Thompson
sw at jkqxz.net
Thu Dec 20 23:46:00 EET 2018
On 18/12/2018 09:54, Zachary Zhou wrote:
> Swap width and height when do clock/cclock rotation
> Add reveral/reveral_flip options
>
> ffmpeg -hwaccel vaapi -vaapi_device /dev/dri/renderD128
> -hwaccel_output_format vaapi -i input.264 -vf "transpose_vaapi=clock_flip"
> -c:v h264_vaapi output.h264
>
> Signed-off-by: Zachary Zhou <zachary.zhou at intel.com>
> ---
> configure | 2 +
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/vf_transpose_vaapi.c | 360 +++++++++++++++++++++++++++++++
> 4 files changed, 364 insertions(+)
> create mode 100644 libavfilter/vf_transpose_vaapi.c
>
> ...
> diff --git a/libavfilter/vf_transpose_vaapi.c b/libavfilter/vf_transpose_vaapi.c
> new file mode 100644
> index 0000000000..a2bf245da9
> --- /dev/null
> +++ b/libavfilter/vf_transpose_vaapi.c
> @@ -0,0 +1,360 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include <string.h>
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +
> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "vaapi_vpp.h"
> +
> +typedef enum {
> + TRANSPOSE_PT_TYPE_NONE,
> + TRANSPOSE_PT_TYPE_LANDSCAPE,
> + TRANSPOSE_PT_TYPE_PORTRAIT,
> +} PassthroughType;
> +
> +enum TransposeDir {
> + TRANSPOSE_CCLOCK_FLIP,
> + TRANSPOSE_CLOCK,
> + TRANSPOSE_CCLOCK,
> + TRANSPOSE_CLOCK_FLIP,
> + TRANSPOSE_REVERAL,
> + TRANSPOSE_REVERAL_FLIP,
"REVERAL"? I guess that's meant to be "REVERSAL". I'm not convinced it's quite the right word, though. REVERSAL_FLIP is standalone as hflip - if you included vflip as well (the missing case from the eight possible transforms, excluding identity) then that name might be better?
Also, please use the enums in transpose.h directly rather than duplicating them.
> +};
> +
> +typedef struct TransposeVAAPIContext {
> + VAAPIVPPContext vpp_ctx; // must be the first field
> + int passthrough; // PassthroughType, landscape passthrough mode enabled
> + int dir; // TransposeDir
> +
> + int rotation_state;
> + int mirror_state;
> +} TransposeVAAPIContext;
> +
> +static int transpose_vaapi_build_filter_params(AVFilterContext *avctx)
> +{
> + VAAPIVPPContext *vpp_ctx = avctx->priv;
> + TransposeVAAPIContext *ctx = avctx->priv;
> + VAStatus vas;
> + int support_flag;
> + VAProcPipelineCaps pipeline_caps;
> +
> + memset(&pipeline_caps, 0, sizeof(pipeline_caps));
> + vas = vaQueryVideoProcPipelineCaps(vpp_ctx->hwctx->display,
> + vpp_ctx->va_context,
> + NULL, 0,
> + &pipeline_caps);
> + if (vas != VA_STATUS_SUCCESS) {
> + av_log(avctx, AV_LOG_ERROR, "Failed to query pipeline "
> + "caps: %d (%s).\n", vas, vaErrorStr(vas));
> + return AVERROR(EIO);
> + }
> +
> + if (!pipeline_caps.rotation_flags) {
This condition doesn't guarantee that all possibly rotations are supported - <https://github.com/intel/libva/blob/master/va/va_vpp.h#L611-L612>.
The mirror flags also need the same treatment.
> + av_log(avctx, AV_LOG_ERROR, "VAAPI driver doesn't support transpose\n");
> + return AVERROR(EINVAL);
> + }
> +
> + switch (ctx->dir) {
> + case TRANSPOSE_CCLOCK_FLIP:
> + ctx->rotation_state = VA_ROTATION_270;
> + ctx->mirror_state = VA_MIRROR_VERTICAL;
> + break;
> + case TRANSPOSE_CLOCK:
> + ctx->rotation_state = VA_ROTATION_90;
> + ctx->mirror_state = VA_MIRROR_NONE;
> + break;
> + case TRANSPOSE_CCLOCK:
> + ctx->rotation_state = VA_ROTATION_270;
> + ctx->mirror_state = VA_MIRROR_NONE;
> + break;
> + case TRANSPOSE_CLOCK_FLIP:
> + ctx->rotation_state = VA_ROTATION_90;
> + ctx->mirror_state = VA_MIRROR_VERTICAL;
> + break;
> + case TRANSPOSE_REVERAL:
> + ctx->rotation_state = VA_ROTATION_180;
> + ctx->mirror_state = VA_MIRROR_NONE;
> + break;
> + case TRANSPOSE_REVERAL_FLIP:
> + ctx->rotation_state = VA_ROTATION_180;
> + ctx->mirror_state = VA_MIRROR_VERTICAL;
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "Failed to set direction to %d\n", ctx->dir);
> + return AVERROR(EINVAL);
> + }
> +
> + support_flag = pipeline_caps.rotation_flags & (1 << ctx->rotation_state);
> + if (!support_flag) {
> + av_log(avctx, AV_LOG_ERROR, "VAAPI driver doesn't support %d\n",
> + ctx->rotation_state);
> + return AVERROR(EINVAL);
> + }
> +
> + return 0;
> +}
> +
> +static int transpose_vaapi_filter_frame(AVFilterLink *inlink, AVFrame *input_frame)
> +{
> + AVFilterContext *avctx = inlink->dst;
> + AVFilterLink *outlink = avctx->outputs[0];
> + VAAPIVPPContext *vpp_ctx = avctx->priv;
> + TransposeVAAPIContext *ctx = avctx->priv;
> + AVFrame *output_frame = NULL;
> + VASurfaceID input_surface, output_surface;
> + VARectangle input_region, output_region;
> +
> + VAProcPipelineParameterBuffer params;
> + int err;
> +
> + if (ctx->passthrough)
> + return ff_filter_frame(outlink, input_frame);
> +
> + av_log(avctx, AV_LOG_DEBUG, "Filter input: %s, %ux%u (%"PRId64").\n",
> + av_get_pix_fmt_name(input_frame->format),
> + input_frame->width, input_frame->height, input_frame->pts);
> +
> + if (vpp_ctx->va_context == VA_INVALID_ID)
> + return AVERROR(EINVAL);
> +
> + input_surface = (VASurfaceID)(uintptr_t)input_frame->data[3];
> + av_log(avctx, AV_LOG_DEBUG, "Using surface %#x for transpose vpp input.\n",
> + input_surface);
> +
> + output_frame = ff_get_video_buffer(outlink, vpp_ctx->output_width,
> + vpp_ctx->output_height);
> + if (!output_frame) {
> + err = AVERROR(ENOMEM);
> + goto fail;
> + }
> +
> + output_surface = (VASurfaceID)(uintptr_t)output_frame->data[3];
> + av_log(avctx, AV_LOG_DEBUG, "Using surface %#x for transpose vpp output.\n",
> + output_surface);
> + memset(¶ms, 0, sizeof(params));
> + input_region = (VARectangle) {
> + .x = 0,
> + .y = 0,
> + .width = input_frame->width,
> + .height = input_frame->height,
> + };
> +
> + output_region = (VARectangle) {
> + .x = 0,
> + .y = 0,
> + .width = output_frame->width,
> + .height = output_frame->height,
> + };
> +
> + switch (ctx->rotation_state) {
> + case VA_ROTATION_NONE:
> + case VA_ROTATION_90:
> + case VA_ROTATION_180:
> + case VA_ROTATION_270:
> + params.rotation_state = ctx->rotation_state;
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "VAAPI doesn't support rotation %d\n",
> + ctx->rotation_state);
This case...
> + err = AVERROR(EINVAL);
> + goto fail;
> + }
> +
> + switch (ctx->mirror_state) {
> + case VA_MIRROR_HORIZONTAL:
> + case VA_MIRROR_VERTICAL:
> + case VA_MIRROR_NONE:
> + params.mirror_state = ctx->mirror_state;
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "VAAPI doesn't support mirroring %d\n",
> + ctx->mirror_state);
... and this case can't ever be hit, can they? (They are only ever set to these values in build_filter_params().) I wouldn't bother checking, but if you do then an assert would be best - EINVAL indicates that the user passed some invalid option, which isn't the case here.
> + err = AVERROR(EINVAL);
> + goto fail;
> + }
> +
> + if (vpp_ctx->nb_filter_buffers) {
> + params.filters = &vpp_ctx->filter_buffers[0];
> + params.num_filters = vpp_ctx->nb_filter_buffers;
> + }
> +
> + params.surface = input_surface;
> + params.surface_region = &input_region;
> + params.surface_color_standard =
> + ff_vaapi_vpp_colour_standard(input_frame->colorspace);
> +
> + params.output_region = &output_region;
> +#define BG_BLACK 0xff000000
> + params.output_background_color = BG_BLACK;
> +#undef BG_BLACK
I'm not sure the inline constant adds anything. All the other VAAPI filters have it too, so maybe a separate patch to add the constant to vaapi_vpp.h (VAAPI_VPP_BACKGROUND_BLACK?) and use it everywhere would make sense?
> + params.output_color_standard = params.surface_color_standard;
> +
> + params.pipeline_flags = 0;
> + params.filter_flags = VA_FRAME_PICTURE;
> +
> + err = ff_vaapi_vpp_render_picture(avctx, ¶ms, output_surface);
> + if (err < 0)
> + goto fail;
> +
> + err = av_frame_copy_props(output_frame, input_frame);
> + if (err < 0)
> + goto fail;
> + av_frame_free(&input_frame);
> +
> + av_log(avctx, AV_LOG_DEBUG, "Filter output: %s, %ux%u (%"PRId64").\n",
> + av_get_pix_fmt_name(output_frame->format),
> + output_frame->width, output_frame->height, output_frame->pts);
> +
> + return ff_filter_frame(outlink, output_frame);
> +
> +fail:
> + av_frame_free(&input_frame);
> + av_frame_free(&output_frame);
> + return err;
> +}
> +
> +static av_cold int transpose_vaapi_init(AVFilterContext *avctx)
> +{
> + VAAPIVPPContext *vpp_ctx = avctx->priv;
> +
> + ff_vaapi_vpp_ctx_init(avctx);
> + vpp_ctx->pipeline_uninit = ff_vaapi_vpp_pipeline_uninit;
> + vpp_ctx->build_filter_params = transpose_vaapi_build_filter_params;
> + vpp_ctx->output_format = AV_PIX_FMT_NONE;
> +
> + return 0;
> +}
> +
> +static void transpose_vaapi_vpp_ctx_uninit(AVFilterContext *avctx)
> +{
> + return ff_vaapi_vpp_ctx_uninit(avctx);
> +}
> +
> +static int transpose_vaapi_vpp_query_formats(AVFilterContext *avctx)
> +{
> + return ff_vaapi_vpp_query_formats(avctx);
> +}
> +
> +static int transpose_vaapi_vpp_config_input(AVFilterLink *inlink)
> +{
> + return ff_vaapi_vpp_config_input(inlink);
> +}
> +
> +static int transpose_vaapi_vpp_config_output(AVFilterLink *outlink)
> +{
> + AVFilterContext *avctx = outlink->src;
> + VAAPIVPPContext *vpp_ctx = avctx->priv;
> + TransposeVAAPIContext *ctx = avctx->priv;
> + AVFilterLink *inlink = avctx->inputs[0];
> +
> + if ((inlink->w >= inlink->h && ctx->passthrough == TRANSPOSE_PT_TYPE_LANDSCAPE) ||
> + (inlink->w <= inlink->h && ctx->passthrough == TRANSPOSE_PT_TYPE_PORTRAIT)) {
> + av_log(avctx, AV_LOG_VERBOSE,
> + "w:%d h:%d -> w:%d h:%d (passthrough mode)\n",
> + inlink->w, inlink->h, inlink->w, inlink->h);
> + return ff_vaapi_vpp_config_output(outlink);
> + }
> +
> + ctx->passthrough = TRANSPOSE_PT_TYPE_NONE;
> +
> + switch (ctx->dir) {
> + case TRANSPOSE_CCLOCK_FLIP:
> + case TRANSPOSE_CCLOCK:
> + case TRANSPOSE_CLOCK:
> + case TRANSPOSE_CLOCK_FLIP:
> + vpp_ctx->output_width = avctx->inputs[0]->h;
> + vpp_ctx->output_height = avctx->inputs[0]->w;
> + av_log(avctx, AV_LOG_DEBUG, "swap width and height for clock/cclock rotation\n");
> + break;
> + default:
> + break;
> + }
> +
> + return ff_vaapi_vpp_config_output(outlink);
> +}
> +
> +static AVFrame *get_video_buffer(AVFilterLink *inlink, int w, int h)
> +{
> + TransposeVAAPIContext *ctx = inlink->dst->priv;
> +
> + return ctx->passthrough ?
> + ff_null_get_video_buffer(inlink, w, h) :
> + ff_default_get_video_buffer(inlink, w, h);
> +}
> +
> +#define OFFSET(x) offsetof(TransposeVAAPIContext, x)
> +#define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_FILTERING_PARAM)
> +static const AVOption transpose_vaapi_options[] = {
> + { "dir", "set transpose direction", OFFSET(dir), AV_OPT_TYPE_INT, { .i64 = TRANSPOSE_CCLOCK_FLIP }, 0, 5, FLAGS, "dir" },
> + { "cclock_flip", "rotate counter-clockwise with vertical flip", 0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK_FLIP }, .flags=FLAGS, .unit = "dir" },
> + { "clock", "rotate clockwise", 0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK }, .flags=FLAGS, .unit = "dir" },
> + { "cclock", "rotate counter-clockwise", 0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK }, .flags=FLAGS, .unit = "dir" },
> + { "clock_flip", "rotate clockwise with vertical flip", 0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP }, .flags=FLAGS, .unit = "dir" },
> + { "reveral", "rotate clockwise 180 degrees", 0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_REVERAL }, .flags=FLAGS, .unit = "dir" },
> + { "reveral_flip", "rotate clockwise 180 degrees with vertical flip", 0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_REVERAL_FLIP }, .flags=FLAGS, .unit = "dir" },
"rotate clockwise 180 degrees with vertical flip" is I think better known as "flip horizontally".
> +
> + { "passthrough", "do not apply transposition if the input matches the specified geometry",
> + OFFSET(passthrough), AV_OPT_TYPE_INT, {.i64=TRANSPOSE_PT_TYPE_NONE}, 0, INT_MAX, FLAGS, "passthrough" },
> + { "none", "always apply transposition", 0, AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE}, INT_MIN, INT_MAX, FLAGS, "passthrough" },
> + { "portrait", "preserve portrait geometry", 0, AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT}, INT_MIN, INT_MAX, FLAGS, "passthrough" },
> + { "landscape", "preserve landscape geometry", 0, AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN, INT_MAX, FLAGS, "passthrough" },
> +
> + { NULL }
> +};
> +
> +
> +AVFILTER_DEFINE_CLASS(transpose_vaapi);
> +
> +static const AVFilterPad transpose_vaapi_inputs[] = {
> + {
> + .name = "default",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .filter_frame = &transpose_vaapi_filter_frame,
> + .get_video_buffer = get_video_buffer,
> + .config_props = &transpose_vaapi_vpp_config_input,
> + },
> + { NULL }
> +};
> +
> +static const AVFilterPad transpose_vaapi_outputs[] = {
> + {
> + .name = "default",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .config_props = &transpose_vaapi_vpp_config_output,
> + },
> + { NULL }
> +};
> +
> +AVFilter ff_vf_transpose_vaapi = {
> + .name = "transpose_vaapi",
> + .description = NULL_IF_CONFIG_SMALL("VAAPI VPP for transpose"),
> + .priv_size = sizeof(TransposeVAAPIContext),
> + .init = &transpose_vaapi_init,
> + .uninit = &transpose_vaapi_vpp_ctx_uninit,
> + .query_formats = &transpose_vaapi_vpp_query_formats,
> + .inputs = transpose_vaapi_inputs,
> + .outputs = transpose_vaapi_outputs,
> + .priv_class = &transpose_vaapi_class,
> + .flags_internal = FF_FILTER_FLAG_HWFRAME_AWARE,
> +};
>
- Mark
More information about the ffmpeg-devel
mailing list