| Message ID | 20170717141926.7442-7-george@nsup.org |
|---|---|
| State | Accepted |
| Commit | 0dd8320e16bcdbe6b928e99489cf47abd16d3255 |
| Headers | show |
On 7/17/17, Nicolas George <george@nsup.org> wrote: > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavfilter/Makefile | 4 ++-- > libavfilter/vf_stack.c | 32 +++++++++++++------------------- > 2 files changed, 15 insertions(+), 21 deletions(-) > > > Works as expected. Including shortest. > And no longer subject to bufferqueue overflows. > > lgtm
On 7/29/17, Paul B Mahol <onemda@gmail.com> wrote: > On 7/17/17, Nicolas George <george@nsup.org> wrote: >> Signed-off-by: Nicolas George <george@nsup.org> >> --- >> libavfilter/Makefile | 4 ++-- >> libavfilter/vf_stack.c | 32 +++++++++++++------------------- >> 2 files changed, 15 insertions(+), 21 deletions(-) >> >> >> Works as expected. Including shortest. >> And no longer subject to bufferqueue overflows. >> >> > > lgtm When this and others gonna be applied?
Le primidi 11 thermidor, an CCXXV, Paul B Mahol a écrit :
> When this and others gonna be applied?
As soon as I have time and motivation to work on it, rebase it, check I
have no fix staged, re-run fate, etc.
Regards,
On Mon, Jul 17, 2017 at 3:19 PM, Nicolas George <george@nsup.org> wrote: > > Signed-off-by: Nicolas George <george@nsup.org> > --- > libavfilter/Makefile | 4 ++-- > libavfilter/vf_stack.c | 32 +++++++++++++------------------- > 2 files changed, 15 insertions(+), 21 deletions(-) > > > Works as expected. Including shortest. > And no longer subject to bufferqueue overflows. Gentlemen, I am sorry to report that this particular patch to lavfi/vf_stack.c breaks FFmpeg on-screen output when a vertical stack (vstack) follows a horizontal stack (hstack). Below, please find a chain to -filter_complex, cut down from a much more complex chain, that reproduces the problem. FFmpeg here is compiled for Windows using mingw-w64 tool chain on Bash for Windows. I usually make this up nearly every day without incident. At commit 4e0e9ce2dc67a94c98d40a46e91fe5aa53ad0376 ("lavfi/framesync2: implement "activate" design"), the following script downloads an audio stream, and successfully produces three visual representations using FFmpeg filters, output to the 'SDL' device. 'opengl' also works. In this script, the avectorscope and showfreqs filters are horizontally stacked; then this combination is stacked vertically with an output from the ebur128 filter. ffmpeg -v debug -i 'http://s3.yesstreaming.net:7001/;stream/' -filter_complex "[a0]asplit=3[a][b][c]; [a]avectorscope=size=512x512[z]; [b]showfreqs[y]; [z][y]hstack[n]; [c]ebur128=video=1[d][e]; [d]scale=1536:512[g]; [n][g]vstack; [e]anullsink" -f SDL Test But once the patch to lavfi_vf_stack is applied, at commit 0dd8320e16bcdbe6b928e99489cf47abd16d3255, the same script results in a blank, white display window using the SDL device, and a blank black window using opengl. The text output from the ebur128 filter stops after one line. Merely applying a single hstack or vstack works: but the two together fail after this point: https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/0dd8320e16bcdbe6b928e99489cf47abd16d3255 ("lavfi/vf_stack: move to "activate" design") Furthermore, compiling from today's git master but reversing ONLY the above patch to vf_stack.c also results in a working FFmpeg when given the above test case. Both debug outputs from the parser are identical: [Parsed_asplit_0 @ 0000022917a3eb60] Setting 'outputs' to value '3' [Parsed_avectorscope_1 @ 0000022917a3e9c0] Setting 'size' to value '512x512' [Parsed_ebur128_4 @ 0000022917a3ec20] Setting 'video' to value '1' [Parsed_ebur128_4 @ 0000022917a3ec20] EBU +9 scale [Parsed_scale_5 @ 0000022917a3e820] Setting 'w' to value '1536' [Parsed_scale_5 @ 0000022917a3e820] Setting 'h' to value '512' [Parsed_scale_5 @ 0000022917a3e820] w:1536 h:512 flags:'bilinear' interl:0 [graph_0_in_0_0 @ 0000022917a3f2a0] Setting 'time_base' to value '1/44100' [graph_0_in_0_0 @ 0000022917a3f2a0] Setting 'sample_rate' to value '44100' [graph_0_in_0_0 @ 0000022917a3f2a0] Setting 'sample_fmt' to value 's16p' [graph_0_in_0_0 @ 0000022917a3f2a0] Setting 'channel_layout' to value '0x3' [graph_0_in_0_0 @ 0000022917a3f2a0] tb:1/44100 samplefmt:s16p samplerate:44100 chlayout:0x3 [Parsed_avectorscope_1 @ 0000022917a3e9c0] auto-inserting filter 'auto_resampler_0' between the filter 'Parsed_asplit_0' and the filter 'Parsed_avectorscope_1' [Parsed_showfreqs_2 @ 0000022917a3e740] auto-inserting filter 'auto_resampler_1' between the filter 'Parsed_asplit_0' and the filter 'Parsed_showfreqs_2' [Parsed_ebur128_4 @ 0000022917a3ec20] auto-inserting filter 'auto_resampler_2' between the filter 'Parsed_asplit_0' and the filter 'Parsed_ebur128_4' [AVFilterGraph @ 0000022917a3d540] query_formats: 10 queried, 12 merged, 9 already done, 0 delayed [auto_resampler_0 @ 0000022917a3e8e0] picking s16 out of 2 ref:s16p [auto_resampler_2 @ 0000022917a3f440] [SWR @ 0000022917f34f80] Using fltp internally between filters [auto_resampler_2 @ 0000022917a3f440] ch:2 chl:stereo fmt:s16p r:44100Hz -> ch:2 chl:stereo fmt:dbl r:48000Hz [auto_resampler_0 @ 0000022917a3e8e0] [SWR @ 0000022917a894c0] Using s16p internally between filters [auto_resampler_0 @ 0000022917a3e8e0] ch:2 chl:stereo fmt:s16p r:44100Hz -> ch:2 chl:stereo fmt:s16 r:44100Hz [auto_resampler_1 @ 0000022917a3ea80] [SWR @ 0000022917f1ffc0] Using s16p internally between filters [auto_resampler_1 @ 0000022917a3ea80] ch:2 chl:stereo fmt:s16p r:44100Hz -> ch:2 chl:stereo fmt:fltp r:44100Hz [Parsed_hstack_3 @ 0000022917a3f1e0] [framesync @ 0000022917a2e608] Selected 1/44100 time base [Parsed_hstack_3 @ 0000022917a3f1e0] [framesync @ 0000022917a2e608] Sync level 1 [swscaler @ 0000022918081100] Forcing full internal H chroma due to input having non subsampled chroma [Parsed_scale_5 @ 0000022917a3e820] w:640 h:480 fmt:rgb24 sar:1/1 -> w:1536 h:512 fmt:rgba sar:4/9 flags:0x2 [Parsed_vstack_6 @ 0000022917a3ed00] [framesync @ 00000229177dd5c8] Selected 1/1000000 time base [Parsed_vstack_6 @ 0000022917a3ed00] [framesync @ 00000229177dd5c8] Sync level 1 [sdl,sdl2 @ 0000022917a4df40] w:1536 h:1024 fmt:rgba -> w:1536 h:1024 I hope this information might be helpful to you. Kind regards, JW
Le nonidi 19 thermidor, an CCXXV, John Warburton a écrit : > Gentlemen, I am sorry to report that this particular patch to > lavfi/vf_stack.c breaks FFmpeg on-screen output when a vertical stack > (vstack) follows a horizontal stack (hstack). Below, please find a > chain to -filter_complex, cut down from a much more complex chain, > that reproduces the problem. > > FFmpeg here is compiled for Windows using mingw-w64 tool chain on Bash > for Windows. I usually make this up nearly every day without incident. > > At commit 4e0e9ce2dc67a94c98d40a46e91fe5aa53ad0376 ("lavfi/framesync2: > implement "activate" design"), the following script downloads an audio > stream, and successfully produces three visual representations using > FFmpeg filters, output to the 'SDL' device. 'opengl' also works. > > In this script, the avectorscope and showfreqs filters are > horizontally stacked; then this combination is stacked vertically with > an output from the ebur128 filter. > > ffmpeg -v debug -i 'http://s3.yesstreaming.net:7001/;stream/' > -filter_complex "[a0]asplit=3[a][b][c]; > [a]avectorscope=size=512x512[z]; [b]showfreqs[y]; [z][y]hstack[n]; > [c]ebur128=video=1[d][e]; [d]scale=1536:512[g]; [n][g]vstack; > [e]anullsink" -f SDL Test > > But once the patch to lavfi_vf_stack is applied, at commit > 0dd8320e16bcdbe6b928e99489cf47abd16d3255, the same script results in a > blank, white display window using the SDL device, and a blank black > window using opengl. The text output from the ebur128 filter stops > after one line. Merely applying a single hstack or vstack works: but > the two together fail after this point: > > https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/0dd8320e16bcdbe6b928e99489cf47abd16d3255 > ("lavfi/vf_stack: move to "activate" design") > > Furthermore, compiling from today's git master but reversing ONLY the > above patch to vf_stack.c also results in a working FFmpeg when given > the above test case. I can reproduce the issue here. Thanks for the report. Regards,
Is ticket #6566 also related? Gyan
On Sun, Aug 6, 2017 at 4:39 PM, Gyan <gyandoshi@gmail.com> wrote:
> Is ticket #6566 also related?
This test case on ticket #6566 indeed freezes during the overlay on my
patched FFmpeg with the alteration to libav/vf_stack.c taken out. I
have not been able to test it in another way yet.
Kind regards,
JW
Le nonidi 19 thermidor, an CCXXV, Gyan a écrit :
> Is ticket #6566 also related?
No, since the changes to overlay are not yet committed and since "Buffer
queue overflow" is precisely the thing that will go away when they will
be.
Regards,
Le nonidi 19 thermidor, an CCXXV, John Warburton a écrit : > Gentlemen, I am sorry to report that this particular patch to > lavfi/vf_stack.c breaks FFmpeg on-screen output when a vertical stack > (vstack) follows a horizontal stack (hstack). Below, please find a > chain to -filter_complex, cut down from a much more complex chain, > that reproduces the problem. This issue seems fixed by these changes: https://ffmpeg.org/pipermail/ffmpeg-devel/2017-July/214306.html I thought it could only be triggered with the specific configuration caused by dualinput, but you obviously found another way. The patches in this series need some review before being pushed. Also, it would help deciding stuff if you, as a user, told us: if you had to write "scale=w=1536:h=512" instead of "scale=1536:512" to be completely sure that your scripts will not break on upgrade, would you consider that an unacceptable burden? Regards,
On Mon, Aug 7, 2017 at 2:38 PM, Nicolas George <george@nsup.org> wrote: > > Also, it would help deciding stuff if you, as a user, told us: if you > had to write "scale=w=1536:h=512" instead of "scale=1536:512" to be > completely sure that your scripts will not break on upgrade, would you > consider that an unacceptable burden? > Obviously not for scripts written now or in the future by those who follow this list, but old/delivered scripts may break and often those executing them aren't the ones who composed them. If the doc patch is being pushed, would you accept if I or others submitted abbreviated alternates for some of the long option names that exist e.g. as in loudnorm.
Le decadi 20 thermidor, an CCXXV, Gyan a écrit : > Obviously not for scripts written now or in the future by those who follow > this list, but old/delivered scripts may break and often those executing > them aren't the ones who composed them. But I hope they test them after upgrading their version of ffmpeg. > If the doc patch is being pushed, would you accept if I or others submitted > abbreviated alternates for some of the long option names that exist e.g. as > in loudnorm. I see personally no objection, as long as the aliases are reasonable. But it would eventually be up to the person who maintains each filter. Regards,
On Mon, Aug 7, 2017 at 10:08 AM, Nicolas George <george@nsup.org> wrote: > > This issue seems fixed by these changes: > > https://ffmpeg.org/pipermail/ffmpeg-devel/2017-July/214306.html > ... > Also, it would help deciding stuff if you, as a user, told us: if you > had to write "scale=w=1536:h=512" instead of "scale=1536:512" to be > completely sure that your scripts will not break on upgrade, would you > consider that an unacceptable burden? Thank you for letting me know about the incoming patch. Two of the hunks failed to apply but I'll try sorting those out later (need to go to a meeting right now). The question about abbreviating parameters to filters: I am fine about writing in the new way, but there are scripts run here regularly that will need updating if the abbreviated form is lost. However, the non-abbreviated form is far easier to read and, therefore, for others to maintain. JW
On Mon, Aug 7, 2017 at 10:08 AM, Nicolas George <george@nsup.org> wrote: > > This issue seems fixed by these changes: > > https://ffmpeg.org/pipermail/ffmpeg-devel/2017-July/214306.html > > I thought it could only be triggered with the specific configuration > caused by dualinput, but you obviously found another way. > > The patches in this series need some review before being pushed. Confirming: applying the patch in https://ffmpeg.org/pipermail/ffmpeg-devel/2017-July/214306.html causes multiple vf_stack calls to work again. It didn't apply first time because I'd incorrectly saved the patch text. Thank you very much for this suggestion! J
diff --git a/libavfilter/Makefile b/libavfilter/Makefile index 4d85f658f3..3d4e55a33c 100644 --- a/libavfilter/Makefile +++ b/libavfilter/Makefile @@ -202,7 +202,7 @@ OBJS-$(CONFIG_HISTEQ_FILTER) += vf_histeq.o OBJS-$(CONFIG_HISTOGRAM_FILTER) += vf_histogram.o OBJS-$(CONFIG_HQDN3D_FILTER) += vf_hqdn3d.o OBJS-$(CONFIG_HQX_FILTER) += vf_hqx.o -OBJS-$(CONFIG_HSTACK_FILTER) += vf_stack.o framesync.o +OBJS-$(CONFIG_HSTACK_FILTER) += vf_stack.o framesync2.o OBJS-$(CONFIG_HUE_FILTER) += vf_hue.o OBJS-$(CONFIG_HWDOWNLOAD_FILTER) += vf_hwdownload.o OBJS-$(CONFIG_HWMAP_FILTER) += vf_hwmap.o @@ -321,7 +321,7 @@ OBJS-$(CONFIG_VFLIP_FILTER) += vf_vflip.o OBJS-$(CONFIG_VIDSTABDETECT_FILTER) += vidstabutils.o vf_vidstabdetect.o OBJS-$(CONFIG_VIDSTABTRANSFORM_FILTER) += vidstabutils.o vf_vidstabtransform.o OBJS-$(CONFIG_VIGNETTE_FILTER) += vf_vignette.o -OBJS-$(CONFIG_VSTACK_FILTER) += vf_stack.o framesync.o +OBJS-$(CONFIG_VSTACK_FILTER) += vf_stack.o framesync2.o OBJS-$(CONFIG_W3FDIF_FILTER) += vf_w3fdif.o OBJS-$(CONFIG_WAVEFORM_FILTER) += vf_waveform.o OBJS-$(CONFIG_WEAVE_FILTER) += vf_weave.o diff --git a/libavfilter/vf_stack.c b/libavfilter/vf_stack.c index 03643b6f96..fa8a02257e 100644 --- a/libavfilter/vf_stack.c +++ b/libavfilter/vf_stack.c @@ -26,7 +26,7 @@ #include "avfilter.h" #include "formats.h" #include "internal.h" -#include "framesync.h" +#include "framesync2.h" #include "video.h" typedef struct StackContext { @@ -58,12 +58,6 @@ static int query_formats(AVFilterContext *ctx) return ff_set_common_formats(ctx, pix_fmts); } -static int filter_frame(AVFilterLink *inlink, AVFrame *in) -{ - StackContext *s = inlink->dst->priv; - return ff_framesync_filter_frame(&s->fs, inlink, in); -} - static av_cold int init(AVFilterContext *ctx) { StackContext *s = ctx->priv; @@ -83,7 +77,6 @@ static av_cold int init(AVFilterContext *ctx) pad.name = av_asprintf("input%d", i); if (!pad.name) return AVERROR(ENOMEM); - pad.filter_frame = filter_frame; if ((ret = ff_insert_inpad(ctx, i, &pad)) < 0) { av_freep(&pad.name); @@ -104,7 +97,7 @@ static int process_frame(FFFrameSync *fs) int i, p, ret, offset[4] = { 0 }; for (i = 0; i < s->nb_inputs; i++) { - if ((ret = ff_framesync_get_frame(&s->fs, i, &in[i], 0)) < 0) + if ((ret = ff_framesync2_get_frame(&s->fs, i, &in[i], 0)) < 0) return ret; } @@ -187,7 +180,7 @@ static int config_output(AVFilterLink *outlink) outlink->time_base = time_base; outlink->frame_rate = frame_rate; - if ((ret = ff_framesync_init(&s->fs, ctx, s->nb_inputs)) < 0) + if ((ret = ff_framesync2_init(&s->fs, ctx, s->nb_inputs)) < 0) return ret; in = s->fs.in; @@ -203,13 +196,7 @@ static int config_output(AVFilterLink *outlink) in[i].after = s->shortest ? EXT_STOP : EXT_INFINITY; } - return ff_framesync_configure(&s->fs); -} - -static int request_frame(AVFilterLink *outlink) -{ - StackContext *s = outlink->src->priv; - return ff_framesync_request_frame(&s->fs, outlink); + return ff_framesync2_configure(&s->fs); } static av_cold void uninit(AVFilterContext *ctx) @@ -217,13 +204,19 @@ static av_cold void uninit(AVFilterContext *ctx) StackContext *s = ctx->priv; int i; - ff_framesync_uninit(&s->fs); + ff_framesync2_uninit(&s->fs); av_freep(&s->frames); for (i = 0; i < ctx->nb_inputs; i++) av_freep(&ctx->input_pads[i].name); } +static int activate(AVFilterContext *ctx) +{ + StackContext *s = ctx->priv; + return ff_framesync2_activate(&s->fs); +} + #define OFFSET(x) offsetof(StackContext, x) #define FLAGS AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_FILTERING_PARAM static const AVOption stack_options[] = { @@ -237,7 +230,6 @@ static const AVFilterPad outputs[] = { .name = "default", .type = AVMEDIA_TYPE_VIDEO, .config_props = config_output, - .request_frame = request_frame, }, { NULL } }; @@ -256,6 +248,7 @@ AVFilter ff_vf_hstack = { .outputs = outputs, .init = init, .uninit = uninit, + .activate = activate, .flags = AVFILTER_FLAG_DYNAMIC_INPUTS, }; @@ -275,6 +268,7 @@ AVFilter ff_vf_vstack = { .outputs = outputs, .init = init, .uninit = uninit, + .activate = activate, .flags = AVFILTER_FLAG_DYNAMIC_INPUTS, };
Signed-off-by: Nicolas George <george@nsup.org> --- libavfilter/Makefile | 4 ++-- libavfilter/vf_stack.c | 32 +++++++++++++------------------- 2 files changed, 15 insertions(+), 21 deletions(-) Works as expected. Including shortest. And no longer subject to bufferqueue overflows.