a new idea of a "transcoder" #205

sentriz · 2022-03-10T00:58:49Z

can encode tracks to mp3, opus, etc as usual
can also decode to pcm audio (to be used in jukebox situations later on)
guesses the Content-Length for on the fly transcoding
introduces a Transcoder interface. implemented by FFmpegTranscoder, CachingTranscoder (takes another Transcoder), NoneTranscoder
profiles are stored as strings (split by shlex) so that users could customise them in future
cache keys are more "exact" (not just checking profile bitrate and replay gain field changes, but the entire ffmpeg command string and filepath)

TODO

see how DSub and other clients react to having a Content-Length header for on the fly transcodes

@spijet thoughts on this one?

sentriz · 2022-03-10T01:01:12Z

server/transcode/transcode.go

+	OpusRG  = NewProfile("audio/ogg", 96, `ffmpeg -v 0 -i <file> -ss <seek> -map 0:a:0 -vn -b:a <bitrate> -c:a libopus -vbr on -af "volume=replaygain=track:replaygain_preamp=6dB:replaygain_noclip=0, alimiter=level=disabled, asidedata=mode=delete:type=REPLAYGAIN" -metadata replaygain_album_gain= -metadata replaygain_album_peak= -metadata replaygain_track_gain= -metadata replaygain_track_peak= -metadata r128_album_gain= -metadata r128_track_gain= -f opus -`)
+
+	PCM16le = NewProfile("audio/wav", 0, `ffmpeg -v 0 -i <file> -ss <seek> -c:a pcm_s16le -ac 2 -f s16le -`)
+)


a bit more tidy without line wrapping 😎

cmd/gonictranscode/gonictranscode.go

spijet · 2022-03-25T18:07:46Z

Hello @sentriz!

Sorry for slow response — a lot of stuff has been happening lately.

This looks very interesting to me, and having encoding profiles stored as strings actually reminds me of SubSonic, where the user could set up multi-stage transcoding rules (e.g. use some_command to decode some specific audio format to raw PCM and then pass it to ffmpeg to encode it to target format via pipe). This is useful for some formats that are somehow not supported by ffmpeg out of the box, like emulated retro-game audio or tracker modules.

I hope I'll be able to carve out some time and check the code in more detail this weekend. :)

sentriz · 2022-03-25T18:20:01Z

thanks! yeah it would be much appreciated if you checked it out.

also I don't think I mentioned yet, the reason for the refactoring is to share the code with jukebox. I'm working on making it take whatever file and decode to PCM. which removes the beep dependency which is giving me trouble at the moment

…ing/caching

spijet · 2022-05-20T18:44:04Z

Oh fug, it's been 2 months now. D:
Looking at it right now.

Also, before I forget it again for another year — what do you think about introducing decoder profiles too? We could use similar kind of profiles to add a decoding step in transcoding chain, which would decode some sound format to PCM and then feed it into ffmpeg or to the sound output (in jukebox mode). This could be useful for some less-common music formats (e.g. tracker modules and emulated retro music).

spijet · 2022-05-20T18:47:16Z

As for Content-Length header, some clients actually use it to estimate the track length. Subsonic API even offers a query parameter for this, so the client could ask the server to (not) add this header for transcoded music.

spijet

Here's a couple of questions/comments from me in case it's not too late to review the code yet. :)

spijet · 2022-05-20T18:53:48Z

server/server.go

 	r.Handle("/getCoverArt{_:(?:\\.view)?}", ctrl.HR(ctrl.ServeGetCoverArt))
 	r.Handle("/stream{_:(?:\\.view)?}", ctrl.HR(ctrl.ServeStream))
+	r.Handle("/download{_:(?:\\.view)?}", ctrl.HR(ctrl.ServeStream))


Are you sure you want to serve transcoded music via download API too? IIRC, many clients use it to download source (i.e. unmodified) tracks as-is, and to pre-cache stuff they just use the stream API instead.

yeah very nice point. i really wasn't sure about this. but what i find annoying is that when i'm on a train streaming music, dsub will transcode the first track that i select so it stream nice and quick. but then for the tracks that it caches ahead, it uses download.view. so since it's downloading straight flacs, it just hangs

you're right that download should just serve the raw file so i'll put it back. but any ideas how to fix my issue? 😁

Hmm, maybe you should file a bug report to dSub developer(s)? :)
In my case, iSub (yet another SubSonic API client for iOS, this time with Opus support) preloads next track (and pre-caches tracks when I tap a "download to cache" button on a playlist) using /stream, as expected. IIRC, some Android client I used to use had different settings for Wi-Fi and 3G/LTE/etc modes, and the default setting was to download and (pre-)cache unmodified tracks in all cases.

spijet · 2022-05-20T18:55:00Z

transcode/transcode.go

+
+// Store as simple strings, since we may let the user provide their own profiles soon
+var (
+	MP3   = NewProfile("audio/mpeg", 128, `ffmpeg -v 0 -i <file> -ss <seek> -map 0:a:0 -vn -b:a <bitrate> -c:a libmp3lame -af "volume=replaygain=track:replaygain_preamp=6dB:replaygain_noclip=0, alimiter=level=disabled, asidedata=mode=delete:type=REPLAYGAIN" -metadata replaygain_album_gain= -metadata replaygain_album_peak= -metadata replaygain_track_gain= -metadata replaygain_track_peak= -metadata r128_album_gain= -metadata r128_track_gain= -f mp3 -`)


That's what I was afraid of — ultra-long ffmpeg command lines! :D

ahaha yeah it's a lot. but i'm kinda into it :D

spijet · 2022-05-20T18:58:55Z

transcode/transcode.go

+
+type Profile struct {
+	bitrate BitRate // the default bitrate, but the user can request a different one
+	seek    time.Duration


What does seek do (or indicate) in this case? Is it like a smallest block of time the format allows to seek or something?

its passed to ffmpeg as the -ss argument which says seek to and then start encoding. only used for jukebox at the moment

Ah, so it's sorta like "skip N seconds from start" instead. Got it. :)

spijet · 2022-05-20T19:23:22Z

transcode/transcoder_caching.go

+func cacheKey(cmd string, args []string) string {
+	// the cache is invalid whenever transcode command (which includes the
+	// absolute filepath, bit rate args, replay gain args, etc.) changes
+	sum := md5.New()


We could use xxhash here, since it seems to be faster than MD5 (I used it as a hash key for cached tracks in my original PR, IIRC, and it's still here in dependency list).

Here's an issue in go-guerrilla project with some "xxhash vs MD5" benchmarks.

yeah i think xxhash is very cool. but in this case we're dealing with really small data, and I like to keep the 3rd party libraries miminal. on my laptop it seems to be 20µs to hash a profile and args

Fair enough. Well, if the xxhash module is not used anywhere else (probably not), then MD5 is a good choice. A bit vintage, but it works! :)

spijet · 2022-05-20T19:28:05Z

WTF, GitHub kept showing this PR as "Open" even after I dropped the browser cache and refreshed the page, but as soon as I submitted my review, it turned into "Merged". :D

Once again, my apologies for being so late to the party.

sentriz · 2022-05-20T19:39:22Z

howdy @spijet 👋

what do you think about introducing decoder profiles too? We could use similar kind of profiles to add a decoding step in transcoding chain, which would decode some sound format to PCM and then feed it into ffmpeg or to the sound output (in jukebox mode). This could be useful for some less-common music formats (e.g. tracker modules and emulated retro music).

yep that's exactly what i'm doing 👍 see the transcode.PCM16le profile, it's used in this PR
#211

As for Content-Length header, some clients actually use it to estimate the track length. Subsonic API even offers a query parameter for this, so the client could ask the server to (not) add this header for transcoded music.

woah cool I didn't know the API had a param for this, i'll check it out

spijet · 2022-05-20T20:15:01Z

I just checked the diff of a local mini-fork I'm still running at home (based on commit 4a4298a :D), and there I just set Content-Length using a transcoded file size, if it exists in cache:

		cacheFile, cfErr := os.Stat(path)
		if cfErr != nil {
			log.Printf("failed to stat cache file `%s`: %v", path, cfErr)
		} else {
			contentLength := fmt.Sprintf("%d", cacheFile.Size())
			w.Header().Set("Content-Length", contentLength)
		}
 		http.ServeFile(w, r, path)

Pretty stupid, but it works. :D

spijet · 2022-05-20T20:22:54Z

see the transcode.PCM16le profile, it's used in this PR

Will do, and will check #211 too.

(Also, it's probably time to update my Gonic installation) :D

sentriz commented Mar 10, 2022

View reviewed changes

sentriz force-pushed the develop branch 3 times, most recently from 3ee7bf2 to b275899 Compare March 10, 2022 01:28

sentriz commented Mar 10, 2022

View reviewed changes

cmd/gonictranscode/gonictranscode.go Outdated Show resolved Hide resolved

sentriz force-pushed the develop branch 2 times, most recently from 4711434 to f49c2c9 Compare March 15, 2022 18:06

sentriz force-pushed the develop branch from f49c2c9 to 5fd1746 Compare March 22, 2022 19:43

sentriz force-pushed the master branch from e8fbc1c to 1ab47d6 Compare March 22, 2022 20:40

sentriz force-pushed the develop branch 2 times, most recently from 2617db6 to f444678 Compare March 23, 2022 23:42

sentriz force-pushed the develop branch 2 times, most recently from c7c5324 to ffd6614 Compare March 25, 2022 18:35

sentriz force-pushed the develop branch 3 times, most recently from 477d624 to 05c07e6 Compare April 12, 2022 22:50

feat(transcode): add a generic transcoding package for encoding/decod…

0cf5296

…ing/caching

sentriz force-pushed the develop branch from 05c07e6 to 0cf5296 Compare April 12, 2022 22:51

refactor: move shared packages up a level

2496741

sentriz merged commit 2496741 into master Apr 12, 2022

spijet reviewed May 20, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

a new idea of a "transcoder" #205

a new idea of a "transcoder" #205

sentriz commented Mar 10, 2022 •

edited

Loading

sentriz Mar 10, 2022

spijet commented Mar 25, 2022

sentriz commented Mar 25, 2022

spijet commented May 20, 2022

spijet commented May 20, 2022

spijet left a comment

spijet May 20, 2022

sentriz May 20, 2022

spijet May 20, 2022

spijet May 20, 2022

sentriz May 20, 2022

spijet May 20, 2022

sentriz May 20, 2022

spijet May 20, 2022

spijet May 20, 2022

sentriz May 20, 2022

spijet May 20, 2022

spijet commented May 20, 2022 •

edited

Loading

sentriz commented May 20, 2022

spijet commented May 20, 2022

spijet commented May 20, 2022

a new idea of a "transcoder" #205

a new idea of a "transcoder" #205

Conversation

sentriz commented Mar 10, 2022 • edited Loading

Choose a reason for hiding this comment

spijet commented Mar 25, 2022

sentriz commented Mar 25, 2022

spijet commented May 20, 2022

spijet commented May 20, 2022

spijet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spijet commented May 20, 2022 • edited Loading

sentriz commented May 20, 2022

spijet commented May 20, 2022

spijet commented May 20, 2022

sentriz commented Mar 10, 2022 •

edited

Loading

spijet commented May 20, 2022 •

edited

Loading