[MPlayer-dev-eng] [PATCH 2/2] playback_speed limits: MP_CMD_SPEED_*

[Robert Juliano]

Reimar Dffinger <Reimar.Doeffinger <at> stud.uni-karlsruhe.de> writes:
> Hello,
> On Sat, Jun 02, 2007 at 03:10:37PM -0400, Robert Juliano wrote:
> > Patch 2: prevent non-positive playback_speed
> > 
> > While -speed and mp_property_playback_speed clamp playback_speed,
> > MP_CMD_SPEED_* didn't.  Ex. with input.conf "[ speed_incr -0.1",
> > playback_speed could still become non-positive.
> 
> Actually I think that is not the case and this is a bug that your first
> patch introduces.

Yeah, the error was present before when there was no audio
at all, but that patch expands it to all cases.

> 
> >                 playback_speed += v;
> > +               if (playback_speed <= 0) playback_speed = .01;
> 
> This check makes no sense, if the current value was e.g. 0.005 and you
> give the command to decrease it the code will actually increase it to
> 0.01 instead of leaving it unchanged.
>  playback_speed = FFMAX(playback_speed, 0.01);
> would work better.

Right again.  I've actually replaced this in a forthcoming patch:
    float prev = playback_speed;
    playback_speed += v;
    if (mpctx->sh_audio
        && !af_playback_speed_change(mpctx->sh_audio->afilter, playback_speed,
mpctx->sh_audio->samplerate)) {
      af_msg(AF_MSG_WARN, "speed changed failed!\n");
      playback_speed = prev;
    }

af_playback_speed_change either finds a filter to handle the
playback speed (e.g. one that preserves the pitch (forthcoming
as well)) or reverts to hacking the input sample rate.

I've written these patches but am still tweeking them.

> And while you're at it you might want to also add a check that the
> sample rate integer will not overflow for too big values.

What would the playback_speed have to be for that to happen?
(2^31 / 192kHz) > 10000!  I'm not too worried.

> 
> Greetings,
> Reimar Dffinger
>