spl0k / supysonic

Supysonic is a Python implementation of the Subsonic server API.
https://supysonic.readthedocs.io
GNU Affero General Public License v3.0
264 stars 58 forks source link

Mostly-transcoded files aren't cached #202

Closed pR0Ps closed 3 years ago

pR0Ps commented 3 years ago

If a client loads 99% of a file with transcoding, then skips to the next one, the 99% transcoded file will be discarded instead of being finished and cached.

I discovered this while trying to figure out why no files were being cached when playing them with Jamstash.

The flow of my issue with Jamstash as I understand it is:

  1. A transcode is requested by Jamstash with estimateContentLength=true (currently required to get a progress bar while loading).
  2. estimateContentLength causes Supysonic to set theContent-Length header to a lowball estimate.
  3. The smaller Content-Length header causes the browser to stop requesting more data and close the connection slightly before fully downloading all the transcoded data.
  4. Supysonic gets a write error while sending the transcoded data and stops processing the response.
  5. The almost completely transcoded file is discarded instead of being cached.

Now obviously it's impossible to accurately estimate the content length and clients that ask for this feature yet also immediately close the connection when they reach the estimated size are kinda shooting themselves in the foot. With that being said though, in Jamstash's case, the HTTP response with the incorrect Content-Length header is handled entirely by the browser so the best option is to just not use estimateContentLength which makes the UI much worse. I would imagine this issue also impacts other web-based players or players that use higher-level HTTP libraries.

Even if all clients did the right thing, having the ability to finish off and cache a transcode even after the connection is closed would be beneficial. If a client loads 99% of a transcoded song before skipping to the next one, it would be nice if that transcoded data could be cached, regardless of why the client stopped loading it.

In my opinion a good general rule would be to always finish transcoding (and therefore caching) a file if the amount of bytes that have already been generated is greater than or equal to some percentage (95%?) of the estimated number of bytes of the response. This would fix the above issue and also allow files that are close to being fully transcoded to be finished off and cached for next time.

spl0k commented 3 years ago

You might want to take a look at the branch issue202 ;)

spl0k commented 3 years ago

This got merged in master. Don't hesitate to reopen if something is wrong.