[FFmpeg-devel] [PATCH] Fix filters permissions
Stefano Sabatini
stefasab at gmail.com
Thu Aug 16 16:36:34 CEST 2012
On date Tuesday 2012-08-14 20:45:52 +0200, Nicolas George encoded:
> Hi.
>
> The attached patches try to fix the use of tricky permissions in filters.
> The filters that have a FATE test still pass it. There are probably a few
> additional enhancements that could be made here or there.
>
> Regards,
>
> --
> Nicolas George
> From 176632a9552b70209deec69a33657eb0c0bf771c Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Tue, 14 Aug 2012 18:34:36 +0200
> Subject: [PATCH 01/26] lavfi: grant all permissions on mallocated video
> buffers.
nit+: lavfi/video
>
> The permissions not requested by the filter that created
> the buffer may be useful for a later filter and avoid a copy.
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavfilter/video.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libavfilter/video.c b/libavfilter/video.c
> index b7a8b86..7a6a928 100644
> --- a/libavfilter/video.c
> +++ b/libavfilter/video.c
> @@ -39,6 +39,10 @@ AVFilterBufferRef *ff_default_get_video_buffer(AVFilterLink *link, int perms, in
> int i;
> AVFilterBufferRef *picref = NULL;
> AVFilterPool *pool = link->pool;
> + int full_perms = AV_PERM_READ | AV_PERM_WRITE | AV_PERM_PRESERVE |
> + AV_PERM_REUSE | AV_PERM_REUSE2 | AV_PERM_ALIGN;
Is there a special reason for which NEG_LINESIZES is not used?
> +
> + av_assert1(!(perms & ~(full_perms | AV_PERM_NEG_LINESIZES)));
>
> if (pool) {
> for (i = 0; i < POOL_SIZE; i++) {
> @@ -49,7 +53,7 @@ AVFilterBufferRef *ff_default_get_video_buffer(AVFilterLink *link, int perms, in
> pool->count--;
> picref->video->w = w;
> picref->video->h = h;
> - picref->perms = perms | AV_PERM_READ;
> + picref->perms = full_perms;
> picref->format = link->format;
> pic->refcount = 1;
> memcpy(picref->data, pic->data, sizeof(picref->data));
> @@ -68,7 +72,7 @@ AVFilterBufferRef *ff_default_get_video_buffer(AVFilterLink *link, int perms, in
> return NULL;
>
> picref = avfilter_get_video_buffer_ref_from_arrays(data, linesize,
> - perms, w, h, link->format);
> + full_perms, w, h, link->format);
> if (!picref) {
> av_free(data[0]);
> return NULL;
> --
> 1.7.10.4
>
> From 178992739bc8c85ead28a1aca6747ff934a2efe7 Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Tue, 14 Aug 2012 18:52:43 +0200
> Subject: [PATCH 02/26] lavfi: fix erroneous use of AV_PERM_PRESERVE in
> ff_inplace_start_frame.
nit+: lavfi/video
>
> ff_inplace_start_frame looks useless anyway.
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavfilter/video.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavfilter/video.c b/libavfilter/video.c
> index 7a6a928..5fb1949 100644
> --- a/libavfilter/video.c
> +++ b/libavfilter/video.c
> @@ -169,7 +169,7 @@ int ff_inplace_start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref)
> AVFilterBufferRef *outpicref = NULL, *for_next_filter;
> int ret = 0;
>
> - if ((inpicref->perms & AV_PERM_WRITE) && !(inpicref->perms & AV_PERM_PRESERVE)) {
> + if (inpicref->perms & AV_PERM_WRITE) {
> outpicref = avfilter_ref_buffer(inpicref, ~0);
> if (!outpicref)
> return AVERROR(ENOMEM);
> --
> 1.7.10.4
>
> From ccd2ad3e3bf7289349032ea90e0a260bc3a7af91 Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Tue, 14 Aug 2012 18:35:50 +0200
> Subject: [PATCH 03/26] lavfi: grant all permissions on mallocated audio
> buffers.
nit+: lavfi/audio
In general I like the module/component syntax, but I won't enforce
that upon you if you dislike it.
> The permissions not requested by the filter that created
> the buffer may be useful for a later filter and avoid a copy.
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavfilter/audio.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libavfilter/audio.c b/libavfilter/audio.c
> index 44d3ab7..ce14acb 100644
> --- a/libavfilter/audio.c
> +++ b/libavfilter/audio.c
> @@ -41,6 +41,10 @@ AVFilterBufferRef *ff_default_get_audio_buffer(AVFilterLink *link, int perms,
> int nb_channels = av_get_channel_layout_nb_channels(link->channel_layout);
> int planes = planar ? nb_channels : 1;
> int linesize;
> + int full_perms = AV_PERM_READ | AV_PERM_WRITE | AV_PERM_PRESERVE |
> + AV_PERM_REUSE | AV_PERM_REUSE2 | AV_PERM_ALIGN;
> +
> + av_assert1(!(perms & ~(full_perms | AV_PERM_NEG_LINESIZES)));
>
> if (!(data = av_mallocz(sizeof(*data) * planes)))
> goto fail;
> @@ -48,7 +52,7 @@ AVFilterBufferRef *ff_default_get_audio_buffer(AVFilterLink *link, int perms,
> if (av_samples_alloc(data, &linesize, nb_channels, nb_samples, link->format, 0) < 0)
> goto fail;
>
> - samplesref = avfilter_get_audio_buffer_ref_from_arrays(data, linesize, perms,
> + samplesref = avfilter_get_audio_buffer_ref_from_arrays(data, linesize, full_perms,
> nb_samples, link->format,
> link->channel_layout);
> if (!samplesref)
> --
> 1.7.10.4
>
[...]
> From 38d916811c84cba0633baa4d5479d3247e10b5f4 Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Tue, 14 Aug 2012 18:45:00 +0200
> Subject: [PATCH 19/26] vf_fps: fix permissions.
>
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavfilter/vf_fps.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
> index 09b7393..a0f305a 100644
> --- a/libavfilter/vf_fps.c
> +++ b/libavfilter/vf_fps.c
> @@ -230,7 +230,7 @@ static int end_frame(AVFilterLink *inlink)
>
> /* duplicate the frame if needed */
> if (!av_fifo_size(s->fifo) && i < delta - 1) {
> - AVFilterBufferRef *dup = avfilter_ref_buffer(buf_out, AV_PERM_READ);
> + AVFilterBufferRef *dup = avfilter_ref_buffer(buf_out, ~0);
>
> av_log(ctx, AV_LOG_DEBUG, "Duplicating frame.\n");
> if (dup)
> @@ -288,12 +288,14 @@ AVFilter avfilter_vf_fps = {
>
> .inputs = (const AVFilterPad[]) {{ .name = "default",
> .type = AVMEDIA_TYPE_VIDEO,
> + .min_perms = AV_PERM_READ | AV_PERM_PRESERVE,
> .start_frame = null_start_frame,
> .draw_slice = null_draw_slice,
> .end_frame = end_frame, },
> { .name = NULL}},
> .outputs = (const AVFilterPad[]) {{ .name = "default",
> .type = AVMEDIA_TYPE_VIDEO,
> + .rej_perms = AV_PERM_WRITE,
why this (setting a rej_perms on an output link?)
> .request_frame = request_frame,
> .config_props = config_props},
> { .name = NULL}},
> --
> 1.7.10.4
>
> From bd09e48032ad842bb5a1134fbdff437c188529eb Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Tue, 14 Aug 2012 18:45:30 +0200
> Subject: [PATCH 20/26] vf_idet: fix permissions.
>
> Only write needs to be removed, other can be left.
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavfilter/vf_idet.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/libavfilter/vf_idet.c b/libavfilter/vf_idet.c
> index 02d073d..938f4a0 100644
> --- a/libavfilter/vf_idet.c
> +++ b/libavfilter/vf_idet.c
> @@ -184,9 +184,9 @@ static int start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
> return 0;
>
> if (!idet->prev)
> - idet->prev = avfilter_ref_buffer(idet->cur, AV_PERM_READ);
> + idet->prev = avfilter_ref_buffer(idet->cur, ~0);
>
> - return ff_start_frame(ctx->outputs[0], avfilter_ref_buffer(idet->cur, AV_PERM_READ));
> + return ff_start_frame(ctx->outputs[0], avfilter_ref_buffer(idet->cur, ~0));
> }
>
> static int end_frame(AVFilterLink *link)
> @@ -327,11 +327,12 @@ AVFilter avfilter_vf_idet = {
> .start_frame = start_frame,
> .draw_slice = null_draw_slice,
> .end_frame = end_frame,
> - .rej_perms = AV_PERM_REUSE2, },
> + .min_perms = AV_PERM_PRESERVE },
> { .name = NULL}},
>
> .outputs = (const AVFilterPad[]) {{ .name = "default",
> .type = AVMEDIA_TYPE_VIDEO,
> + .rej_perms = AV_PERM_WRITE,
> .poll_frame = poll_frame,
> .request_frame = request_frame, },
> { .name = NULL}},
> --
> 1.7.10.4
>
> From 9cfef126df2f9937c38191c543f9b72e8a325585 Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Tue, 14 Aug 2012 18:46:49 +0200
> Subject: [PATCH 21/26] vf_overlay: fix permissions.
>
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavfilter/vf_overlay.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c
> index afe45b5..24c366f 100644
> --- a/libavfilter/vf_overlay.c
> +++ b/libavfilter/vf_overlay.c
> @@ -621,19 +621,18 @@ AVFilter avfilter_vf_overlay = {
> .start_frame = start_frame_main,
> .draw_slice = draw_slice_main,
> .end_frame = end_frame_main,
> - .min_perms = AV_PERM_READ,
> - .rej_perms = AV_PERM_REUSE2|AV_PERM_PRESERVE, },
> + .min_perms = AV_PERM_READ | AV_PERM_WRITE | AV_PERM_PRESERVE },
> { .name = "overlay",
> .type = AVMEDIA_TYPE_VIDEO,
> .config_props = config_input_overlay,
> .start_frame = start_frame_over,
> .draw_slice = null_draw_slice,
> .end_frame = end_frame_over,
> - .min_perms = AV_PERM_READ,
> - .rej_perms = AV_PERM_REUSE2, },
> + .min_perms = AV_PERM_READ | AV_PERM_PRESERVE },
> { .name = NULL}},
> .outputs = (const AVFilterPad[]) {{ .name = "default",
> .type = AVMEDIA_TYPE_VIDEO,
> + .rej_perms = AV_PERM_WRITE,
> .config_props = config_output,
> .request_frame = request_frame, },
> { .name = NULL}},
> --
> 1.7.10.4
>
> From 593c35eb83be036dfb990cf3765579c8f9ef9a2a Mon Sep 17 00:00:00 2001
> From: Nicolas George <nicolas.george at normalesup.org>
> Date: Tue, 14 Aug 2012 18:49:17 +0200
> Subject: [PATCH 23/26] vf_select: require AV_PERM_PRESERVE.
>
> This is only necessary because of the poll_frame implementation.
> Removing it altogether would be another solution.
Regarding this, I wonder if this solution is already possible (I also
had a decimate filter which duplicate the select logic, which I'd like
to commit).
>
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
> libavfilter/vf_select.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libavfilter/vf_select.c b/libavfilter/vf_select.c
> index 4d19b33..05f3953 100644
> --- a/libavfilter/vf_select.c
> +++ b/libavfilter/vf_select.c
> @@ -415,6 +415,7 @@ AVFilter avfilter_vf_select = {
> .inputs = (const AVFilterPad[]) {{ .name = "default",
> .type = AVMEDIA_TYPE_VIDEO,
> .get_video_buffer = ff_null_get_video_buffer,
> + .min_perms = AV_PERM_PRESERVE,
> .config_props = config_input,
> .start_frame = start_frame,
> .draw_slice = draw_slice,
> --
> 1.7.10.4
>
[...]
Looks good otherwise, of course assuming it has been tested.
--
FFmpeg = Fostering and Faithless Minimalistic Purposeless Enlightened Gorilla
More information about the ffmpeg-devel
mailing list