Home of the Squeezebox™ & Transporter® network music players.
Results 1 to 6 of 6
  1. #1
    Senior Member
    Join Date
    Aug 2014
    Location
    UK
    Posts
    555

    Slim::Music::Info::setDelayedCallback incorrect delay calc for 48000 sample rate

    I have been trying to get precise track info display on the BBC sounds plugin which uses Slim::Music::Info::setDelayedCallback to provide the track info at the right time in sync with the audio.
    I've noticed that the 'getStreamDelay' operation that it uses to calculate the delay require, has the sample rate of 44100 hard coded into the calculation. BBC Sounds audio uses a 48000 sample rate which makes the delay calculation (for the output buffer bit) around 8% out.

    Code:
    sub getStreamDelay {
    	my ( $client, $outputDelayOnly ) = @_;
    	
    	return 0 unless $client->streamingSong();
    	
    	my $bitrate = $client->streamingSong()->streambitrate() || 128000;
    	my $delay   = 0;
    	
    	if ( $bitrate > 0 ) {
    		my $decodeBuffer = $client->bufferFullness() / ( int($bitrate / 8) );
    		my $outputBuffer = $client->outputBufferFullness() / (44100 * 8);
    	
    		if ( $outputDelayOnly ) {
    			$delay = $outputBuffer;
    		}
    		else {
    			$delay = $decodeBuffer + $outputBuffer;
    		}
    	}
    	
    	return $delay;
    }
    It isn't that noticeable, to be honest, and if I really wanted to fix it I could easily bring the calculation and timer into the plugin.

    But I wondered if this is like this for historical reasons and is best left alone, and it is a more complicated subject than I realise? (e.g. resampling can happen etc). If it isn't, I'm happy to do a PR to fix it.

  2. #2
    Babelfish's Best Boy mherger's Avatar
    Join Date
    Apr 2005
    Location
    Switzerland
    Posts
    20,627

    Slim::Music::Info::setDelayedCallback incorrectdelay calc for 48000 sample rate

    > But I wondered if this is like this for historical reasons and is best
    > left alone, and it is a more complicated subject than I realise? (e.g.
    > resampling can happen etc). If it isn't, I'm happy to do a PR to fix
    > it.


    It's there because... it got implemented this way more than a decade
    ago, and nobody ever noticed it or bothered to fix it. Doesn't need to
    be this way. If you feel like fixing it, go ahead. 8.3 is here for this
    kind of change.

  3. #3
    Senior Member
    Join Date
    May 2010
    Location
    London, UK
    Posts
    923
    Quote Originally Posted by mherger View Post
    It's there because... it got implemented this way more than a decade
    ago, and nobody ever noticed it or bothered to fix it. Doesn't need to
    be this way. If you feel like fixing it, go ahead. 8.3 is here for this
    kind of change.
    Is this PR related to the issue ?

    https://github.com/Logitech/slimserver/pull/644

  4. #4
    Senior Member
    Join Date
    Aug 2014
    Location
    UK
    Posts
    555
    Quote Originally Posted by mrw View Post
    Is this PR related to the issue ?

    https://github.com/Logitech/slimserver/pull/644
    Similarly caused by hardcoding the sample rate. Although the setDelayedCallback is a much less impactful problem, I suspect. As it is probably only used for meta data info changes (as per my use)

  5. #5
    Senior Member
    Join Date
    May 2008
    Location
    Canada
    Posts
    7,631
    Code:
    if ( $bitrate > 0 ) {
    		my $decodeBuffer = $client->bufferFullness() / ( int($bitrate / 8) );
    		my $outputBuffer = $client->outputBufferFullness() / (44100 * 8);
    bufferFullness and outputBufferFullness are the current level in bytes of the stream buffer and the decoded (raw) samples buffer

    So I agree that bufferfulness/(bitrate/8) gives a delay value, but outputBufferFullness / (44100 * 8) will not always make sense as it assumes outputbuffer has 8 bytes per frame, which is not always true. But still, using the samplerate when known is better, still

    BTW, there are a few other places where 44100 is hard coded.
    LMS 8.2 on Odroid-C4 - SqueezeAMP!, 5xRadio, 5xBoom, 2xDuet, 1xTouch, 1xSB3. Sonos PLAY:3, PLAY:5, Marantz NR1603, Foobar2000, ShairPortW, 2xChromecast Audio, Chromecast v1 and v2, Squeezelite on Pi, Yamaha WX-010, AppleTV 4, Airport Express, GGMM E5, RivaArena 1 & 3

  6. #6
    Senior Member
    Join Date
    Aug 2014
    Location
    UK
    Posts
    555
    Quote Originally Posted by philippe_44 View Post
    Code:
    if ( $bitrate > 0 ) {
    		my $decodeBuffer = $client->bufferFullness() / ( int($bitrate / 8) );
    		my $outputBuffer = $client->outputBufferFullness() / (44100 * 8);
    bufferFullness and outputBufferFullness are the current level in bytes of the stream buffer and the decoded (raw) samples buffer

    So I agree that bufferfulness/(bitrate/8) gives a delay value, but outputBufferFullness / (44100 * 8) will not always make sense as it assumes outputbuffer has 8 bytes per frame, which is not always true. But still, using the samplerate when known is better, still
    Ah, I guessed it was more complicated than it appeared . Oh well, I've submitted the pull request which removes the hard coding of sample rate (if known), which, as you say, is an improvement at least.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •