* [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
@ 2022-03-01 13:27 Paul B Mahol
2022-03-03 18:30 ` Nicolas George
2022-03-06 14:08 ` Nicolas George
0 siblings, 2 replies; 9+ messages in thread
From: Paul B Mahol @ 2022-03-01 13:27 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
Fix possible hangs if (a)split filter is used in graph and one of outputs ends
earlier than others.
Then filter may never receive EOF from input provided by (a)split filter.
See ticket #9152 for commands how to reproduce issue.
---
libavfilter/split.c | 68 +++++++++++++++++++++++++++++++++------------
1 file changed, 51 insertions(+), 17 deletions(-)
diff --git a/libavfilter/split.c b/libavfilter/split.c
index 6b9b656708..98b51f976e 100644
--- a/libavfilter/split.c
+++ b/libavfilter/split.c
@@ -63,28 +63,62 @@ static av_cold int split_init(AVFilterContext *ctx)
return 0;
}
-static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
+static int activate(AVFilterContext *ctx)
{
- AVFilterContext *ctx = inlink->dst;
- int i, ret = AVERROR_EOF;
+ AVFilterLink *inlink = ctx->inputs[0];
+ AVFrame *in;
+ int status, ret;
+ int64_t pts;
- for (i = 0; i < ctx->nb_outputs; i++) {
- AVFrame *buf_out;
+ for (int i = 0; i < ctx->nb_outputs; i++) {
+ FF_FILTER_FORWARD_STATUS_BACK_ALL(ctx->outputs[i], ctx);
+ }
- if (ff_outlink_get_status(ctx->outputs[i]))
- continue;
- buf_out = av_frame_clone(frame);
- if (!buf_out) {
- ret = AVERROR(ENOMEM);
- break;
+ ret = ff_inlink_consume_frame(inlink, &in);
+ if (ret < 0)
+ return ret;
+ if (ret > 0) {
+ for (int i = 0; i < ctx->nb_outputs; i++) {
+ AVFrame *buf_out;
+
+ if (ff_outlink_get_status(ctx->outputs[i]))
+ continue;
+ buf_out = av_frame_clone(in);
+ if (!buf_out) {
+ ret = AVERROR(ENOMEM);
+ break;
+ }
+
+ ret = ff_filter_frame(ctx->outputs[i], buf_out);
+ if (ret < 0)
+ break;
}
- ret = ff_filter_frame(ctx->outputs[i], buf_out);
+ av_frame_free(&in);
if (ret < 0)
- break;
+ return ret;
+ }
+
+ if (ff_inlink_acknowledge_status(inlink, &status, &pts)) {
+ for (int i = 0; i < ctx->nb_outputs; i++) {
+ if (ff_outlink_get_status(ctx->outputs[i]))
+ continue;
+ ff_outlink_set_status(ctx->outputs[i], status, pts);
+ }
+ return 0;
}
- av_frame_free(&frame);
- return ret;
+
+ for (int i = 0; i < ctx->nb_outputs; i++) {
+ if (ff_outlink_get_status(ctx->outputs[i]))
+ continue;
+
+ if (ff_outlink_frame_wanted(ctx->outputs[i])) {
+ ff_inlink_request_frame(inlink);
+ return 0;
+ }
+ }
+
+ return FFERROR_NOT_READY;
}
#define OFFSET(x) offsetof(SplitContext, x)
@@ -100,7 +134,6 @@ static const AVFilterPad avfilter_vf_split_inputs[] = {
{
.name = "default",
.type = AVMEDIA_TYPE_VIDEO,
- .filter_frame = filter_frame,
},
};
@@ -110,6 +143,7 @@ const AVFilter ff_vf_split = {
.priv_size = sizeof(SplitContext),
.priv_class = &split_class,
.init = split_init,
+ .activate = activate,
FILTER_INPUTS(avfilter_vf_split_inputs),
.outputs = NULL,
.flags = AVFILTER_FLAG_DYNAMIC_OUTPUTS | AVFILTER_FLAG_METADATA_ONLY,
@@ -119,7 +153,6 @@ static const AVFilterPad avfilter_af_asplit_inputs[] = {
{
.name = "default",
.type = AVMEDIA_TYPE_AUDIO,
- .filter_frame = filter_frame,
},
};
@@ -129,6 +162,7 @@ const AVFilter ff_af_asplit = {
.priv_class = &split_class,
.priv_size = sizeof(SplitContext),
.init = split_init,
+ .activate = activate,
FILTER_INPUTS(avfilter_af_asplit_inputs),
.outputs = NULL,
.flags = AVFILTER_FLAG_DYNAMIC_OUTPUTS | AVFILTER_FLAG_METADATA_ONLY,
--
2.33.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
2022-03-01 13:27 [FFmpeg-devel] [PATCH] avfilter/split: switch to activate() Paul B Mahol
@ 2022-03-03 18:30 ` Nicolas George
2022-03-03 20:58 ` Paul B Mahol
2022-03-06 14:08 ` Nicolas George
1 sibling, 1 reply; 9+ messages in thread
From: Nicolas George @ 2022-03-03 18:30 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 693 bytes --]
Paul B Mahol (12022-03-01):
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>
> Fix possible hangs if (a)split filter is used in graph and one of outputs ends
> earlier than others.
> Then filter may never receive EOF from input provided by (a)split filter.
>
> See ticket #9152 for commands how to reproduce issue.
I am trying to reproduce the issue, and I notice it hangs with sine.mp3
but not with sine.wav (my build did not have a MP3 encoder), and I find
it very strange, even with the current code. Please let me look into it
closer; but if you have an idea about why MP3 input behaves different
than WAV please let me know.
Regards,
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
2022-03-03 18:30 ` Nicolas George
@ 2022-03-03 20:58 ` Paul B Mahol
2022-03-04 13:13 ` Nicolas George
0 siblings, 1 reply; 9+ messages in thread
From: Paul B Mahol @ 2022-03-03 20:58 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On 3/3/22, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12022-03-01):
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>
>> Fix possible hangs if (a)split filter is used in graph and one of outputs
>> ends
>> earlier than others.
>> Then filter may never receive EOF from input provided by (a)split filter.
>>
>> See ticket #9152 for commands how to reproduce issue.
>
> I am trying to reproduce the issue, and I notice it hangs with sine.mp3
> but not with sine.wav (my build did not have a MP3 encoder), and I find
> it very strange, even with the current code. Please let me look into it
> closer; but if you have an idea about why MP3 input behaves different
> than WAV please let me know.
It is caused by number of sample per frame.
I tested actually with -f lavfi -i sine only.
And patch resolves issue.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
2022-03-03 20:58 ` Paul B Mahol
@ 2022-03-04 13:13 ` Nicolas George
2022-03-04 22:49 ` Paul B Mahol
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas George @ 2022-03-04 13:13 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 537 bytes --]
Paul B Mahol (12022-03-03):
> It is caused by number of sample per frame.
>
> I tested actually with -f lavfi -i sine only.
>
> And patch resolves issue.
I do not doubt it does. But even without activate, EOF should not depend
on the number of samples per frame. There is something wrong going on
there, and I want to understand what before this change makes it go
away: otherwise, we might be missing other similar bugs.
It has waited several months, a few days more will not hurt.
Regards,
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
2022-03-04 13:13 ` Nicolas George
@ 2022-03-04 22:49 ` Paul B Mahol
2022-03-05 11:04 ` Nicolas George
0 siblings, 1 reply; 9+ messages in thread
From: Paul B Mahol @ 2022-03-04 22:49 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On 3/4/22, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12022-03-03):
>> It is caused by number of sample per frame.
>>
>> I tested actually with -f lavfi -i sine only.
>>
>> And patch resolves issue.
>
> I do not doubt it does. But even without activate, EOF should not depend
> on the number of samples per frame. There is something wrong going on
> there, and I want to understand what before this change makes it go
> away: otherwise, we might be missing other similar bugs.
>
> It has waited several months, a few days more will not hurt.
>
No, wait must stop.
The issue is that EOF is never reported if there is >1 frame left in
queue before EOF for filters that do not use .activate (and use >1
inputs).
> Regards,
>
> --
> Nicolas George
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
2022-03-04 22:49 ` Paul B Mahol
@ 2022-03-05 11:04 ` Nicolas George
2022-03-05 12:56 ` Paul B Mahol
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas George @ 2022-03-05 11:04 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 543 bytes --]
Paul B Mahol (12022-03-04):
> No, wait must stop.
>
> The issue is that EOF is never reported if there is >1 frame left in
> queue before EOF for filters that do not use .activate (and use >1
> inputs).
Let me tell it one more time:
split should not require switching to activate to work, including EOF.
There is a bug somewhere, and your analysis is not enough to know
exactly where.
Until I understand what is going on exactly and if there is a framework
bug that needs fixing, I demand you hold.
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
2022-03-05 11:04 ` Nicolas George
@ 2022-03-05 12:56 ` Paul B Mahol
2022-03-05 19:37 ` Nicolas George
0 siblings, 1 reply; 9+ messages in thread
From: Paul B Mahol @ 2022-03-05 12:56 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On 3/5/22, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (12022-03-04):
>> No, wait must stop.
>>
>> The issue is that EOF is never reported if there is >1 frame left in
>> queue before EOF for filters that do not use .activate (and use >1
>> inputs).
>
> Let me tell it one more time:
>
> split should not require switching to activate to work, including EOF.
> There is a bug somewhere, and your analysis is not enough to know
> exactly where.
>
> Until I understand what is going on exactly and if there is a framework
> bug that needs fixing, I demand you hold.
>
Your patronizing and authoritarian tone is not helping you in any way.
As you have no technical understanding in general and in this special
case anyway,
will gonna apply this working solution soon.
Unless you provide clear and concise explanation why this should not
be applied as is.
My analysis was very clean, your ignorance and belittling tone is not
helping at all.
> --
> Nicolas George
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
2022-03-05 12:56 ` Paul B Mahol
@ 2022-03-05 19:37 ` Nicolas George
0 siblings, 0 replies; 9+ messages in thread
From: Nicolas George @ 2022-03-05 19:37 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 474 bytes --]
Paul B Mahol (12022-03-05):
> will gonna apply this working solution soon.
Unacceptable. And I say this for all the times you do this, once and for
all: if you push after I asked for more time to review, I will revert
and complain to the community committee.
Le me tell you, all you have won by being so annoying is that I cannot
consider looking into it right now because I am too annoyed and angry.
This is all I have to say to you.
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avfilter/split: switch to activate()
2022-03-01 13:27 [FFmpeg-devel] [PATCH] avfilter/split: switch to activate() Paul B Mahol
2022-03-03 18:30 ` Nicolas George
@ 2022-03-06 14:08 ` Nicolas George
1 sibling, 0 replies; 9+ messages in thread
From: Nicolas George @ 2022-03-06 14:08 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1282 bytes --]
Paul B Mahol (12022-03-01):
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>
> Fix possible hangs if (a)split filter is used in graph and one of outputs ends
> earlier than others.
> Then filter may never receive EOF from input provided by (a)split filter.
>
> See ticket #9152 for commands how to reproduce issue.
>
> ---
> libavfilter/split.c | 68 +++++++++++++++++++++++++++++++++------------
> 1 file changed, 51 insertions(+), 17 deletions(-)
Ok, the problem is in forward_status_change() in avfilter.c: it makes a
request on the first output, that triggers the status change on the
input, and therefore it considers itself satisfied without making a
request on the other outputs.
It could be changed to make at least one round and requesting on each
output, but it would be inefficient for filters that have several
outputs connected to different inputs, and that is a more common case.
Therefore, I agree, filters with several outputs connected to the same
input like split need a real activate implementation.
Patch ok, I did not look very carefully at the code itself.
Can you disclose why it was so urgent to push a fix for a but seven
months old that you would rather be rude than wait a few days?
--
Nicolas George
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-06 14:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 13:27 [FFmpeg-devel] [PATCH] avfilter/split: switch to activate() Paul B Mahol
2022-03-03 18:30 ` Nicolas George
2022-03-03 20:58 ` Paul B Mahol
2022-03-04 13:13 ` Nicolas George
2022-03-04 22:49 ` Paul B Mahol
2022-03-05 11:04 ` Nicolas George
2022-03-05 12:56 ` Paul B Mahol
2022-03-05 19:37 ` Nicolas George
2022-03-06 14:08 ` Nicolas George
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git