On 2018/1/19 8:25, Mark Thompson wrote:
> On 18/01/18 05:18, Jun Zhao wrote:
>> From d157fdbffebd07066b1a857398e1067615f908b3 Mon Sep 17 00:00:00 2001
>> From: Jun Zhao <jun.z...@intel.com>
>> Date: Mon, 8 Jan 2018 16:02:35 +0800
>> Subject: [PATCH V3 2/6] lavfi: use common VPP infrastructure for
>> vf_scale_vaapi.
>>
>> Use the common VPP infrastructure re-work vf_scale_vaapi.
>>
>> Signed-off-by: Jun Zhao <jun.z...@intel.com>
>> ---
>> libavfilter/Makefile | 2 +-
>> libavfilter/vf_scale_vaapi.c | 353
>> +++++--------------------------------------
>> 2 files changed, 41 insertions(+), 314 deletions(-)
>>
>> ...
>>
>> static av_cold int scale_vaapi_init(AVFilterContext *avctx)
>> {
>> - ScaleVAAPIContext *ctx = avctx->priv;
>> + VAAPIVPPContext *vpp_ctx = avctx->priv;
>> + ScaleVAAPIContext *ctx = (ScaleVAAPIContext *)vpp_ctx->priv_data;
> The extra indirection means this is reading ctx before it's set. E.g. try:
>
> $ gdb --args ./ffmpeg_g -y -hwaccel vaapi -hwaccel_device /dev/dri/renderD129
> -hwaccel_output_format vaapi -i input.mp4 -vf 'scale_vaapi=format=invalid' -f
> null -
> ...
> Thread 1 "ffmpeg_g" received signal SIGSEGV, Segmentation fault.
> 0x0000555556927515 in format_line (avcl=0x55555843b950, level=16,
> fmt=0x555556984f69 "Invalid output format.\n", vl=0x7fffffffcfb0,
> part=0x7fffffffbf20, print_prefix=0x55555708c708 <print_prefix>,
> type=0x7fffffffbb18) at src/libavutil/log.c:258
> 258 AVClass** parent = *(AVClass ***) (((uint8_t *) avcl) +
> (gdb) bt
> #0 0x0000555556927515 in format_line (avcl=0x55555843b950, level=16,
> fmt=0x555556984f69 "Invalid output format.\n", vl=0x7fffffffcfb0,
> part=0x7fffffffbf20, print_prefix=0x55555708c708 <print_prefix>,
> type=0x7fffffffbb18) at src/libavutil/log.c:258
> #1 0x00005555569278da in av_log_default_callback (ptr=0x55555843b950,
> level=16, fmt=0x555556984f69 "Invalid output format.\n", vl=0x7fffffffcfb0)
> at src/libavutil/log.c:320
> #2 0x0000555556927d05 in av_vlog (avcl=0x55555843b950, level=16,
> fmt=0x555556984f69 "Invalid output format.\n", vl=0x7fffffffcfb0) at
> src/libavutil/log.c:377
> #3 0x0000555556927cc4 in av_log (avcl=0x55555843b950, level=16,
> fmt=0x555556984f69 "Invalid output format.\n") at src/libavutil/log.c:369
> #4 0x0000555555844bac in scale_vaapi_init (avctx=0x55555836ad80) at
> src/libavfilter/vf_scale_vaapi.c:151
>
>
> I still think having VAAPIVPPContext as the first element of
> ScaleVAAPIContext would be cleaner.
>
This issue have been fix in local, if we put VAAPIVPPContext as the
first element of ScaleVAAPIContex, I can't find a way to reuse VPP
common APP.
e,g, for: scale_vaapi_vpp_config_input, need to implement as: (the first
version )
static int scale_vaapi_config_input(AVFilterLink *inlink)
{
AVFilterContext *avctx = inlink->dst;
ScaleVAAPIContext *ctx = avctx->priv;
VAAPIVPPContext *vpp_ctx = ctx->vpp_ctx; return vaapi_vpp_config_input(inlink,
vpp_ctx);
}
And I don't like this part, and I think if we use a common part
(VAAPIVPPContext) + a specific part like (ScaleVPPContext), we can get
more clean abstract.
>>
>> - ctx->va_config = VA_INVALID_ID;
>> - ctx->va_context = VA_INVALID_ID;
>> - ctx->valid_ids = 1;
>> + vaapi_vpp_ctx_init(vpp_ctx);
>> + vpp_ctx->pipeline_uninit = vaapi_vpp_pipeline_uninit;
>>
>> if (ctx->output_format_string) {
>> - ctx->output_format = av_get_pix_fmt(ctx->output_format_string);
>> - if (ctx->output_format == AV_PIX_FMT_NONE) {
>> + vpp_ctx->output_format = av_get_pix_fmt(ctx->output_format_string);
>> + if (vpp_ctx->output_format == AV_PIX_FMT_NONE) {
>> av_log(ctx, AV_LOG_ERROR, "Invalid output format.\n");
>> return AVERROR(EINVAL);
>> }
>> } else {
>> // Use the input format once that is configured.
>> - ctx->output_format = AV_PIX_FMT_NONE;
>> + vpp_ctx->output_format = AV_PIX_FMT_NONE;
>> }
>>
>> return 0;
>> }
>>
>> -static av_cold void scale_vaapi_uninit(AVFilterContext *avctx)
>> -{
>> - ScaleVAAPIContext *ctx = avctx->priv;
>> -
>> - if (ctx->valid_ids)
>> - scale_vaapi_pipeline_uninit(ctx);
>> -
>> - av_buffer_unref(&ctx->input_frames_ref);
>> - av_buffer_unref(&ctx->output_frames_ref);
>> - av_buffer_unref(&ctx->device_ref);
>> -}
>> -
>> -
>> -#define OFFSET(x) offsetof(ScaleVAAPIContext, x)
>> +#define OFFSET(x) (offsetof(VAAPIVPPContext, priv_data) + \
>> + offsetof(ScaleVAAPIContext, x))
>> #define FLAGS (AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM)
>> static const AVOption scale_vaapi_options[] = {
>> { "w", "Output video width",
>> @@ -441,19 +172,14 @@ static const AVOption scale_vaapi_options[] = {
>> { NULL },
>> };
>>
>> -static const AVClass scale_vaapi_class = {
>> - .class_name = "scale_vaapi",
>> - .item_name = av_default_item_name,
>> - .option = scale_vaapi_options,
>> - .version = LIBAVUTIL_VERSION_INT,
>> -};
>> +AVFILTER_DEFINE_CLASS(scale_vaapi);
>>
>> static const AVFilterPad scale_vaapi_inputs[] = {
>> {
>> .name = "default",
>> .type = AVMEDIA_TYPE_VIDEO,
>> .filter_frame = &scale_vaapi_filter_frame,
>> - .config_props = &scale_vaapi_config_input,
>> + .config_props = &vaapi_vpp_config_input,
>> },
>> { NULL }
>> };
>> @@ -470,10 +196,11 @@ static const AVFilterPad scale_vaapi_outputs[] = {
>> AVFilter ff_vf_scale_vaapi = {
>> .name = "scale_vaapi",
>> .description = NULL_IF_CONFIG_SMALL("Scale to/from VAAPI surfaces."),
>> - .priv_size = sizeof(ScaleVAAPIContext),
>> + .priv_size = (sizeof(VAAPIVPPContext) +
>> + sizeof(ScaleVAAPIContext)),
>> .init = &scale_vaapi_init,
>> - .uninit = &scale_vaapi_uninit,
>> - .query_formats = &scale_vaapi_query_formats,
>> + .uninit = &vaapi_vpp_ctx_uninit,
>> + .query_formats = &vaapi_vpp_query_formats,
>> .inputs = scale_vaapi_inputs,
>> .outputs = scale_vaapi_outputs,
>> .priv_class = &scale_vaapi_class,
>> --
>> 2.14.1
>>
> Otherwise LGTM.
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
