| Message ID | 20170518133721.23603-1-mfcc64@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : > Should fix Ticket6349. > Modifying data pointer may make it unaligned. > > Also change frame->nb_samples < max to frame->nb_samples <= max. > This improves performance. Benchmark: > ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null null > old: > 25767 decicycles in take_samples, 1023 runs, 1 skips > 25422 decicycles in take_samples, 2047 runs, 1 skips > 25181 decicycles in take_samples, 4095 runs, 1 skips > 24904 decicycles in take_samples, 8191 runs, 1 skips > > new: > 550 decicycles in take_samples, 1024 runs, 0 skips > 548 decicycles in take_samples, 2048 runs, 0 skips > 545 decicycles in take_samples, 4096 runs, 0 skips > 544 decicycles in take_samples, 8192 runs, 0 skips > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libavfilter/avfilter.c | 3 ++- > libavfilter/framequeue.c | 2 ++ > libavfilter/framequeue.h | 5 +++++ > 3 files changed, 9 insertions(+), 1 deletion(-) This is an interesting idea, but... > > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c > index 08b86b0..1b6c432 100644 > --- a/libavfilter/avfilter.c > +++ b/libavfilter/avfilter.c > @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned min, unsigned max, > called with enough samples. */ > av_assert1(samples_ready(link, link->min_samples)); > frame0 = frame = ff_framequeue_peek(&link->fifo, 0); > - if (frame->nb_samples >= min && frame->nb_samples < max) { > + if (!link->fifo.samples_skipped && frame->nb_samples >= min && frame->nb_samples <= max) { > *rframe = ff_framequeue_take(&link->fifo); > return 0; > } > @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame **rframe) > *rframe = NULL; > if (!ff_inlink_check_available_frame(link)) > return 0; > + av_assert1(!link->fifo.samples_skipped); ... I am pretty sure that this assert can fail. Not with the current code, but with future filters that use the ff_inlink API directly. > frame = ff_framequeue_take(&link->fifo); > consume_update(link, frame); > *rframe = frame; > diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c > index 26bfa49..fed1118 100644 > --- a/libavfilter/framequeue.c > +++ b/libavfilter/framequeue.c > @@ -107,6 +107,7 @@ AVFrame *ff_framequeue_take(FFFrameQueue *fq) > fq->tail &= fq->allocated - 1; > fq->total_frames_tail++; > fq->total_samples_tail += b->frame->nb_samples; > + fq->samples_skipped = 0; > check_consistency(fq); > return b->frame; > } > @@ -146,5 +147,6 @@ void ff_framequeue_skip_samples(FFFrameQueue *fq, size_t samples, AVRational tim > for (i = 0; i < planes && i < AV_NUM_DATA_POINTERS; i++) > b->frame->data[i] = b->frame->extended_data[i]; > fq->total_samples_tail += samples; > + fq->samples_skipped = 1; > ff_framequeue_update_peeked(fq, 0); > } > diff --git a/libavfilter/framequeue.h b/libavfilter/framequeue.h > index 5aa2c72..c49d872 100644 > --- a/libavfilter/framequeue.h > +++ b/libavfilter/framequeue.h > @@ -100,6 +100,11 @@ typedef struct FFFrameQueue { > */ > uint64_t total_samples_tail; > > + /** > + * Indicate that samples are skipped > + */ > + int samples_skipped; > + > } FFFrameQueue; > > /** Regards,
On 5/18/17, Nicolas George <george@nsup.org> wrote: > Le nonidi 29 floreal, an CCXXV, Muhammad Faiz a ecrit : >> Should fix Ticket6349. >> Modifying data pointer may make it unaligned. >> >> Also change frame->nb_samples < max to frame->nb_samples <= max. >> This improves performance. Benchmark: >> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null >> null >> old: >> 25767 decicycles in take_samples, 1023 runs, 1 skips >> 25422 decicycles in take_samples, 2047 runs, 1 skips >> 25181 decicycles in take_samples, 4095 runs, 1 skips >> 24904 decicycles in take_samples, 8191 runs, 1 skips >> >> new: >> 550 decicycles in take_samples, 1024 runs, 0 skips >> 548 decicycles in take_samples, 2048 runs, 0 skips >> 545 decicycles in take_samples, 4096 runs, 0 skips >> 544 decicycles in take_samples, 8192 runs, 0 skips >> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >> --- >> libavfilter/avfilter.c | 3 ++- >> libavfilter/framequeue.c | 2 ++ >> libavfilter/framequeue.h | 5 +++++ >> 3 files changed, 9 insertions(+), 1 deletion(-) > > This is an interesting idea, but... > >> >> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c >> index 08b86b0..1b6c432 100644 >> --- a/libavfilter/avfilter.c >> +++ b/libavfilter/avfilter.c >> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned >> min, unsigned max, >> called with enough samples. */ >> av_assert1(samples_ready(link, link->min_samples)); >> frame0 = frame = ff_framequeue_peek(&link->fifo, 0); >> - if (frame->nb_samples >= min && frame->nb_samples < max) { >> + if (!link->fifo.samples_skipped && frame->nb_samples >= min && >> frame->nb_samples <= max) { >> *rframe = ff_framequeue_take(&link->fifo); >> return 0; >> } >> @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, >> AVFrame **rframe) >> *rframe = NULL; >> if (!ff_inlink_check_available_frame(link)) >> return 0; > >> + av_assert1(!link->fifo.samples_skipped); > > ... I am pretty sure that this assert can fail. Not with the current > code, but with future filters that use the ff_inlink API directly. Missingle single thing about future filters, and why would they use ff_inlink API directly. If you can not cooperate, have very short time to work on FFmpeg, can not stand criticism of other FFmpeg developers,.. just leave the project for once.
On 5/18/2017 12:49 PM, Paul B Mahol wrote: > On 5/18/17, Nicolas George <george@nsup.org> wrote: >> Le nonidi 29 floreal, an CCXXV, Muhammad Faiz a ecrit : >>> Should fix Ticket6349. >>> Modifying data pointer may make it unaligned. >>> >>> Also change frame->nb_samples < max to frame->nb_samples <= max. >>> This improves performance. Benchmark: >>> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null >>> null >>> old: >>> 25767 decicycles in take_samples, 1023 runs, 1 skips >>> 25422 decicycles in take_samples, 2047 runs, 1 skips >>> 25181 decicycles in take_samples, 4095 runs, 1 skips >>> 24904 decicycles in take_samples, 8191 runs, 1 skips >>> >>> new: >>> 550 decicycles in take_samples, 1024 runs, 0 skips >>> 548 decicycles in take_samples, 2048 runs, 0 skips >>> 545 decicycles in take_samples, 4096 runs, 0 skips >>> 544 decicycles in take_samples, 8192 runs, 0 skips >>> >>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >>> --- >>> libavfilter/avfilter.c | 3 ++- >>> libavfilter/framequeue.c | 2 ++ >>> libavfilter/framequeue.h | 5 +++++ >>> 3 files changed, 9 insertions(+), 1 deletion(-) >> >> This is an interesting idea, but... >> >>> >>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c >>> index 08b86b0..1b6c432 100644 >>> --- a/libavfilter/avfilter.c >>> +++ b/libavfilter/avfilter.c >>> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned >>> min, unsigned max, >>> called with enough samples. */ >>> av_assert1(samples_ready(link, link->min_samples)); >>> frame0 = frame = ff_framequeue_peek(&link->fifo, 0); >>> - if (frame->nb_samples >= min && frame->nb_samples < max) { >>> + if (!link->fifo.samples_skipped && frame->nb_samples >= min && >>> frame->nb_samples <= max) { >>> *rframe = ff_framequeue_take(&link->fifo); >>> return 0; >>> } >>> @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, >>> AVFrame **rframe) >>> *rframe = NULL; >>> if (!ff_inlink_check_available_frame(link)) >>> return 0; >> >>> + av_assert1(!link->fifo.samples_skipped); >> >> ... I am pretty sure that this assert can fail. Not with the current >> code, but with future filters that use the ff_inlink API directly. > > Missingle single thing about future filters, and why would they use > ff_inlink API > directly. > > If you can not cooperate, have very short time to work on FFmpeg, can not stand > criticism of other FFmpeg developers,.. just leave the project for once. Let's work on a solution instead of fighting and shit flinging for once...
On Thu, May 18, 2017 at 10:56 PM, James Almer <jamrial@gmail.com> wrote: > On 5/18/2017 12:49 PM, Paul B Mahol wrote: >> On 5/18/17, Nicolas George <george@nsup.org> wrote: >>> Le nonidi 29 floreal, an CCXXV, Muhammad Faiz a ecrit : >>>> Should fix Ticket6349. >>>> Modifying data pointer may make it unaligned. >>>> >>>> Also change frame->nb_samples < max to frame->nb_samples <= max. >>>> This improves performance. Benchmark: >>>> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null >>>> null >>>> old: >>>> 25767 decicycles in take_samples, 1023 runs, 1 skips >>>> 25422 decicycles in take_samples, 2047 runs, 1 skips >>>> 25181 decicycles in take_samples, 4095 runs, 1 skips >>>> 24904 decicycles in take_samples, 8191 runs, 1 skips >>>> >>>> new: >>>> 550 decicycles in take_samples, 1024 runs, 0 skips >>>> 548 decicycles in take_samples, 2048 runs, 0 skips >>>> 545 decicycles in take_samples, 4096 runs, 0 skips >>>> 544 decicycles in take_samples, 8192 runs, 0 skips >>>> >>>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >>>> --- >>>> libavfilter/avfilter.c | 3 ++- >>>> libavfilter/framequeue.c | 2 ++ >>>> libavfilter/framequeue.h | 5 +++++ >>>> 3 files changed, 9 insertions(+), 1 deletion(-) >>> >>> This is an interesting idea, but... >>> >>>> >>>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c >>>> index 08b86b0..1b6c432 100644 >>>> --- a/libavfilter/avfilter.c >>>> +++ b/libavfilter/avfilter.c >>>> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned >>>> min, unsigned max, >>>> called with enough samples. */ >>>> av_assert1(samples_ready(link, link->min_samples)); >>>> frame0 = frame = ff_framequeue_peek(&link->fifo, 0); >>>> - if (frame->nb_samples >= min && frame->nb_samples < max) { >>>> + if (!link->fifo.samples_skipped && frame->nb_samples >= min && >>>> frame->nb_samples <= max) { >>>> *rframe = ff_framequeue_take(&link->fifo); >>>> return 0; >>>> } >>>> @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, >>>> AVFrame **rframe) >>>> *rframe = NULL; >>>> if (!ff_inlink_check_available_frame(link)) >>>> return 0; >>> >>>> + av_assert1(!link->fifo.samples_skipped); >>> >>> ... I am pretty sure that this assert can fail. Not with the current >>> code, but with future filters that use the ff_inlink API directly. >> >> Missingle single thing about future filters, and why would they use >> ff_inlink API >> directly. >> >> If you can not cooperate, have very short time to work on FFmpeg, can not stand >> criticism of other FFmpeg developers,.. just leave the project for once. > > Let's work on a solution instead of fighting and shit flinging for once... I'm sorry. I've answered to Nicolas (falsely to Nicolas, not to ffmpeg-devel, that's my fault), and he gives positive review. Thank's.
On Thu, May 18, 2017 at 10:32 PM, Muhammad Faiz <mfcc64@gmail.com> wrote: > On Thu, May 18, 2017 at 9:59 PM, Nicolas George <george@nsup.org> wrote: >> Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >>> Should fix Ticket6349. >>> Modifying data pointer may make it unaligned. >>> >>> Also change frame->nb_samples < max to frame->nb_samples <= max. >>> This improves performance. Benchmark: >>> ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null null >>> old: >>> 25767 decicycles in take_samples, 1023 runs, 1 skips >>> 25422 decicycles in take_samples, 2047 runs, 1 skips >>> 25181 decicycles in take_samples, 4095 runs, 1 skips >>> 24904 decicycles in take_samples, 8191 runs, 1 skips >>> >>> new: >>> 550 decicycles in take_samples, 1024 runs, 0 skips >>> 548 decicycles in take_samples, 2048 runs, 0 skips >>> 545 decicycles in take_samples, 4096 runs, 0 skips >>> 544 decicycles in take_samples, 8192 runs, 0 skips >>> >>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >>> --- >>> libavfilter/avfilter.c | 3 ++- >>> libavfilter/framequeue.c | 2 ++ >>> libavfilter/framequeue.h | 5 +++++ >>> 3 files changed, 9 insertions(+), 1 deletion(-) >> >> This is an interesting idea, but... >> >>> >>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c >>> index 08b86b0..1b6c432 100644 >>> --- a/libavfilter/avfilter.c >>> +++ b/libavfilter/avfilter.c >>> @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned min, unsigned max, >>> called with enough samples. */ >>> av_assert1(samples_ready(link, link->min_samples)); >>> frame0 = frame = ff_framequeue_peek(&link->fifo, 0); >>> - if (frame->nb_samples >= min && frame->nb_samples < max) { >>> + if (!link->fifo.samples_skipped && frame->nb_samples >= min && frame->nb_samples <= max) { >>> *rframe = ff_framequeue_take(&link->fifo); >>> return 0; >>> } >>> @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame **rframe) >>> *rframe = NULL; >>> if (!ff_inlink_check_available_frame(link)) >>> return 0; >> >>> + av_assert1(!link->fifo.samples_skipped); >> >> ... I am pretty sure that this assert can fail. Not with the current >> code, but with future filters that use the ff_inlink API directly. > > IMHO, the solution is to document it properly to not mix > ff_inlink_consume_samples with ff_inlink_consume_frame, similar to > av_buffersink_get_frame vs av_buffersink_get_samples. > > Thank's.
On Thu, May 18, 2017 at 10:37 PM, Nicolas George <george@nsup.org> wrote: > Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >> >> Should fix Ticket6349. > > Forgot: please remove that, it is not true. The bug in this ticket is in > libavcodec, it cannot be fixed by a change in libavfilter. In my opinion, it can. The bigger problem about alignment should be separated from this ticket e.g opening new ticket that gives failed alignment test case. > >> IMHO, the solution is to document it properly to not mix >> ff_inlink_consume_samples with ff_inlink_consume_frame, similar to >> av_buffersink_get_frame vs av_buffersink_get_samples. > > Maybe. I am not sure I like this solution, but it should be technically > acceptable, unlike no change at all. I would like better something that > gives the performance boost you observe without limiting the > expressiveness of the API, but I do not know if it is possible. A patch documenting it has been posted. Thank's.
Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit :
> In my opinion, it can.
No, it cannot: the frame that crashes goes from the application to
libavcodec. It came earlier from libavfilter, but that is irrelevant: an
application could have done the same thing without libavfilter, and
resulted in the same crash while completely compatible with the
documented API.
Regards,
Le nonidi 29 floréal, an CCXXV, James Almer a écrit :
> Let's work on a solution instead of fighting and shit flinging for once...
Thank you.
Regards,
On Thu, May 18, 2017 at 11:59 PM, Nicolas George <george@nsup.org> wrote: > Le nonidi 29 floréal, an CCXXV, Muhammad Faiz a écrit : >> In my opinion, it can. > > No, it cannot: the frame that crashes goes from the application to > libavcodec. It came earlier from libavfilter, but that is irrelevant: an > application could have done the same thing without libavfilter, and > resulted in the same crash while completely compatible with the > documented API. Every patch that can fix the crash of ffmpeg -i clip.wav -af alimiter -c:a mp3 -b:a 128k -ar 48k -f null - can claim that it fixes ticket 6349. Other cases should be in separate tickets. Thank's.
On Thu, May 18, 2017 at 08:37:21PM +0700, Muhammad Faiz wrote: > Should fix Ticket6349. > Modifying data pointer may make it unaligned. > > Also change frame->nb_samples < max to frame->nb_samples <= max. > This improves performance. Benchmark: > ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null null > old: > 25767 decicycles in take_samples, 1023 runs, 1 skips > 25422 decicycles in take_samples, 2047 runs, 1 skips > 25181 decicycles in take_samples, 4095 runs, 1 skips > 24904 decicycles in take_samples, 8191 runs, 1 skips > > new: > 550 decicycles in take_samples, 1024 runs, 0 skips > 548 decicycles in take_samples, 2048 runs, 0 skips > 545 decicycles in take_samples, 4096 runs, 0 skips > 544 decicycles in take_samples, 8192 runs, 0 skips > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libavfilter/avfilter.c | 3 ++- > libavfilter/framequeue.c | 2 ++ > libavfilter/framequeue.h | 5 +++++ > 3 files changed, 9 insertions(+), 1 deletion(-) This patch also fixes a crash/regression with avxsynth [...]
On 5/18/17, Muhammad Faiz <mfcc64@gmail.com> wrote: > Should fix Ticket6349. > Modifying data pointer may make it unaligned. > > Also change frame->nb_samples < max to frame->nb_samples <= max. > This improves performance. Benchmark: > ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null > null > old: > 25767 decicycles in take_samples, 1023 runs, 1 skips > 25422 decicycles in take_samples, 2047 runs, 1 skips > 25181 decicycles in take_samples, 4095 runs, 1 skips > 24904 decicycles in take_samples, 8191 runs, 1 skips > > new: > 550 decicycles in take_samples, 1024 runs, 0 skips > 548 decicycles in take_samples, 2048 runs, 0 skips > 545 decicycles in take_samples, 4096 runs, 0 skips > 544 decicycles in take_samples, 8192 runs, 0 skips > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libavfilter/avfilter.c | 3 ++- > libavfilter/framequeue.c | 2 ++ > libavfilter/framequeue.h | 5 +++++ > 3 files changed, 9 insertions(+), 1 deletion(-) > LGTM
Le primidi 1er prairial, an CCXXV, Paul B Mahol a écrit :
> LGTM
I want time to comment.
Regards,
On 5/20/17, Nicolas George <george@nsup.org> wrote: > Le primidi 1er prairial, an CCXXV, Paul B Mahol a ecrit : >> LGTM > > I want time to comment. No, you just want time to prolong this nightmare.
Le primidi 1er prairial, an CCXXV, Paul B Mahol a écrit :
> No, you just want time to prolong this nightmare.
Ad-hominem, non-constructive, fanning the flame.
Muhammad: do not push this patch before I comment.
Regards,
Le decadi 30 floréal, an CCXXV, Muhammad Faiz a écrit : > Every patch that can fix the crash of > ffmpeg -i clip.wav -af alimiter -c:a mp3 -b:a 128k -ar 48k -f null - > can claim that it fixes ticket 6349. > > Other cases should be in separate tickets. No, you are confusing the bug with its symptom. Chewing carefully does not CURE a dental cavity, it only works around it. The correct cure is to go to the dentist. This is the same here: you are working around the bug, but not fixing it. The correct fix is there: https://patchwork.ffmpeg.org/patch/3694/ The project having become such an inhospitable place, I will no longer be working on it, but please feel free to take it over, pushing it as is or reworking it. Anyway, I will be rejecting any patch that pretends to "fix" this ticket with changes in lavfi. > IMHO, the solution is to document it properly to not mix > ff_inlink_consume_samples with ff_inlink_consume_frame, similar to > av_buffersink_get_frame vs av_buffersink_get_samples. As I said, I do not like the idea of limiting the API if it is not absolutely necessary. And I think it is not. What about this: If samples_skipped is set, then ff_inlink_consume_frame() can call ff_inlink_consume_samples() with the exact number of samples to force it to realign the frame at this point. (Of course, it requires disabling the case that does the opposite.) Would you look carefully at the code and tell me I did not miss something that would make it harder than it looks. If not, if you confirm it looks feasible, then I think this patch can go in almost as is: we do not need to actually implement it before it is required, that would be a waste of time, just add a comment near the assert to tell it is the plan. (And of course, remove "fix" from the commit message.) Now, I am not entirely sure that I understand correctly how this patch takes into account the number of samples skipped. I would like to look at it more carefully, but as you can see, Paul is rushing me. Regards,
On Sat, May 20, 2017 at 4:56 PM, Nicolas George <george@nsup.org> wrote: > Le decadi 30 floréal, an CCXXV, Muhammad Faiz a écrit : >> Every patch that can fix the crash of >> ffmpeg -i clip.wav -af alimiter -c:a mp3 -b:a 128k -ar 48k -f null - >> can claim that it fixes ticket 6349. >> >> Other cases should be in separate tickets. > > No, you are confusing the bug with its symptom. Chewing carefully does > not CURE a dental cavity, it only works around it. The correct cure is > to go to the dentist. > > This is the same here: you are working around the bug, but not fixing > it. The correct fix is there: > https://patchwork.ffmpeg.org/patch/3694/ > The project having become such an inhospitable place, I will no longer > be working on it, but please feel free to take it over, pushing it as is > or reworking it. Anyway, I will be rejecting any patch that pretends to > "fix" this ticket with changes in lavfi. OK. > >> IMHO, the solution is to document it properly to not mix >> ff_inlink_consume_samples with ff_inlink_consume_frame, similar to >> av_buffersink_get_frame vs av_buffersink_get_samples. > > As I said, I do not like the idea of limiting the API if it is not > absolutely necessary. And I think it is not. What about this: > > If samples_skipped is set, then ff_inlink_consume_frame() can call > ff_inlink_consume_samples() with the exact number of samples to force it > to realign the frame at this point. (Of course, it requires disabling > the case that does the opposite.) OK > > Would you look carefully at the code and tell me I did not miss > something that would make it harder than it looks. > > If not, if you confirm it looks feasible, then I think this patch can go > in almost as is: we do not need to actually implement it before it is > required, that would be a waste of time, just add a comment near the > assert to tell it is the plan. (And of course, remove "fix" from the > commit message.) > > Now, I am not entirely sure that I understand correctly how this patch > takes into account the number of samples skipped. I would like to look > at it more carefully, but as you can see, Paul is rushing me. > > Regards, > > -- > Nicolas George > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 08b86b0..1b6c432 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -1191,7 +1191,7 @@ static int take_samples(AVFilterLink *link, unsigned min, unsigned max, called with enough samples. */ av_assert1(samples_ready(link, link->min_samples)); frame0 = frame = ff_framequeue_peek(&link->fifo, 0); - if (frame->nb_samples >= min && frame->nb_samples < max) { + if (!link->fifo.samples_skipped && frame->nb_samples >= min && frame->nb_samples <= max) { *rframe = ff_framequeue_take(&link->fifo); return 0; } @@ -1522,6 +1522,7 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame **rframe) *rframe = NULL; if (!ff_inlink_check_available_frame(link)) return 0; + av_assert1(!link->fifo.samples_skipped); frame = ff_framequeue_take(&link->fifo); consume_update(link, frame); *rframe = frame; diff --git a/libavfilter/framequeue.c b/libavfilter/framequeue.c index 26bfa49..fed1118 100644 --- a/libavfilter/framequeue.c +++ b/libavfilter/framequeue.c @@ -107,6 +107,7 @@ AVFrame *ff_framequeue_take(FFFrameQueue *fq) fq->tail &= fq->allocated - 1; fq->total_frames_tail++; fq->total_samples_tail += b->frame->nb_samples; + fq->samples_skipped = 0; check_consistency(fq); return b->frame; } @@ -146,5 +147,6 @@ void ff_framequeue_skip_samples(FFFrameQueue *fq, size_t samples, AVRational tim for (i = 0; i < planes && i < AV_NUM_DATA_POINTERS; i++) b->frame->data[i] = b->frame->extended_data[i]; fq->total_samples_tail += samples; + fq->samples_skipped = 1; ff_framequeue_update_peeked(fq, 0); } diff --git a/libavfilter/framequeue.h b/libavfilter/framequeue.h index 5aa2c72..c49d872 100644 --- a/libavfilter/framequeue.h +++ b/libavfilter/framequeue.h @@ -100,6 +100,11 @@ typedef struct FFFrameQueue { */ uint64_t total_samples_tail; + /** + * Indicate that samples are skipped + */ + int samples_skipped; + } FFFrameQueue; /**
Should fix Ticket6349. Modifying data pointer may make it unaligned. Also change frame->nb_samples < max to frame->nb_samples <= max. This improves performance. Benchmark: ./ffmpeg -filter_complex "aevalsrc=0:n=1166,firequalizer=fixed=on" -f null null old: 25767 decicycles in take_samples, 1023 runs, 1 skips 25422 decicycles in take_samples, 2047 runs, 1 skips 25181 decicycles in take_samples, 4095 runs, 1 skips 24904 decicycles in take_samples, 8191 runs, 1 skips new: 550 decicycles in take_samples, 1024 runs, 0 skips 548 decicycles in take_samples, 2048 runs, 0 skips 545 decicycles in take_samples, 4096 runs, 0 skips 544 decicycles in take_samples, 8192 runs, 0 skips Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> --- libavfilter/avfilter.c | 3 ++- libavfilter/framequeue.c | 2 ++ libavfilter/framequeue.h | 5 +++++ 3 files changed, 9 insertions(+), 1 deletion(-)