[paplayer] - when player is paused - don't process streams but just yield... #1910

Merged
merged 1 commit into from Dec 10, 2012

4 participants

@Memphiz
Team Kodi member

... cpu to other threads.

The following problem exists on osx and ios (was not able to reproduce this on linux).

On all audio codecs in paplayer which get decoded via ffmpeg (dvdplayercodec) when pausing a track it might happen that it just unpauses itself and skips to the next track.

The following stuff is going on when this happens:

  1. During pause the paplayer still loops in his threadloop and processes its streams
  2. In ProcessStream it calls si->m_decoder.ReadSamples(PACKET_SIZE) and queues more data
  3. In some circumstances (not always - but mostly when i start XBMC and start the music tracks for the first time) this leads to some sort of read error in https://github.com/xbmc/xbmc/blob/master/xbmc/cores/dvdplayer/DVDDemuxers/DVDDemuxFFmpeg.cpp#L660 (result is a big negative number which i couldn't assign to any specific error code).
  4. This leads to the flush and to the fact returning NULL
  5. Returning NULL is treated as EOF in the upper layers
  6. EOF is now processed by the paplayer which is still paused. He queues the next track and resumes it. (gui is still in paused state because paplayer does this on his own with out telling anyone).

I know that pausing paplayer in the way i propose in this PR will stop filling the buffers during pause. But hey this is audio - every crappy wlan should get this through the air with out the need of a big cache.

I also know that the core issue might lay much deeper (maybe even in ffmpeg or a compile issue on osx/ios). But we need a fix for frodo - and i'm already done again with that paplayer stuff (i can't stand it more then 3 hours in a row ;) ).

ping to @elupus @gnif @davilla

@davilla

Sleep() instead of CThread::Sleep(), this code is in PAPlayer::Process and the inherited Sleep has a check for bStop so it will abort if the the PAPlayer thread is stopped. 10ms is not much but hey, it all adds up :)

Team Kodi member

The same sleep is used 5 lines down for sleeping the watermark stuff. What is the issue when the sleep gets aborted when paplayer thread is stopped? This even would be nice imo?

And i could increase it to 100ms if we want :)

I guess who every added the other did not know about how the inherited Sleep works, or it was some cp monster running around :)

Team Kodi member

But what is the problem with sleep aborting when m_bStop is set? The thread is going to stop then anyway - so?

there is no problem, I'm confused :) Sleep() and CThread::Sleep() are the same thing inside a CThread

@DDDamian

IIRC the CThread::Sleep was recommended by davilla in an IRC conversation a long time ago specifically for PAPlayer, but can't recall why.

Is the big negative number returned consistent?

If this fix helps iOS no concerns merging (although we obviously don't fill the queues as fast) but I haven't been able to reproduce here on Win.

@davilla

/me swims over to the treasure chest and blows a bubble.

@Memphiz
Team Kodi member

I can reproduce this as follows.
1. having a test folder on smb or nfs share which contains 3 flac or 3 m4a testfiles (or anything else which is decoded through dvdplayercodec)
2. Start xbmc
3. go into music->files->testfolder
4. start playback of the first testfile
5. enter fullscreen
6. pause the stream (enter for bringing up the osd, enter again for pausing the stream)
7. wait between 5 and 20 secs until the stream autostarts the next track

When i have done this with my testfiles for a while the problem occurs more seldom. But its always there after a fresh restart of XBMC.

@Memphiz
Team Kodi member

Can anyone think of negative influences this could have (beside it the fact that it doesn't fill the cache during pause) which would block this from going in? (or should we learn it the hard way maybe? ^^)

@davilla davilla merged commit 45fad54 into xbmc:master Dec 10, 2012
@jmarshallnz
Team Kodi member

Why does this happen only during pause? What is different in that case?

@Memphiz
Team Kodi member

Maybe some buffer gets overrun in ffmpeg or so? (cause during pause it just keeps decoding)

@jmarshallnz
Team Kodi member

If it's paused, wouldn't it just decode until the player buffer is full, and then just sleep at that point until things are resumed?

@Memphiz
Team Kodi member

Well i guess it would do. This is a bug somewhere else i suppose. As of my reproduction it was harder to reproduce the longer i tried without restarting XBMC. This is just the quick fix for it for frodo (well as long as nobody else wants to dig into this at this speedy moment).

@jmarshallnz
Team Kodi member

I really don't like fixes going in that no one understands, particularly when they alter fundamental bits of code such as the main loop of the audio player. I certainly wouldn't be confident that this won't break other stuff.

I suspect you've uncovered a real problem, but I can't see why it would happen only on pause. If we don't understand it, we can't know that it has actually fixed anything.

@Memphiz
Team Kodi member

can you jump on irc?

@Memphiz Memphiz deleted the Memphiz:paplayerfix branch Jan 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment