[FFmpeg-devel] [PATCH] swresample: Add swr_get_out_samples()

[Michael Niedermayer]

On Wed, Jun 03, 2015 at 11:11:23AM +0200, wm4 wrote:
> On Wed,  3 Jun 2015 01:22:49 +0200
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libswresample/resample.c            |   17 +++++++++++++++++
> >  libswresample/swresample.c          |   30
> > ++++++++++++++++++++++++++++++ libswresample/swresample.h
> > |   14 ++++++++++++++ libswresample/swresample_internal.h |    2 ++
> >  libswresample/version.h             |    2 +-
> >  5 files changed, 64 insertions(+), 1 deletion(-)
> > 
> 
> Well, it's interesting to see that the implementation is much more
> complicated than what's suggested in the public API doxygen, or what
> af_aresample.c does.

i dont know, it doesnt look much more complex to me, its handling
compensation_distance, and does a bit of error checking


> 
> > diff --git a/libswresample/resample.c b/libswresample/resample.c
> > index d4c7d06..c61bcb0 100644
> > --- a/libswresample/resample.c
> > +++ b/libswresample/resample.c
> > @@ -345,6 +345,22 @@ static int64_t get_delay(struct SwrContext *s,
> > int64_t base){ return av_rescale(num, base,
> > s->in_sample_rate*(int64_t)c->src_incr << c->phase_shift); }
> >  
> > +static int64_t get_out_samples(struct SwrContext *s, int in_samples)
> > {
> > +    ResampleContext *c = s->resample;
> > +    int64_t num = s->in_buffer_count + 2LL + in_samples;
> > +    num *= 1 << c->phase_shift;
> > +    num -= c->index;
> > +    num = av_rescale_rnd(num, s->out_sample_rate,
> > ((int64_t)s->in_sample_rate) << c->phase_shift, AV_ROUND_UP); +
> > +    if (c->compensation_distance) {
> > +        if (num > INT_MAX)
> > +            return AVERROR(EINVAL);
> > +
> > +        num = FFMAX(num, (num * c->ideal_dst_incr - 1) / c->dst_incr
> > + 1);
> > +    }
> > +    return num + 2;
> 
> What's the are the two "+2" for (here and at initialization of num)?
> Maybe there should be a comment explaining them.

comment added


> 
> > +}
> > +
> >  static int resample_flush(struct SwrContext *s) {
> >      AudioData *a= &s->in_buffer;
> >      int i, j, ret;
> > @@ -414,4 +430,5 @@ struct Resampler const swri_resampler={
> >    set_compensation,
> >    get_delay,
> >    invert_initial_buffer,
> > +  get_out_samples,
> >  };
> > diff --git a/libswresample/swresample.c b/libswresample/swresample.c
> > index 3d3ab83..a5f5930 100644
> > --- a/libswresample/swresample.c
> > +++ b/libswresample/swresample.c
> > @@ -672,11 +672,15 @@ int attribute_align_arg swr_convert(struct
> > SwrContext *s, uint8_t *out_arg[SWR_C const uint8_t *in_arg
> > [SWR_CH_MAX], int  in_count){ AudioData * in= &s->in;
> >      AudioData *out= &s->out;
> > +    int av_unused max_output;
> >  
> >      if (!swr_is_initialized(s)) {
> >          av_log(s, AV_LOG_ERROR, "Context has not been
> > initialized\n"); return AVERROR(EINVAL);
> >      }
> > +#if ASSERT_LEVEL >1
> > +    max_output = swr_get_out_samples(s, in_count);
> > +#endif
> >  
> >      while(s->drop_output > 0){
> >          int ret;
> > @@ -719,6 +723,9 @@ int attribute_align_arg swr_convert(struct
> > SwrContext *s, uint8_t *out_arg[SWR_C int ret =
> > swr_convert_internal(s, out, out_count, in, in_count); if(ret>0
> > && !s->drop_output) s->outpts += ret * (int64_t)s->in_sample_rate;
> > +
> > +        av_assert2(max_output < 0 || ret < 0 || ret <= max_output);
> > +
> >          return ret;
> >      }else{
> >          AudioData tmp= *in;
> > @@ -770,6 +777,7 @@ int attribute_align_arg swr_convert(struct
> > SwrContext *s, uint8_t *out_arg[SWR_C }
> >          if(ret2>0 && !s->drop_output)
> >              s->outpts += ret2 * (int64_t)s->in_sample_rate;
> > +        av_assert2(max_output < 0 || ret2 < 0 || ret2 <= max_output);
> >          return ret2;
> >      }
> >  }
> > @@ -821,6 +829,28 @@ int64_t swr_get_delay(struct SwrContext *s,
> > int64_t base){ }
> >  }
> >  
> > +int swr_get_out_samples(struct SwrContext *s, int in_samples)
> > +{
> > +    int64_t out_samples;
> > +
> > +    if (in_samples < 0)
> > +        return AVERROR(EINVAL);
> > +
> > +    if (s->resampler && s->resample) {
> > +        if (!s->resampler->get_out_samples)
> > +            return AVERROR(ENOSYS);
> 
> When does this happen? Unless it's only before the context is
> "opened", I'd say this is unacceptable. It should at least attempt to
> return an upper bound. How else would the API user be able to tell how
> much data to read?
> 
> (Unless you require the API user to do repeated resample calls with no
> input to drain the remaining internal buffers?)

ENOSYS happens if you select the soxr resampler, ive not yet looked
into if this is possible to know this based on API without depending
on soxr internals.

either its possible to do then it should get implemented
or its not in which case its just not possible if the user selects
soxr


> 
> > +        out_samples = s->resampler->get_out_samples(s, in_samples);
> > +    } else {
> > +        out_samples = swr_get_delay(s, s->out_sample_rate) + 2 +
> > in_samples;
> 
> Magic +2 again...

removed


> 
> > +        av_assert0(s->out_sample_rate == s->in_sample_rate);
> > +    }
> > +
> > +    if (out_samples > INT_MAX)
> > +        return AVERROR(EINVAL);
> > +
> > +    return out_samples;
> > +}
> > +
> >  int swr_set_compensation(struct SwrContext *s, int sample_delta, int
> > compensation_distance){ int ret;
> >  
> > diff --git a/libswresample/swresample.h b/libswresample/swresample.h
> > index 37656a6..3b7b916 100644
> > --- a/libswresample/swresample.h
> > +++ b/libswresample/swresample.h
> > @@ -436,6 +436,20 @@ int swr_inject_silence(struct SwrContext *s, int
> > count); int64_t swr_get_delay(struct SwrContext *s, int64_t base);
> >  
> >  /**
> > + * Finds an upper bound on the number of samples that the next
> > swr_convert will output.
> > + *
> > + * @param in_samples    number of input samples.
> > + * @note any call to swr_inject_silence(), swr_convert(),
> > swr_next_pts()
> > + *       or swr_set_compensation() invalidates this limit
> 
> This sounds a bit unclear. How about:
> 
>   Find an upper bound on the number of samples that the next swr_convert
>   call will output, if called with in_samples of input samples. This
>   depends on the internal state, and anything changing the internal state
>   (like further swr_convert() calls) will may change the number of samples
>   swr_get_out_samples() returns for the same number of input samples.
> 
> I'd also add a similar notice to swr_convert():
> 
>    If more input is provided than output space, then the input will be buffered. 
>    You can avoid this buffering by using swr_get_out_samples() to retrieve an
>    upper bound on the required number of output samples for the given number of
>    input samples. Conversion will run directly without copying whenever possible.

replaced


applied

thanks

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150604/f14dc706/attachment.asc>