On Thu, Jun 04, 2015 at 10:48:01AM +0200, wm4 wrote:
> On Thu, 4 Jun 2015 04:37:31 +0200
> Michael Niedermayer <michae...@gmx.at> wrote:
>
> > On Wed, Jun 03, 2015 at 11:11:23AM +0200, wm4 wrote:
> > > On Wed, 3 Jun 2015 01:22:49 +0200
> > > Michael Niedermayer <michae...@gmx.at> wrote:
> > >
> > > > Signed-off-by: Michael Niedermayer <michae...@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
>
> The comment you added to the final version had 2 typos.
i fixed 3 typos
>
> >
> > >
> > > > +}
> > > > +
> > > > 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
> >
>
> This is a bit of a problem. Since soxr can be enabled via the generic
> AVOptions, a user could easily break the API. Which is not good. (I
> already had this issue with libswresample/soxr in a different case. I
> didn't even know about it, until a user tried to use it.)
I do want to get this implemented for soxr
too but i need to check with someone from soxr to make sure its
implemented correctly.
dont hesitate to ping this if you see no implementation in the next
2 weeks or so
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
