Skip to content
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

sound improvements - better queue handling, refactoring and cleanups #400

Closed
totaam opened this issue Aug 2, 2013 · 28 comments
Closed

sound improvements - better queue handling, refactoring and cleanups #400

totaam opened this issue Aug 2, 2013 · 28 comments
Labels

Comments

@totaam
Copy link
Collaborator

totaam commented Aug 2, 2013

Issue migrated from trac ticket # 400

component: sound | priority: minor | resolution: fixed

2013-08-02 06:54:11: totaam created the issue


0.10 fixed a number of sound issues (ie: #362 and #379) but there is more we can and should do:

  • refactor the code so both the client and server use the same logic for creating and destroying the pipelines (servers typiccally run on Linux so they tend to be more forgiving - but we still tend to play catch up with fixes already client side: xpra server hangs on disconnect #396)
  • if the pipeline fails to start, we don't tell the client/server that requested it about the failure at the moment, which leaves a dead pipeline hanging. This is wasting cpu and showing the wrong state in the UI or "xpra info".
  • the way we deal with overruns is to re-start the whole pipeline which is wasteful (and potentially problematic), we should instead use the fact that changing the min-threshold-time (and maybe other attributes?) flushes the whole queue, effectively clearing the overrun
  • choose the sound quality: stereo vs mono, 48KHz, etc..
  • use a python wrapper to access pulseaudio rather than execing commands, one of:
@totaam
Copy link
Collaborator Author

totaam commented Oct 17, 2013

2013-10-17 08:43:56: totaam changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Oct 17, 2013

2013-10-17 08:43:56: totaam changed owner from antoine to totaam

@totaam
Copy link
Collaborator Author

totaam commented Oct 17, 2013

2013-10-17 08:43:56: totaam commented


some minor improvements were made for 0.11, the rest should get done for 0.12

@totaam
Copy link
Collaborator Author

totaam commented Mar 3, 2014

2014-03-03 15:07:44: totaam commented


This is needed, but needs to be done early in the release cycle to find all the problems. Re-scheduling.

@totaam
Copy link
Collaborator Author

totaam commented May 9, 2014

2014-05-09 15:12:50: totaam changed priority from major to minor

@totaam
Copy link
Collaborator Author

totaam commented May 9, 2014

2014-05-09 15:12:50: totaam commented


Some cleanups were done as part of #90: see r6315, r6308, etc

Too late for the large refactoring.. so re-scheduling it again.

@totaam
Copy link
Collaborator Author

totaam commented Aug 28, 2014

2014-08-28 14:54:06: totaam edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Aug 29, 2014

2014-08-29 07:21:50: totaam commented


See also: #663, #669 and #673.

@totaam
Copy link
Collaborator Author

totaam commented Sep 15, 2014

2014-09-15 11:04:12: totaam edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Sep 19, 2014

2014-09-19 15:40:50: totaam commented


Started in r7700.

@totaam
Copy link
Collaborator Author

totaam commented Sep 23, 2014

2014-09-23 07:06:53: totaam changed status from assigned to new

@totaam
Copy link
Collaborator Author

totaam commented Sep 23, 2014

2014-09-23 07:06:53: totaam changed owner from totaam to afarr

@totaam
Copy link
Collaborator Author

totaam commented Sep 23, 2014

2014-09-23 07:06:53: totaam commented


r7757 fixes this sink warning found on OSX:

pipeline warning: ['gstbasesink.c(3638): gst_base_sink_chain_unlocked (): \
    /GstPipeline:pipeline1/GstOsxAudioSink:osxaudiosink1:\n\
    Received buffer without a new-segment. Assuming timestamps start from 0.']

afarr: please check that I haven't broken anything on win32 or osx.

@totaam
Copy link
Collaborator Author

totaam commented Sep 23, 2014

2014-09-23 16:00:53: totaam edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Oct 6, 2014

2014-10-06 23:34:31: afarr changed owner from afarr to totaam

@totaam
Copy link
Collaborator Author

totaam commented Oct 6, 2014

2014-10-06 23:34:31: afarr commented


Sound still seems good, both OSX and Windows. SInk warning also gone.

Probably good to close this.

@totaam
Copy link
Collaborator Author

totaam commented Oct 7, 2014

2014-10-07 03:41:22: totaam changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Oct 7, 2014

2014-10-07 03:41:22: totaam commented


I'm not closing this ticket yet because there are still quite a few items that I haven't dealt with.

@totaam
Copy link
Collaborator Author

totaam commented Oct 27, 2014

2014-10-27 19:28:32: totaam commented


We can also deal with #669 and use multiprocessing.

@totaam
Copy link
Collaborator Author

totaam commented Nov 19, 2014

2014-11-19 22:54:40: antoine commented


We can't use multiprocessing with pygst because it relies on the gobject / glib main loop exclusively - and we can only have on of those.

@totaam
Copy link
Collaborator Author

totaam commented Mar 10, 2015

2015-03-10 02:30:15: antoine uploaded file sound-underrun.patch (2.0 KiB)

deal with underruns better: try to use min-threshold-time

@totaam
Copy link
Collaborator Author

totaam commented Mar 17, 2015

2015-03-17 13:51:11: antoine commented


The "sound underrun" patch has been applied as part of r8786 for #669.

This may affect underrun and overrun events...

@totaam
Copy link
Collaborator Author

totaam commented Mar 23, 2015

2015-03-23 17:19:12: antoine commented


r8827 allows us to disable this new variable min queue code using:

XPRA_VARIABLE_MIN_QUEUE=0 xpra attach ...

@totaam
Copy link
Collaborator Author

totaam commented Mar 30, 2015

2015-03-30 11:31:14: antoine commented


Some improvements in r8870 (see commit message).

@totaam
Copy link
Collaborator Author

totaam commented Mar 30, 2015

2015-03-30 11:32:26: antoine uploaded file sound-netcompress.patch (5.6 KiB)

allows us to compress raw WAV using the standard packet compressors (only saves about 25% with lz4, probably not worth it)

@totaam
Copy link
Collaborator Author

totaam commented Apr 30, 2015

2015-04-30 15:36:03: antoine changed status from assigned to closed

@totaam
Copy link
Collaborator Author

totaam commented Apr 30, 2015

2015-04-30 15:36:03: antoine set resolution to fixed

@totaam
Copy link
Collaborator Author

totaam commented Apr 30, 2015

2015-04-30 15:36:03: antoine commented


I think this ticket is good to close, we can deal with testing in #669.

If someone has time, running benchmarks with the netcompress patch could be useful, to see what kind of trade-off we get: 25% compression is poor, but the CPU load is going to be minimal..

@totaam totaam closed this as completed Apr 30, 2015
@totaam totaam added the audio label Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant