don't know if on purpose, but it seems the 3.3 android app sends periodic getRandomSongs requests (every ~10 seconds) for a while, when first starting it, even if don't click on any of the "random" button on the home screen.
I found the reason for the app's behavior: getRandomSongs() is called regularly through ShufflePlayBuffer. This class (being instantiated by DownloadService) is responsible for keeping a buffer of randomized songs always available for shuffle mode.
Initially, 50 songs are picked randomly from the server and pushed into a buffer. In shuffle mode, songs are played from that buffer. As soon as the buffer size drops below a certain threshold (40 songs), ShufflePlayBuffer will download more randomized songs to refill the buffer to 50 songs again. The threshold check occurs at regular intervals of 10 seconds.
There is somewhat of a design flaw in ShufflePlayBuffer as it completely fills the buffer on startup even when shuffle mode is disabled. Although the refill logic is triggered regularly through an ExecutorService job, subsequent runs do not affect performance in non-shuffle mode as the buffer won't be depleted then. Still, the initial download request is unneeded and, in my opinion, may even affect user experience in low-bandwidth environments (read: EDGE/GPRS networks).
My proposed solution to fix this is to modify the code such that the ExecutorService is not started in ShufflePlayBuffer's constructor but when shuffle mode is enabled. This would be an external trigger in a suitable location, such as DownloadServiceImpl.setShufflePlayEnabled(). Similarly, the ExecutorService job should be stopped as soon as shuffle mode is disabled. I am not sure how the current code handles scenarios where the user enables shuffle mode and starts playback before ShufflePlayBuffer can get a chance to fill the buffer; some effort may be needed to get this case right. (Though this may happen even with the current implementation.)
I'd be willing to take a stab at modifying the code, doing some testing, and providing a patch. Before doing so, however, I'd like to get Sindre's feedback on the issue. Sindre, do you read this? :)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Using the built-in demo server, I can confirm at least one call to getRandomSongs() on startup.
Unless anyone else has already started investigations, I'll try to do so.
I found the reason for the app's behavior: getRandomSongs() is called regularly through ShufflePlayBuffer. This class (being instantiated by DownloadService) is responsible for keeping a buffer of randomized songs always available for shuffle mode.
Initially, 50 songs are picked randomly from the server and pushed into a buffer. In shuffle mode, songs are played from that buffer. As soon as the buffer size drops below a certain threshold (40 songs), ShufflePlayBuffer will download more randomized songs to refill the buffer to 50 songs again. The threshold check occurs at regular intervals of 10 seconds.
There is somewhat of a design flaw in ShufflePlayBuffer as it completely fills the buffer on startup even when shuffle mode is disabled. Although the refill logic is triggered regularly through an ExecutorService job, subsequent runs do not affect performance in non-shuffle mode as the buffer won't be depleted then. Still, the initial download request is unneeded and, in my opinion, may even affect user experience in low-bandwidth environments (read: EDGE/GPRS networks).
My proposed solution to fix this is to modify the code such that the ExecutorService is not started in ShufflePlayBuffer's constructor but when shuffle mode is enabled. This would be an external trigger in a suitable location, such as DownloadServiceImpl.setShufflePlayEnabled(). Similarly, the ExecutorService job should be stopped as soon as shuffle mode is disabled. I am not sure how the current code handles scenarios where the user enables shuffle mode and starts playback before ShufflePlayBuffer can get a chance to fill the buffer; some effort may be needed to get this case right. (Though this may happen even with the current implementation.)
I'd be willing to take a stab at modifying the code, doing some testing, and providing a patch. Before doing so, however, I'd like to get Sindre's feedback on the issue. Sindre, do you read this? :)
I created a patch for upstream integration: https://sourceforge.net/tracker/?func=detail&aid=3477473&group_id=126265&atid=705134
Please note the UX restriction described on that page.