LMS-Community / plugin-Qobuz

Squeebox plugin to stream from qobuz.com
31 stars 16 forks source link

Replaced sysread in order to use chunck reading (range) from qobuz. #15

Closed marcoc1712 closed 1 year ago

marcoc1712 commented 6 years ago

As discussed here: http://forums.slimdevices.com/showthread.php?108577-QOBUZ-Plugin-buffering-caching, here we have the patch to enable chunk reading in qobuz plugin, thanks to bpa and Philippe_44 help.

Nothing else has been changed and - in my system - hicups disappered and dropouts have been reduced (some is still here now and then).

The problem:

Qobuz plugin uses the core::sysread to get the audio file from qobuz, that means http get request is made with no range support, so download could not be interrupted and resumed later on by another request.

Other than this we have all the pipeline buffering parameters being hardcoded or stored in hidden preferences that different modules uses with different meaning.

As a result, the actual chunk size is about 1Kb, the lower threshold is calculated in different ways when playing or rebuffering, same for 'upper' theshold.

The proposed solution:

a. overload the plugin protocolhandle class with a new sysread routine, that uses chunked reading.

b. use the range size instead of the actual chunk size (that is hidden to the application) as the minimal amount of data passed to the decoding pipe.

c. buffering (in the player) is now limited only by the player buffer size.

d. the input buffer (just after http gets) normally contains less or equal the range size, then is moved to the out buffer (just before the decoding pipeline) using the range size. This buffer size is limited (to the player buffer size) to avoid it could raise to much in case of decoding errors.

e. thanks to d, before playback starts we always have RANGE SIZE of data ready to be decoded (or transfered to the player) other than at least the BUFFERTHRESHOLD that is still limited to 255 Kb as before (see notes below).

NOTES:

This is probably enougth to higly reduce hicups and dropouts caused by line problems and comunication errors, but is not a complete solution becouse (from major to minor) :

  1. We still store only two tracks, no matter the lenght, so if we have, say:

  2. 5 min

  3. 4 secs

  4. 10 min.

and assuming we have a big enought buffer, we will store first and second tracks in the first seconds of first track playback, but then download will stop until the first track streams out, then download of the third start, but buffer is now only the size of the second (4 secs) + max(BUFFERTHRESHOLD, RANGE SIZE).

Better than before, but still a bad situation that could be avoided starting download of the third track a little bit in advance, instead of waiting for complete streamout.

  1. PLAY (defined in Squeezebox.pm) and REBUFFER (defined in Player.pm) calculates thresholds and buffer sizes with different math and usind data with different meanings, preferences (buffersecs and bufferThreshold) are only partially considered, bitrate are guessed.

Best solution will be to subclass player and overload there at least play and rebuffer, using same math and 'real' values for calculations.

  1. chunksizes used in pumping data throught the pipeline to the player output are, normally, 32 Kb, when in input and, probably, in decoding applications are bigger, that meaning we need more cycles to feed each step. Probably it could be moved to a preference as Is done here for the first 'pump'.

Maybe we also have other points, I live these notes to some brave enougth.

Marco.

michaelherger commented 6 years ago

Oh, one more thing: could you summarize in a comment what that change is supposed to do? I don't want to go to that thread to try to figure out what it is doing.

marcoc1712 commented 6 years ago

This feaures are currently being tested by some people and anyone could partecipate just downloading from my beta repo: http://www.marcoc1712.it/downloads/repository_beta.xml, retained myself to publish it in sb user forum, butI could do, just tell me what's your preference.

marcoc1712 commented 6 years ago

Loaded a new commit with all changes required but being optional, I have to study a way for that, in the code is a matter of calling CORE::sysread or _sysread, but based on what? Shall I put the choice in the UI?

marcoc1712 commented 6 years ago

Edited the firs comment and added a description of the problem and proposed solution.

Added some controls in the UI, now the usage of _sysread is optional, as per your request. Second commit is becouse I forgot some Dump...

NOTE: String are only in english (and maybe is better you have a look at).

To me is finished, now is up to you.

michaelherger commented 4 years ago

BTW: isn't this PR obsolete with the recent changes @philippe44 applied to LMS 8 (see https://github.com/Logitech/slimserver/pull/384 and follow-up changes)?

philippe44 commented 4 years ago

BTW: isn't this PR obsolete with the recent changes @philippe44 applied to LMS 8 (see Logitech/slimserver#384 and follow-up changes)? I'm not 100% sure. What I think can be obsoleted is the code to handle seeking into flac files. If we add a parsing of the flac $track (like you did in WiMP), then seeking & resynchronizing will be handled by the LMS core.

About what @marcoc1712 is doing, I re-read some of the discussions we had at that time (and that was way before I acquired a proper knowledge of some core LMS streaming mechansim) and it's similar to what I do in YT, but for other reasons as DASH/HLS are naturally divided into small chunks. He seems to want to create very large chunk.

If this is about interruption why not setting the thresholds of the stream buffer + outputbuffer high enough so that we have enough delay of acquired streaming bytes before actually playing, why not using bufferThreshold which is an overloadable method as overloaded by the protocolHandler?