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

[Paul B Mahol]

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.