Incomplete cache files on interrupted requests

[spijet]

Hello @sentriz!

I've been poking around with v0.15.0 and found that some cached songs are either chopped off after a couple of seconds from the start or mangled in many funny YTP-like ways. After some trial and error I can reproduce the "chop-off" thingy with 100% reliability in Sonixd, which seems to preload next songs by requesting stream.view and then RST'ing the connection after getting several seconds of audio, which makes gonic to stop writing to the cache file as well.

Looking at the source for new caching transcode module, I see that the cache is being written straight to result file, without any temporary files, and removed only on transcode (i.e. ffmpeg) errors:

https://github.com/sentriz/gonic/blob/v0.15.0/transcode/transcoder_caching.go#L44-L52

In the original encode module I proposed, cache was supposed to be written to a temporary/partial file and then flushed/synced to disk before being renamed with a "proper" filename, meaning it's now safe to use it:

gonic/server/encode/encode.go

Lines 218 to 225 in fd211d7

	// close all pipes and flush cache part file
	_ = pipeWriter.Close()
	if err := cacheFile.Sync(); err != nil {
	return fmt.Errorf("flushing %q: %w", cachePath, err)
	}
	_ = cacheFile.Close()
	// rename cache part file to mark it as valid cache file
	_ = os.Rename(cachePartPath, cachePath)

[sentriz]

ah you're right, thanks! do you think about this approach? ce31310

which is to make Transcode() return an error when it was killed (for example when the http request was terminated early)

that way we hit the caching transcoders case of deleting cache files. and we don't need to worry about deleting any temp files

[spijet]

I'll check if it works with the case I found and will let you know. :)

Actually, I was thinking about letting ffmpeg finish writing the output into the cache file even if request was terminated, but this approach can be undesirable in some cases (e.g. if you accidentally tapped on a 40-minute-long one-track album 😁).

[spijet]

As a bonus, you can have a laugh at one track that got mangled on my phone:
https://magada.ga/shared/fuef.m4a.gpg

Use gpg to decrypt it with a password (the password is the name of this project). ;)

[sentriz]

As a bonus, you can have a laugh at one track that got mangled on my phone: https://magada.ga/shared/fuef.m4a.gpg

Use gpg to decrypt it with a password (the password is the name of this project). ;)

😁 thats hilarious, the drumming reminds me of this https://www.youtube.com/watch?v=CNhhXmXQ_bY

[spijet]

Or this: https://www.youtube.com/watch?v=B5NoXaSajq0 :)

[thirtythreeforty]

Thank you both very much for finding and fixing this. I kept hitting this intermittently, and I knew it was a gonic bug, but I could never actually reproduce it on command. Cheers!