New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audio: Add a config option for queue buffer size #1442

Merged
merged 3 commits into from Feb 13, 2016

Conversation

3 participants
@trygveaa
Member

trygveaa commented Feb 13, 2016

It may help to increase this for users that are experiencing buffering
before track changes. Workaround for #1409.

Not sure if I should have added any tests for this, and if so how?

audio: Add a config option for queue buffer size
It may help to increase this for users that are experiencing buffering
before track changes. Workaround for #1409.
Expects an integer above 0.
Sets the buffer size of the GStreamer queue. If you experience buffering
before track changes, it may help to increase this. The default is letting

This comment has been minimized.

@adamcik

adamcik Feb 13, 2016

Member

On my system the defaults are the following:

  max-size-buffers    : Max. number of buffers in the queue (0=disable)
                        flags: readable, writable
                        Unsigned Integer. Range: 0 - 4294967295 Default: 200
  max-size-bytes      : Max. amount of data in the queue (bytes, 0=disable)
                        flags: readable, writable
                        Unsigned Integer. Range: 0 - 4294967295 Default: 10485760
  max-size-time       : Max. amount of data in the queue (in ns, 0=disable)
                        flags: readable, writable
                        Unsigned Integer64. Range: 0 - 18446744073709551615 Default: 1000000000

So for time we have one second as our presumed default, but the other max values might also kick in limiting the time value. Should we mention anything about this, and/or suggest "at least a few seconds" as the possible size?

This comment has been minimized.

@trygveaa

trygveaa Feb 13, 2016

Member

Hm, so would you have to increase all those three in worst case? I guess it would be a bit overkill to make all configurable?

This comment has been minimized.

@adamcik

adamcik Feb 13, 2016

Member

Yeah, you might need all of them. But I would rather keep this simple and expose just this simplified concept. We can always guesstimate buffers and bytes per ms if we find out we need to.

This comment has been minimized.

@trygveaa

trygveaa Feb 13, 2016

Member

I added "at least a few seconds" as per your suggestion, and specified the GStreamer default.

@adamcik

This comment has been minimized.

Member

adamcik commented Feb 13, 2016

A bunch of tests didn't like this. So you either need to add {'buffer_time': None} to the test settings. Or update the code to use .get('buffer_time').

@@ -169,6 +169,10 @@ Audio
This should be fixed properly together with :issue:`1222`. (Fixes:
:issue:`1430`, PR: :issue:`1438`)
- Add a new config option, buffer_time, for setting the buffer time of the

This comment has been minimized.

@jodal

jodal Feb 13, 2016

Member

Use the following to link to the docs:

:confval:`audio/buffer_time`
@trygveaa

This comment has been minimized.

Member

trygveaa commented Feb 13, 2016

Fixed the tests and docs link.

jodal added a commit that referenced this pull request Feb 13, 2016

Merge pull request #1442 from trygveaa/fix/audio-config-buffer-size
audio: Add a config option for queue buffer size

@jodal jodal merged commit 6b87381 into mopidy:develop Feb 13, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment