[FFmpeg-devel] [PATCH] avfilter: take_samples: do not directly return frame when samples are skipped

Paul B Mahol onemda at gmail.com
Thu May 18 18:49:12 EEST 2017


On 5/18/17, Nicolas George <george at 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 at 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.


More information about the ffmpeg-devel mailing list