Rockbox

Tasklist

FS#11387 - PictureFlow runs jerkily while playing music on Nano2G after the recent feature addition

Attached to Project: Rockbox
Opened by Michael Marley (mamarley) - Wednesday, 09 June 2010, 10:35 GMT
Last edited by Thomas Martitz (kugel.) - Monday, 13 June 2011, 16:50 GMT
Task Type Bugs
Category Plugins
Status Closed
Assigned To No-one
Operating System iPod Nano 2G
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

After the series of commits that added WPS integration for PictureFlow, I have noticed that PictureFlow runs very jerkily if the player is playing music at the same time. Before those commits, it was almost perfectly smooth, even when playing music. I tried forcing boost before the test, but that didn't affect the performance.
This task depends upon

Closed by  Thomas Martitz (kugel.)
Monday, 13 June 2011, 16:50 GMT
Reason for closing:  Fixed
Additional comments about closing:  r30000
Comment by Jonathan Gordon (jdgordon) - Wednesday, 09 June 2010, 10:40 GMT
can you track down which commit did it?
Comment by Michael Marley (mamarley) - Wednesday, 09 June 2010, 11:49 GMT
Not at the moment, but I should be able to later today. I will get back to you when I have.
Comment by Michael Marley (mamarley) - Wednesday, 09 June 2010, 13:47 GMT
OK, I have narrowed it down to somewhere between 26713 and 26719. Considering that the only commit in that range that actually affects PictureFlow is 26713, I am reasonably sure that is the one that caused the problem.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 09 June 2010, 13:55 GMT
cheers.
can you roll back to 26712 and try patching each of the patches from http://www.rockbox.org/tracker/task/11270#comment35882 starting from #2? I would guess #4 would be the problem...
Comment by Chris Savery (csavery) - Wednesday, 09 June 2010, 14:26 GMT
No doubt it's #6 - audio_dropouts patch. This one inserts new yields in the rendering loop.
On Fuze this improves the smoothness and prevents audio from dropping out when scrolling quickly.
Comment by Chris Savery (csavery) - Wednesday, 09 June 2010, 14:45 GMT
If the problem is visible in USim then I could try finding a fix. I have no Nano2G to test.
That patch is worthwhile for Fuze targets as it improves both scrolling and audio playback.
Comment by Chris Savery (csavery) - Wednesday, 09 June 2010, 14:55 GMT
If it's a matter of the Nano2G not being able to handle the increased yields then perhaps try adding a condition around the 3 lines inserted at line 1807 so that it yields less often. eg. if( x % 10 == 0 ) { ... 3lines }. I did this during testing on Fuze but it worked the same or better without it. It doesn't need to yield so often but yielding more than once per render makes a marked improvement.
Comment by Michael Marley (mamarley) - Wednesday, 09 June 2010, 15:17 GMT
I was thinking it was probably #6, too. Why not just put an #ifdef around it so it only applies for the targets that need it?
Comment by Chris Savery (csavery) - Wednesday, 09 June 2010, 17:04 GMT
I'm ok with an #ifdef but I don't have any idea what targets have problems or not. I wouldn't want it to be a big messy thing there.
I mean for the time being it could be #ifndef for nano2g b/c we know that has problems. But if more deviations turn up we should figure out what underlying cause (if one) makes it a problem.

Did you try running with those 3 lines removed to see if PF returned to past behaviour? I can make a patch for testing that if need be.
Comment by Michael Marley (mamarley) - Wednesday, 09 June 2010, 18:02 GMT
Yes, removing those four lines does cause it to return to normal behavior. I don't understand why it doesn't work well though, since (I think) the Nano2G is one of the more powerful targets...
Comment by Chris Savery (csavery) - Thursday, 10 June 2010, 01:52 GMT
Try leaving the yield line in place but commenting the two following lines that resync (bmp = ... , and ptr = ...). The surface line can cause loading a file at a point which may introduce a visible delay. Disabling those may smooth the display but cause visible glitches to appear. If that helps then I'll look for a better approach for that.
Comment by Chris Savery (csavery) - Thursday, 10 June 2010, 04:25 GMT
Michael,
Can please test this patch?
It still works fine on my fuze but cuts down yields by 10x.
I tried some other methods here but none were smooth and glitch free.
I suspect on the Nano2G must have higher overhead on memory mgmt and/or task switching.
Let me know if this improves a little or a lot.
Comment by Michael Marley (mamarley) - Thursday, 10 June 2010, 10:36 GMT
With just the patch, it still runs jerkily (though less so than without it.) I am now trying removing those two lines...
Comment by Michael Marley (mamarley) - Thursday, 10 June 2010, 11:08 GMT
With the resync code commented out, there is no visible difference from just the yield-test patch.
Comment by Thomas Martitz (kugel.) - Tuesday, 17 August 2010, 13:36 GMT
I'm thinking the added yield was an incorrect fix.

On my fuze, you can get drop outs when scrolling slowly too, and it is loading slides (loading happens in a seperate thread, so it yields). That means the audio thread doesn't run even though there are yields.
Comment by Thomas Martitz (kugel.) - Tuesday, 17 August 2010, 13:58 GMT
Indeed, decreasing the priority (tested with PRIORITY_BUFFERING) of the loading thread let's pf run less flaky and without audio dropouts. I cannot tell if loading slides takes longer or not though (but I suspect it does).

Loading...