Buffer-size and buffer-duration configurable: where to post my patch?

General
1

Hello,

First of all, big thanks to all for this great software!

As I was enjoying my music playing with mopidy, I noticed a relatively long latency when switching tune-in channels, so I looked into the code and saw this:

# TODO: turn into config values...
playbin.set_property("buffer-size", 5 << 20)  # 5MB
playbin.set_property("buffer-duration", 5 * Gst.SECOND)

So I decided to make this patch to do exactly that. I made a patch that introduces two new configuration parameters in the audio section, min-buffer-size and min-buffer-duration, that can set these playbin-parameters.

Documentation on these parameters would be:

min-buffer-size: the minimum buffer-size that is given to gstream-playbin, in bytes.
Default value is 5242880 bytes, which was the old hardcoded value.
Minimum value is 0, which seems to work fine (but your mileage may vary); that way you can regulate the buffer size with the min-buffer-duration parameter.
min-buffer-duration: the minimum buffer-duration that is given to gstream-playbin, in milliseconds.
Default value is 5000, which was the old hardcoded value.
Minimum value is 0, although I recommend to use a minimum of 250ms so small internet disturbances will not disturb your audio stream.

Being new in this forum: would this patch be supported by the developers, and if so, what is the easiest procedure? I have a patch in diff format, can email it (or post it here), do you prefer a pull request, you want me to open an issue ā€¦?

Please let me know the best way to contribute to this beautiful project!

EDIT: this is in addition to the audio config parameter ā€œbuffer_timeā€, that seems to define the maximum buffer time gstreamer can store in itā€™s queue. In my experience buffer_time has no influence on latency, since it defines a maximum ā€¦

EDIT2: With the recommended values, my latency between switching live streams (tune-in) went down from 5-7 seconds to 2-5 seconds :grinning:

2 Likes
2

Thank you, thatā€™s definitely something that can improve the experience of listening to webradios ! This might also make it better to unpause a stream after a long pause (currently, the stream will play from the buffer until it is empty, then will get stuck).

I think the best way to submit the patch is to open a pull request on the github repo.

3

Yes, a PR. There might actually already be an open issue about this because we did actualy introduce some buffer changes like this a long time ago which ended up breaking some peopleā€™s audio. Maybe git blame will show that.

4

I did this one a while back to allow buffering the whole track. Should it be something similar to that?

5

@jjok, no my patch does not involve strategies like downloading entire tracks; it just makes the two playbin-variables buffer-size and buffer-duration configurable in mopidy.

6

Yes, my patch also was to make a playbin property configurable, like yours.

7

Ok people, Iā€™m going to need your help. Built a dev environment like excellently documented in docs/devenv.rst , applied my patch, run it through the flake8 and black tests.

BUT the pe37 tests fail, because of KeyError messages; which makes sense, since I add two keys, which I added to the schema, BUT tox seems to download the 3.1.0 environment and test against that.
It also generates some other errors, in code I didnā€™t even modify, but I suspect this is because of the KeyError messages ā€¦

How to deal with this?
If you want to look at the patch and/or run the tests yourself, I pushed my changes to https://github.com/dingo35/mopidy/tree/feature/reduce-latency

It is really a simple patch, only 20 lines, so I hope you wonā€™t disappoint me now I put all this time in your dev environment :slight_smile:

8

Well all the tests do pass for me, with python3.8.

tox -e py38
GLOB sdist-make: /home/sapristi/Documents/dev/external_code/dingo_mopidt/setup.py
py38 inst-nodeps: /home/sapristi/Documents/dev/external_code/dingo_mopidt/.tox/.tmp/package/1/Mopidy-3.1.0.zip
py38 installed: alabaster==0.7.12,appdirs==1.4.4,argcomplete==1.12.2,args==0.1.0,arrow==0.15.7,asn1crypto==1.4.0,astroid==2.4.2,attrs==19.3.0,autopep8==1.5.3,Babel==2.8.0,backcall==0.2.0,binaryornot==0.4.4,black==19.10b0,bleach==3.1.0,btrfsutil==5.9,CacheControl==0.12.6,cachetools==4.1.0,catfish==1.4.13,ceph==1.0.0,ceph-volume==1.0.0,cephfs==2.0.0,cephfs-shell==0.0.1,cffi==1.14.0,chardet==3.0.4,check-manifest==0.42,click==7.1.2,clint==0.5.1,colorama==0.4.4,configobj==5.0.6,contextlib2==0.6.0.post1,cookiecutter==1.7.2,coverage==5.2,cryptography==2.8,cupshelpers==1.0,decorator==4.4.2,distlib==0.3.1,distro==1.5.0,docopt==0.6.2,docutils==0.16,filelock==3.0.12,flake8==3.8.3,flake8-bugbear==20.1.4,flake8-import-order==0.18.1,google-api-core==1.17.0,google-api-python-client==1.8.3,google-auth==1.15.0,google-auth-httplib2==0.0.3,google-auth-oauthlib==0.4.1,googleapis-common-protos==1.51.0,gufw==20.4.0,hjson==3.0.2,html5lib==1.1,httplib2==0.18.1,idna==2.10,imagesize==1.2.0,ipython==7.16.1,ipython-genutils==0.2.0,isort==4.3.21,jedi==0.17.1,jeepney==0.4.2,Jinja2==2.11.2,jinja2-time==0.2.0,keymapviz==1.2.1,keyring==21.1.0,keyutils==0.6,lazy-object-proxy==1.4.3,lensfun==0.3.95,lightdm-gtk-greeter-settings==1.2.2,louis==3.15.0,lxml==4.6.2,Mako==1.1.3,Markdown==3.2.2,MarkupSafe==1.1.1,mccabe==0.6.1,menulibre==2.2.1,milc==1.0.10,mock==4.0.2,-e git+https://github.com/dingo35/mopidy.git@15b7b10724fb4ccd942541f9afec74e73e880d2e#egg=Mopidy,# Editable install with no version control (Mopidy-Bookmarks==0.1.4),-e /home/sapristi/.local/lib/python3.8/site-packages,Mopidy-Local==3.1.1,Mopidy-Mowecl @ file:///home/sapristi/Documents/dev/mopidy-mowecl,more-itertools==8.6.0,msgpack==1.0.0,mugshot==0.4.2,netsnmp-python==1.0a1,nose2==0.9.2,npyscreen==4.10.5,nr.collections==0.0.1,nr.databind.core==0.0.20.post1,nr.databind.json==0.0.13,nr.interface==0.0.3,nr.metaclass==0.0.5,nr.parsing.date==0.2.0,nr.pylang.utils==0.0.3,nr.stream==0.0.4,nr.utils.re==0.1.0,numpy==1.19.3,oauthlib==3.1.0,openshot-qt==2.5.1,ordered-set==4.0.2,packaging==20.4,pacman-mirrors==4.16.4,pandas==1.1.4,parso==0.7.0,pathspec==0.8.0,pathtools==0.1.2,pbr==5.5.1,pdoc3==0.8.3,peewee==3.13.3,pep517==0.9.1,pexpect==4.8.0,pickleshare==0.7.5,Pillow==7.2.0,pkginfo==1.5.0.1,pluggy==0.13.1,ply==3.11,poyo==0.5.0,progress==1.5,prompt-toolkit==3.0.5,protobuf==3.12.1,psutil==5.7.3,ptyprocess==0.6.0,py==1.9.0,pyasn1==0.4.8,pyasn1-modules==0.2.8,pycairo==1.20.0,pycodestyle==2.6.0,pycparser==2.19,pycryptodomex==3.9.9,pycups==2.0.1,pycurl==7.43.0.6,pydoc-markdown==3.1.1,pydocstyle==5.0.2,pyflakes==2.2.0,Pygments==2.5.2,PyGObject==3.38.0,Pykka==2.0.3,pylint==2.5.3,pyOpenSSL==19.1.0,pyparsing==2.4.7,PyQt5==5.15.2,PyQt5-sip==12.8.1,pytest==5.4.3,pytest-cov==2.10.0,python-dateutil==2.8.1,python-distutils-extra==2.39,python-jsonrpc-server==0.3.4,python-language-server==0.34.1,python-slugify==4.0.0,pytz==2020.1,pyxdg==0.26,PyYAML==5.3.1,pyzmq==19.0.1,qmk==0.0.36,rados==2.0.0,rbd==2.0.0,readme-renderer==24.0,regex==2020.7.14,reportlab==3.5.55,requests==2.24.0,requests-oauthlib==1.3.0,requests-toolbelt==0.9.1,resolvelib==0.5.1,responses==0.12.1,retrying==1.3.3,rgw==2.0.0,rope==0.17.0,rsa==4.0,samloader @ file:///home/sapristi/Documents/dev/external_code/samloader,SecretStorage==3.1.2,six==1.15.0,snowballstemmer==2.0.0,sphinxcontrib-applehelp==1.0.2,sphinxcontrib-devhelp==1.0.2,sphinxcontrib-htmlhelp==1.0.3,sphinxcontrib-jsmath==1.0.1,sphinxcontrib-qthelp==1.0.3,sphinxcontrib-serializinghtml==1.1.4,stevedore==3.3.0,team==1.0,terminator==1.92,text-unidecode==1.3,toml==0.10.2,tornado==6.1,tox==3.17.1,tqdm==4.42.1,traitlets==4.3.3,twine==3.1.1,typed-ast==1.4.1,udiskie==2.2.0,ufw==0.36,ujson==1.35,uritemplate==3.0.1,uritools==3.0.0,urllib3==1.25.10,virtualenv==20.0.21,virtualenv-clone==0.5.4,virtualenvwrapper==4.8.4,watchdog==0.10.3,wcwidth==0.2.5,webencodings==0.5.1,wrapt==1.12.1,yapf==0.30.0,youtube-dl==2020.12.2
py38 run-test-pre: PYTHONHASHSEED='996407653'
py38 run-test: commands[0] | python -m pytest --basetemp=/home/sapristi/Documents/dev/external_code/dingo_mopidt/.tox/py38/tmp --cov=mopidy --cov-report=term-missing
======================================================================================== test session starts =========================================================================================
platform linux -- Python 3.8.6, pytest-5.4.3, py-1.9.0, pluggy-0.13.1
cachedir: .tox/py38/.pytest_cache
rootdir: /home/sapristi/Documents/dev/external_code/dingo_mopidt, inifile: setup.cfg
plugins: cov-2.10.0
collected 1257 items                                                                                                                                                                                 

tests/test_commands.py ...........................................                                                                                                                             [  3%]
tests/test_exceptions.py .......                                                                                                                                                               [  3%]
tests/test_ext.py ................................                                                                                                                                             [  6%]
tests/test_help.py .                                                                                                                                                                           [  6%]
tests/test_httpclient.py ................                                                                                                                                                      [  7%]
tests/test_mixer.py ...                                                                                                                                                                        [  8%]
tests/test_version.py .                                                                                                                                                                        [  8%]
tests/audio/test_actor.py ...................................................................                                                                                                  [ 13%]
tests/audio/test_listener.py ......                                                                                                                                                            [ 14%]
tests/audio/test_scan.py ............                                                                                                                                                          [ 14%]
tests/audio/test_tags.py .....................................................                                                                                                                 [ 19%]
tests/audio/test_utils.py ..                                                                                                                                                                   [ 19%]
tests/backend/test_backend.py ...                                                                                                                                                              [ 19%]
tests/backend/test_listener.py ..                                                                                                                                                              [ 19%]
tests/config/test_config.py ......................................                                                                                                                             [ 22%]
tests/config/test_defaults.py ....                                                                                                                                                             [ 23%]
tests/config/test_schemas.py .........                                                                                                                                                         [ 23%]
tests/config/test_types.py .....................................................................................                                                                               [ 30%]
tests/config/test_validator.py ...............                                                                                                                                                 [ 31%]
tests/core/test_actor.py ........                                                                                                                                                              [ 32%]
tests/core/test_events.py .............                                                                                                                                                        [ 33%]
tests/core/test_history.py .......                                                                                                                                                             [ 33%]
tests/core/test_library.py .......................................................                                                                                                             [ 38%]
tests/core/test_listener.py ...............                                                                                                                                                    [ 39%]
tests/core/test_mixer.py ..............................                                                                                                                                        [ 41%]
tests/core/test_playback.py ...................................................................................                                                                                [ 48%]
tests/core/test_playlists.py ............................................                                                                                                                      [ 52%]
tests/core/test_tracklist.py .......................                                                                                                                                           [ 53%]
tests/file/test_browse.py .                                                                                                                                                                    [ 53%]
tests/file/test_lookup.py .                                                                                                                                                                    [ 54%]
tests/http/test_events.py ..                                                                                                                                                                   [ 54%]
tests/http/test_handlers.py ..............                                                                                                                                                     [ 55%]
tests/http/test_server.py ............................                                                                                                                                         [ 57%]
tests/internal/test_deps.py ........                                                                                                                                                           [ 58%]
tests/internal/test_http.py ...                                                                                                                                                                [ 58%]
tests/internal/test_jsonrpc.py ........................................                                                                                                                        [ 61%]
tests/internal/test_models.py ...............................                                                                                                                                  [ 64%]
tests/internal/test_network.py .....                                                                                                                                                           [ 64%]
tests/internal/test_path.py ...................................                                                                                                                                [ 67%]
tests/internal/test_playlists.py ............................                                                                                                                                  [ 69%]
tests/internal/test_validation.py .........................                                                                                                                                    [ 71%]
tests/internal/test_xdg.py ........                                                                                                                                                            [ 72%]
tests/m3u/test_playlists.py ..................................................................                                                                                                 [ 77%]
tests/m3u/test_translator.py .........................                                                                                                                                         [ 79%]
tests/models/test_fields.py ..........................................                                                                                                                         [ 82%]
tests/models/test_legacy.py .........................                                                                                                                                          [ 84%]
tests/models/test_models.py .................................................................................................................................................................. [ 97%]
..................                                                                                                                                                                             [ 98%]
tests/stream/test_library.py ....                                                                                                                                                              [ 99%]
tests/stream/test_playback.py .........                                                                                                                                                        [100%]

----------- coverage: platform linux, python 3.8.6-final-0 -----------
Name                               Stmts   Miss  Cover   Missing
----------------------------------------------------------------
mopidy/__init__.py                     8      1    88%   8
mopidy/__main__.py                   130    130     0%   1-231
mopidy/audio/__init__.py               4      0   100%
mopidy/audio/actor.py                440    173    61%   63-86, 89-98, 103-107, 113-120, 124-135, 138-142, 147-151, 154-155, 158-159, 162, 165-166, 169, 172-173, 213, 217-218, 220-221, 223, 225-226, 228-229, 231, 236, 266-267, 289, 295-296, 299-303, 313, 327-333, 336-337, 342, 345-366, 369-374, 384-399, 402-416, 454, 463-465, 508-512, 530, 540, 550, 553-562, 570, 598, 608, 620, 641-646, 664, 685-693, 824-856
mopidy/audio/constants.py              4      0   100%
mopidy/audio/listener.py              15      0   100%
mopidy/audio/scan.py                 188     67    64%   30, 85, 88, 96-99, 111-115, 135-138, 142-153, 157-162, 168, 176, 211, 214-217, 224-228, 247, 256, 261-264, 272, 276-302
mopidy/audio/tags.py                 102     21    79%   78, 81-86, 98-101, 108-123
mopidy/audio/utils.py                 45      6    87%   8, 36, 68-70, 86
mopidy/backend.py                     94     15    84%   67, 100, 113, 131, 139, 150, 214, 283, 295, 379, 396, 411, 419, 436, 456
mopidy/commands.py                   286    124    57%   124-125, 240-249, 301-348, 351-370, 373-381, 384-389, 392-393, 396-418, 421-427, 430-438, 441-443, 446-450, 453-455, 458-459, 462-463, 470-471, 474-480, 487-488, 491-492
mopidy/config/__init__.py            195     23    88%   104-111, 115-117, 196, 206-209, 246, 252, 309, 312-315, 318, 321, 324
mopidy/config/keyring.py              98     83    15%   7-8, 17, 27-64, 72-126, 130, 141-147, 153-158, 166, 170-172, 176-177
mopidy/config/schemas.py              75      6    92%   9, 69, 116-118, 124
mopidy/config/types.py               168      0   100%
mopidy/config/validators.py           13      0   100%
mopidy/core/__init__.py                8      0   100%
mopidy/core/actor.py                 149      8    95%   126, 153-154, 162-163, 255-257
mopidy/core/history.py                39      2    95%   67-68
mopidy/core/library.py               151      3    98%   149, 299, 346
mopidy/core/listener.py               35      0   100%
mopidy/core/mixer.py                  58      0   100%
mopidy/core/playback.py              291     31    89%   116, 150-151, 153-156, 160-164, 174, 287, 300, 305, 307-308, 380, 416, 439-440, 446, 455, 458-459, 467, 474, 500, 509, 543-544
mopidy/core/playlists.py             116      0   100%
mopidy/core/tracklist.py             258     34    87%   32, 197-202, 220, 222, 243-248, 271, 282, 313-318, 380, 400, 478, 484, 486, 488, 490, 492, 538-539, 542-543, 546-547, 570, 575, 581
mopidy/exceptions.py                  32      5    84%   13, 22-23, 44-45
mopidy/ext.py                        124     11    91%   182, 189, 192, 195, 198, 213-217, 286-287, 343
mopidy/file/__init__.py               23     11    52%   17-18, 21-27, 30-32
mopidy/file/backend.py                12      0   100%
mopidy/file/library.py                88     40    55%   19-25, 45, 48-53, 58-91, 102-104, 118-123, 125-130, 134, 142-143
mopidy/http/__init__.py               35     21    40%   17-18, 21-29, 32-35, 38-45
mopidy/http/actor.py                 129     44    66%   20-21, 32-55, 58-71, 74-79, 82, 109-125, 128-129, 154-156, 207
mopidy/http/handlers.py              134     13    90%   136, 154-156, 159-161, 204, 214-221, 273
mopidy/httpclient.py                  20      0   100%
mopidy/internal/__init__.py            0      0   100%
mopidy/internal/deprecation.py        20      5    75%   34, 44, 47-49
mopidy/internal/deps.py               70      6    91%   14-23, 101, 131, 136
mopidy/internal/formatting.py         17      4    76%   25-28
mopidy/internal/gi.py                 19      4    79%   9-24, 37
mopidy/internal/http.py               36      2    94%   33-34
mopidy/internal/jsonrpc.py           186      4    98%   290-292, 350
mopidy/internal/log.py               110     75    32%   29-30, 33-36, 43-45, 49-79, 83-93, 98-99, 102-110, 158-159, 163-164, 167-173, 176-185, 188-199
mopidy/internal/models.py             27      0   100%
mopidy/internal/network.py            18      0   100%
mopidy/internal/path.py               61      0   100%
mopidy/internal/playlists.py          93     12    87%   38-40, 52-54, 64, 71-72, 87, 128-129
mopidy/internal/process.py            26     17    35%   11-13, 26-27, 31-34, 38-52
mopidy/internal/storage.py            27      4    85%   29-31, 59
mopidy/internal/timer.py              10      3    70%   12-14
mopidy/internal/validation.py         57      1    98%   116
mopidy/internal/versioning.py         17      3    82%   10-11, 25
mopidy/internal/xdg.py                25      2    92%   63, 65
mopidy/listener.py                    15      2    87%   45-47
mopidy/m3u/__init__.py                22     10    55%   17-18, 21-26, 29-31
mopidy/m3u/backend.py                  8      0   100%
mopidy/m3u/playlists.py              139     18    87%   19, 31-34, 40-42, 53, 69, 82-83, 108-109, 145-146, 162, 168
mopidy/m3u/translator.py              55      3    95%   28-29, 74
mopidy/mixer.py                       31      6    81%   32, 51, 63, 85, 97, 112
mopidy/models/__init__.py             90      0   100%
mopidy/models/fields.py               66      1    98%   102
mopidy/models/immutable.py           107      0   100%
mopidy/models/serialize.py            14      1    93%   22
mopidy/softwaremixer/__init__.py      16     16     0%   1-24
mopidy/softwaremixer/mixer.py         39     39     0%   1-58
mopidy/stream/__init__.py             21     10    52%   14-15, 18-22, 25, 28-30
mopidy/stream/actor.py                88     11    88%   49-54, 78-79, 87, 90-91, 131-137, 157-162
mopidy/zeroconf.py                    72     57    21%   8-9, 17, 25-28, 48-72, 75, 86-135, 143-150
----------------------------------------------------------------
TOTAL                               5169   1188    77%


======================================================================================= 1257 passed in 10.28s ========================================================================================
______________________________________________________________________________________________ summary _______________________________________________________________________________________________
 py38: commands succeeded
 congratulations :)
9

Are you sure you are testing the feature/reduce-latency branch?

I get exactly your result in the developer branch, but the new code is in the new branch, as requested in the contribution guidelinesā€¦

I test py37, could that be the problem?

EDIT: Well I took a shot and made a pull request:

and guess what? 4 out of 5 tests failed, giving the exact same errors as in my dev environment.

So the good news is: my dev environment is ok.

The bad news: no clue on what causes all these errors, while the code runs fine ā€¦

10

Youā€™re right, I was on the develop branch, and tests indeed fail on your branch.

I donā€™t have much time to look at it right now, but my wild guess is that itā€™s just that some tests might need to be rewritten.

11

Ok I added the necesaary code to the tests, found the third-party-orb settings in CircleCI and found out how to rerun all the jobsā€¦ boy you sure have to jump a lot of hoops to add a few lines of code, but hey, I finally got the green light !!!

So now hoping someone is going to review my codeā€¦

For those testing: The default settings for min_buffer_size and min_buffer_duration are the current, hardcoded ones. If you want to reduce latency I recommend:

[audio]
min_buffer_size = 0
min_buffer_duration = 250

Please feedback in this thread on which settings work best for you, so we can find out what the best default settings should beā€¦

12

Happy New Year all !!

Now my pull request had passed all tests more than a week ago, had the green status from CircleCI, and now it is gone back to the status ā€œpassed 6 out of 8ā€, awaiting status of the other tests.

What to do? Do I keep pushing fake commits to restart CircleCI, until the next resync with dev branch is needed, or is someone going to review this pull request and commit it in the dev branch?

I did some measurments by profiling the mopidy code, and turns out the latency on my machine with gst-launch only is 1,6 * 10^9 , with mopidy it is 45 * 10^9 ; so there is still a lot to be gained in the mopidy code, but it doesnā€™t seem to make much sense of diving into this if my patches donā€™t get committedā€¦

13

The reason for the checks not being good enough any longer is that we recently switched from CircleCI to GitHub Actions. If you rebase on top of the develop branch and force push your branch, the checks will hopefully be satisfied once again.

14

Ok not sure what you mean by this, I am getting lost in CircleCI hell. This is what I did:
git pull (to update my local clone with the upstream developer branch)
git push dingo35 feature/reduce-latency (to push the changes back)

Now CircleCI emails me that this build is failingā€¦

EDIT: well this seemed to have done the trick, all green now. Hopefully someone reviews this soon before the next tool change, ha ha.

15

git pull only pulls in any changes from the tracked branch, e.g. dingo35/feature/reduce-latency. It wonā€™t include any changes to Mopidy upstream.

Instead, try this:

git fetch -a to get the latest changes from all remotes, or git fetch origin to get all changes from the remote named origin.

git rebase origin/develop to start with the develop branch from Mopidy and then reapply your changes on top.

git push -f -u dingo35 feature/reduce-latency to update the PR with your rebased branch. Because of -u git will remember what branch to push to next time, so if you do further changes, you can simply git push or git push -f. -f is required when rewriting history, like rebasing.

1 Like
16

My PR has been waiting for 8 days now, who is supposed to do what to get this baby reviewed?

17

I have no idea :smiley: I not part of mopidyā€™s maintainers

18

Ok. So it seems that someone else spotted this problem too :slight_smile:

I think I will give Commits Ā· dingo35/mopidy Ā· GitHub a try.

Thank you very much.

19

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.