[MPlayer-dev-eng] [PATCH] af_scaletempo

[Robert Juliano]

On Sat, Oct 06, 2007 at 10:26:01PM +0300, Uoti Urpala wrote:
> On Fri, 2007-10-05 at 22:11 +0200, Reimar D??ffinger wrote:
> > Hello,
> > On Thu, Oct 04, 2007 at 08:19:17PM -0400, Robert Juliano wrote:
> > > So, what do I have to do to get someone to commit this?
> > 
> > I'll try to find time to test it this weekend and the apply it (minus
> > cosmetics to mplayer.c).
> > If anyone has objections against this please speak up (note I do not
> > claim it to be perfect, but if there are issues remaining I think they
> > will be easier to fix after applying).
> 
> I think it's better to fix some of the general filter issues before
> adding the filter. It buffers more data internally than than the
> existing resample filters.

Yeah, and it uses more memory than format and more CPU than
channels?  I don't see the relevance of the comparison.

> The following are some issues I think should
> be fixed in or before the first version that adds the filter:
> 
> - make the final audio buffer before ao dynamically sized like all the
> filter buffers

I'm not sure what you mean by this.

> - remove the rational number input/output ratio fields from filters and
> replace them with floats (the rationals can overflow and the values do
> not need to be exact)

I tried this but it broke a number of things.  Maybe you have
a better understanding of dec_audio.  Though floats are
probably the better long term implementation, switching to
64-bit ints is the quickest fix (2-3 lines), needs minimal
testing, and least likely to break anything else.  Or I've
already posted a patch that needs even less testing and is
even less likely to break anything.

> - enable the filter delay logic again and dynamically set the delay
> value in af_scaletempo after each call

Unless someone shows me otherwise, delay = 0.  The way the
filter works is by consuming input at a scaled rate and output
at the nominal rate or it can be viewed as skipping forwards
or backwards every few milliseconds.  I'm not sure delay even
makes sense in this context.  And if it's disabled, what does
it have to do with adding a filter today?

> - when used for runtime playback speed adjustment, make sure
> af_scaletempo never alters the audio when input and output rate are
> identical, either by rebuilding the filter chain without it or by logic
> inside the filter

Does what's in the latest patch (9/30) not do this?

> 
> I haven't done those yet mainly because of the release plans.

Sounds to me like you have a number of backlogged tasks that
would require changes to many filters and the audio chain,
and from your follow-up email, it sound to me like
af_scaletempo is fine but the filter framework has issues.

So, I'm left with similar questions as Reimar.  What about
af_scaletempo would prevent it from being committed today?
What, if any, of the issues with the filter framework/audio
chain should prevent it from being committed?  And, how much
more difficult would it be to make the changes after it was
committed, and is that sufficient to warrant delaying it?

I'm here to help if you want it.
robert