[Alsaplayer-devel] bug in save/restore of the pause functionality

[Dominique Michel]

Le Wed, 07 Mar 2007 13:27:56 +0100,
"Christoph Delfs" <christoph.delfs at gmx.de> a écrit :

> Hi all,

Hi Christof,

> I am wondering whether the problem Eric Winn has described earlier (in 2005)
> has been analyzed further in the past. I would like to cite from the mailing
> list below. Any comments from the audience? Cheers Christoph
> 
It is hard to say because no one in the actual team was working on the
alsaplayer before a few months ago.

But it seam at at least part of this problem have been fixed. I just try to
reproduce this bug but was not able to do so. But I found something else that
can be related.

I was running alsaplayer with jack output at 48kHz and playing a mp3 song at
44.1kHz. I get no problem with the pause-unpause, both in the GUI or with the
command line.

alsaplayer --status reported a speed of 91% all the time when playing.

But, when increasing the speed by one comma, status reported a speed of 93%
(0.91875*1.013643) before the pause-unpause, and 91% after the pause-unpause.
93% is correct, but 91% after the pause-unpause is not, especially with the GUI
where it is another button to set the speed at normal playback speed. 

In fact, it is the same with the daemon, if a song is paused, alsaplayer
--start will do the same as alsaplayer --pause.

So, someone will have to lock into this. I suggest to keep the --start
option and the 2 buttons (normal playback and reverse speed) in gtk GUI as
they are, but to modify the pause behaviour so at it take in account not only
the normal speed, but also the change of speed that an user can made before
pausing. This value must be stored somewhere as alsaplayer --status is able to
report it.

Any volunteer?

Ciao,
Dominique

> ######################################
> Hi Erik,
> 
> I will have a look at the problem. Sounds like a bug in the save/restore
> of the pause functionality...
> 
> Thanks,
> Andy
> 
> On Wed, Nov 05, 2003 at 11:18:27PM -0800, Erik Winn wrote:
> > Hi,
> > 
> >  Tried a sort of brief question about this a few days ago - but it seems
> > not to have even made it to the list; I would very much appreciate some
> > sort of acknowledgement of this post if it is going to the private list (to
> > which I have subscribed but also without response ...) so that I know if I
> > am talking to the wall or not.
> > 
> >  For everything else, the latest version running with jackd 0.8 is great
> > and quite impressive (I especially like the reverse playback feature - neat 
> > trick.), but I have found a nice glitch in pause/unpause funtionality that 
> > causes incremental decrease in playback sample rates. 
> > 
> >  Here is the basic problem - I have isolated it to be related to the mad 
> > plugin's interaction with CorePlayer::GetSpeed/SetSpeed after Pause when
> > the file being played is of a lower sample rate. For instance, I play an
> > mp3 with 56 kbps, 22 kHz (joint stereo), no CRC, no padding. At 44100,
> > ap_get_speed() returns 50 - which is correct scaling, but it seems that
> > this is stored somewhere and used to SetSpeed after pause. Here is what I
> > have found so far:
> > 
> >  First, it saves a pitch point in CorePlayer::Start like this
> > CorePlayer.cpp, line 585:
> > 	output_rate = node->SamplingRate();
> > 	input_rate = plugin->sample_rate(the_object);
> > 
> > 	if (input_rate == output_rate)
> > 		SetSpeedMulti(1.0);
> > 	else
> > 		SetSpeedMulti((float) input_rate / (float) output_rate);
> > 	update_pitch();
> > 
> >  ... well, mad_sample_rate will do this:
> > 
> > mad_engine.c, line 745:
> > 	if (data)
> > 		return data->samplerate;
> >  
> >  ... which, at this point should be 22050. Meaning, we call, effectively,
> > SetSpeedMulti(0.5), after which pitch_multi gets 0.5 - and then
> > update_pitch essentially divides pitch in half:
> > 
> > CorePlayer.cpp, line 953:
> > 		pitch = pitch_point;
> > 	}
> > 	pitch *= pitch_multi;
> > 
> >  ... Now, the next call to GetSpeed will return 0.5, so if we now pause and 
> > unpause ... well, here it is:
> > 
> > void CorePlayer::Pause()
> > {
> > 	save_speed = GetSpeed ();
> > 	SetSpeed (0.0);
> > }
> > 
It is actually changed to

void CorePlayer::Pause()
{
        //save_speed = GetSpeed ();
        SetSpeed (0.0);
}

> > 
> > void CorePlayer::UnPause()
> > {
> > 	if (save_speed)
> > 	    SetSpeed (save_speed);
> > 	else
> > 	    SetSpeed (1.0);
> > }
and

void CorePlayer::UnPause()
{
        //if (save_speed)
        //    SetSpeed (save_speed);
        //else
        SetSpeed (1.0);
}

> > 
> >  The result is that everytime one pauses/unpauses, the speed is cut in
> > half. The same applies for 44100 samplerate mp3s if we are running jackd at
> > 48000 and neglect to use -F 48000, frequency will be correctly scaled and 
> > ap_get_speed will return 92, so every un/pause will reduce playback by 
> > (previous_rate * 0.91875), and so on.
> > 
> >  I am quite unsure where to introduce a fix for this - I have only just 
> > started looking at the code and spent but a few hours with it. I would be 
> > happy to implement it and submit a patch if you would be good enough to 
> > suggest where/how this might be done to fit and not break frequency scaling.
> > 
> > Best, and thanks for a nice player,
> > Erik Winn
> #######################################
> 


-- 
Dominique Michel

--
N.B.: Tous les emails que je reçois sont filtrés par spamassassin avant de me
parvenir.