tsquillario / Jamstash

HTML5 Music Streamer for Subsonic
http://jamstash.com
272 stars 88 forks source link

Improve song length handling when transcoding #258

Closed mvn23 closed 5 years ago

mvn23 commented 5 years ago

d2e57cc introduced the "Estimate Content Length" option for transcoded streams. As stated in the commit itself, this option has its limitations, especially with VBR transcoding. This PR reverts that commit, effectively removing the option, and replaces it with jplayer's duration option. We provide the known song length to jplayer directly rather than asking the server to estimate.

pR0Ps commented 5 years ago

I just tested adding the duration to the player on the latest version of Firefox and it doesn't quite replace the estimateContentLength option.

I tested with a server that uses transcoding (so the length is unknown) but correctly estimates the file size when asked to do so.

What I see with the duration set and the estimateContentLength param enabled is a grey bar on the black background representing the percentage of the song that's loaded and a white bar that represents the current place in the song. This is correct. It looks like this:

|=====-----          | (25% played, 50% loaded)
|=====----------     | (25% played, 75% loaded)
|=====---------------| (25% played, 100% loaded)

With the duration set and estimateContentLength disabled, the dark grey loading bar is always at 100% of the screen. As more of the file loads, the white play bar jumps around as it adjusts to the new percentage of the loaded file that has been played. It looks like this:

|==========----------| (25% played, 50% loaded)
|=======-------------| (25% played, 75% loaded)
|=====---------------| (25% played, 100% loaded)

This is the same behavior I see without duration specified. The only change is that the duration listed below the loading bar is now correct (instead of increasing as more of the song loads).

To be clear, adding the duration is definitely a win, I just think that the estimateContentLength option is still a good idea.


I couldn't let this go so I messed around with jPlayer to try to understand it better. Turns out the duration that's set on the media is actually only used for the duration display, not anything to do with the loaded/played bars. This matches what I saw above.

Since the duration that's displayed when the file size isn't known increases as more of the file is downloaded, it can be combined with the known duration to get a duration-based progress bar. The following quick patch to jPlayer caused it to show an accurate loading bar (EDIT: Firefox-only) without the server estimating anything:

--- bower_components/jplayer/dist/jplayer/jquery.jplayer.js 2014-12-14 19:55:35.000000000 -0500
+++ bower_components/jplayer/dist/jplayer/jquery.jplayer.js 2019-10-02 23:33:08.810664424 -0400
@@ -1579,9 +1579,9 @@
            }

            ct = media.currentTime;
-           cpa = (this.status.duration > 0) ? 100 * ct / this.status.duration : 0;
+           cpa = (this.status.media.duration > 0) ? 100 * ct / this.status.media.duration : 0;
            if((typeof media.seekable === "object") && (media.seekable.length > 0)) {
-               sp = (this.status.duration > 0) ? 100 * media.seekable.end(media.seekable.length-1) / this.status.duration : 100;
+               sp = (this.status.media.duration > 0) ? 100 * media.seekable.end(media.seekable.length-1) / this.status.media.duration : 100;
                cpr = (this.status.duration > 0) ? 100 * media.currentTime / media.seekable.end(media.seekable.length-1) : 0; // Duration conditional for iOS duration bug. ie., seekable.end is a NaN in that case.
            } else {
                sp = 100;

I'm not sure where jPlayer is getting it's partial duration from (maybe it's doing it's own estimations based on the bitrate?) but it seems to work. I even tested it with VBR transcoding and while in the first few seconds the loading bar jumped around a bit (the playing bar was perfect), it stabilized quickly.


TL;DR: Adding the duration is good, but don't remove the estmateContentLength option yet. It can definitely be removed if/when jPlayer is fixed upstream/patched locally though.

mvn23 commented 5 years ago

Thanks for the feedback. I did not see the same behavior on Chromium, but I will test Firefox as well now. If combining the duration with estimateContentLength yields better results on all common browsers I will update the PR accordingly.

edit: indeed, combining both solutions (estimateContentLength and jPlayer duration) seems to yield the best results across the board, especially with VBR transcoding. This PR will get a lot smaller soon ;)

pR0Ps commented 5 years ago

Great, thanks!