| Message ID | 20200524170308.9994-3-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avfilter/af_biquads: implement 1st order allpass | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Will apply soon. On 5/24/20, Paul B Mahol <onemda@gmail.com> wrote: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavfilter/avfilter.c | 61 +++++++++++++++++++++++++++++++++++++----- > libavfilter/filters.h | 17 ++++++++++++ > 2 files changed, 72 insertions(+), 6 deletions(-) >
Paul B Mahol (12020-05-28):
> Will apply soon.
I will hurry looking at it.
Paul B Mahol (12020-05-24): > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavfilter/avfilter.c | 61 +++++++++++++++++++++++++++++++++++++----- > libavfilter/filters.h | 17 ++++++++++++ > 2 files changed, 72 insertions(+), 6 deletions(-) I am against this. It adds quite a lot of complexity in common code. This complexity is not needed: in patch 4/4, you always peek s->window_size samples and skip s->hop_size, constants: just allocate a permanent buffer with size s->window_size and roll by s->hop_size in it. It will be more efficient (less copying around, less allocations) and simpler. Regards,
On 5/28/20, Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12020-05-24): >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavfilter/avfilter.c | 61 +++++++++++++++++++++++++++++++++++++----- >> libavfilter/filters.h | 17 ++++++++++++ >> 2 files changed, 72 insertions(+), 6 deletions(-) > > I am against this. > > It adds quite a lot of complexity in common code. > > This complexity is not needed: in patch 4/4, you always peek > s->window_size samples and skip s->hop_size, constants: just allocate a > permanent buffer with size s->window_size and roll by s->hop_size in it. > It will be more efficient (less copying around, less allocations) and > simpler. > Your arrogance means nothing to me. Will apply in 5 minutes.
Paul B Mahol (12020-05-28):
> Will apply in 5 minutes.
No. This code is unnecessary and complicates things.
On 5/28/20, Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12020-05-28): >> Will apply in 5 minutes. > > No. This code is unnecessary and complicates things. You can say whatever you want. I simply do not care. You could block it months ago instead of requesting me to lose all my precious time on your requested changes.
Paul B Mahol (12020-05-28): > You can say whatever you want. I simply do not care. > > You could block it months ago instead of requesting me to lose all my > precious time on your requested changes. This is not about you, it is about the code. I did not notice the problem then, I notice it now. Better than after it is pushed And since you are being rude, I am allowed to say what I have on my mind: I would not have to reject your code if it was good in the first place. Spend fifteen minutes more thinking about your code before writing it, and we would not have to waste both hours arguing over it.
On 5/28/20, Nicolas George <george@nsup.org> wrote: > Paul B Mahol (12020-05-28): >> You can say whatever you want. I simply do not care. >> >> You could block it months ago instead of requesting me to lose all my >> precious time on your requested changes. > > This is not about you, it is about the code. I did not notice the > problem then, I notice it now. Better than after it is pushed > > And since you are being rude, I am allowed to say what I have on my > mind: > > I would not have to reject your code if it was good in the first place. > Spend fifteen minutes more thinking about your code before writing it, > and we would not have to waste both hours arguing over it. > You are one that are extremely unpleasant and rude to me and to anyone else. Your technical explanation is invalid, and your proposed solution extremely convoluted and needs more code for each filter needing this functionality. Watch me applying this set.
diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 394811916d..85010c88fe 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -1131,6 +1131,7 @@ static int samples_ready(AVFilterLink *link, unsigned min) } static int take_samples(AVFilterLink *link, unsigned min, unsigned max, + unsigned peek_samples, AVFrame **rframe) { AVFrame *frame0, *frame, *buf; @@ -1142,7 +1143,10 @@ static int take_samples(AVFilterLink *link, unsigned min, unsigned max, av_assert1(samples_ready(link, link->min_samples)); frame0 = frame = ff_framequeue_peek(&link->fifo, 0); if (!link->fifo.samples_skipped && frame->nb_samples >= min && frame->nb_samples <= max) { - *rframe = ff_framequeue_take(&link->fifo); + if (peek_samples) + *rframe = av_frame_clone(frame); + else + *rframe = ff_framequeue_take(&link->fifo); return 0; } nb_frames = 0; @@ -1172,18 +1176,20 @@ static int take_samples(AVFilterLink *link, unsigned min, unsigned max, p = 0; for (i = 0; i < nb_frames; i++) { - frame = ff_framequeue_take(&link->fifo); + frame = peek_samples ? ff_framequeue_peek(&link->fifo, i) : ff_framequeue_take(&link->fifo); av_samples_copy(buf->extended_data, frame->extended_data, p, 0, frame->nb_samples, link->channels, link->format); p += frame->nb_samples; - av_frame_free(&frame); + if (!peek_samples) + av_frame_free(&frame); } if (p < nb_samples) { unsigned n = nb_samples - p; - frame = ff_framequeue_peek(&link->fifo, 0); + frame = ff_framequeue_peek(&link->fifo, peek_samples ? i : 0); av_samples_copy(buf->extended_data, frame->extended_data, p, 0, n, link->channels, link->format); - ff_framequeue_skip_samples(&link->fifo, n, link->time_base); + if (!peek_samples) + ff_framequeue_skip_samples(&link->fifo, n, link->time_base); } *rframe = buf; @@ -1504,7 +1510,7 @@ int ff_inlink_consume_samples(AVFilterLink *link, unsigned min, unsigned max, return 0; if (link->status_in) min = FFMIN(min, ff_framequeue_queued_samples(&link->fifo)); - ret = take_samples(link, min, max, &frame); + ret = take_samples(link, min, max, 0, &frame); if (ret < 0) return ret; consume_update(link, frame); @@ -1512,6 +1518,49 @@ int ff_inlink_consume_samples(AVFilterLink *link, unsigned min, unsigned max, return 1; } +int ff_inlink_peek_samples(AVFilterLink *link, unsigned nb_samples, + AVFrame **rframe) +{ + AVFrame *frame; + int ret; + + av_assert1(nb_samples); + *rframe = NULL; + if (!ff_inlink_check_available_samples(link, nb_samples)) + return 0; + if (link->status_in) + nb_samples = FFMIN(nb_samples, ff_framequeue_queued_samples(&link->fifo)); + ret = take_samples(link, nb_samples, nb_samples, 1, &frame); + if (ret < 0) + return ret; + *rframe = frame; + return !!frame; +} + +void ff_inlink_skip_samples(AVFilterLink *link, unsigned skip_samples) +{ + skip_samples = FFMIN(skip_samples, ff_framequeue_queued_samples(&link->fifo)); + + while (skip_samples > 0) { + AVFrame *frame = ff_inlink_peek_frame(link, 0); + if (skip_samples >= frame->nb_samples) { + frame = ff_framequeue_take(&link->fifo); + skip_samples -= frame->nb_samples; + av_frame_free(&frame); + } else { + break; + } + } + + if (skip_samples) + ff_framequeue_skip_samples(&link->fifo, skip_samples, link->time_base); + + if (ff_inlink_queued_frames(link)) { + AVFrame *frame = ff_inlink_peek_frame(link, 0); + consume_update(link, frame); + } +} + AVFrame *ff_inlink_peek_frame(AVFilterLink *link, size_t idx) { return ff_framequeue_peek(&link->fifo, idx); diff --git a/libavfilter/filters.h b/libavfilter/filters.h index 1157755403..7dc0b35981 100644 --- a/libavfilter/filters.h +++ b/libavfilter/filters.h @@ -115,6 +115,23 @@ int ff_inlink_consume_frame(AVFilterLink *link, AVFrame **rframe); int ff_inlink_consume_samples(AVFilterLink *link, unsigned min, unsigned max, AVFrame **rframe); +/** + * Peek samples from the link's FIFO. + * + * @return >0 if a samples are available, + * 0 and set rframe to NULL if no samples are available, + * or AVERROR code + */ +int ff_inlink_peek_samples(AVFilterLink *link, unsigned nb_samples, + AVFrame **rframe); + +/** + * Skip samples from the link's FIFO. + * + * @note May trigger process_command() and/or update is_disabled. + */ +void ff_inlink_skip_samples(AVFilterLink *link, unsigned skip); + /** * Access a frame in the link fifo without consuming it. * The first frame is numbered 0; the designated frame must exist.
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavfilter/avfilter.c | 61 +++++++++++++++++++++++++++++++++++++----- libavfilter/filters.h | 17 ++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-)