| Message ID | 1331652059-10090-12-git-send-email-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 13 Mar 2012, Marc-Andr? Lureau wrote: > - period seems to be unused now > - plive is very obscure and should either be documented or perhaps removed Plive is obscure because the use case was, in any case it's been years since i've used it so it can, probably, safely go away. Period on the other hand is unused because i somehow missed the subtle change of behavior in one of the patches made by, i think, Gerd. What i know would like to know is this: Jan, back in the day, advocated for making the mixeng the default (for musicpal), so, Jan - can you test this series? [..snip..]
On 03/13/12 16:47, malc wrote: > On Tue, 13 Mar 2012, Marc-Andr? Lureau wrote: > >> - period seems to be unused now > Period on the other hand is unused because i somehow missed the subtle > change of behavior in one of the patches made by, i think, Gerd. Uhm, which patch? I think that wasn't intentional, at least I can't remember intentionally disabling period. Maybe I missed the subtle change of behavior too. I do see the point in having this configurable as this is a cpu overhead vs. latency tradeoff which one might want to tweak depending on the use case. If we keep it we should pass it down to the audio backends I think. pulseaudio for example uses buffer sizes adjusted for the default period (see qpa_init_out()), when running with a non-default period pulseaudio should be able to adjust the buffer sizes accordingly. cheers, Gerd
Hi On Wed, Mar 14, 2012 at 10:22 AM, Gerd Hoffmann <kraxel@redhat.com> wrote: > On 03/13/12 16:47, malc wrote: >> On Tue, 13 Mar 2012, Marc-Andr? Lureau wrote: >> >>> - period seems to be unused now > >> Period on the other hand is unused because i somehow missed the subtle >> change of behavior in one of the patches made by, i think, Gerd. > > Uhm, which patch? I think that wasn't intentional, at least I can't > remember intentionally disabling period. Maybe I missed the subtle > change of behavior too. Could be: 39deb1e496de81957167daebf5cf5d1fbd5e47c2 > I do see the point in having this configurable as this is a cpu overhead > vs. latency tradeoff which one might want to tweak depending on the use > case. But that would impact a/v sync, or can you report added latency back to the guest somehow? I imagine audio backend latency should depend on the configured device buffering/latency, not on an environment tweak. regards
On 03/14/12 12:20, Marc-André Lureau wrote: > Hi > > On Wed, Mar 14, 2012 at 10:22 AM, Gerd Hoffmann <kraxel@redhat.com> wrote: >> On 03/13/12 16:47, malc wrote: >>> On Tue, 13 Mar 2012, Marc-Andr? Lureau wrote: >>> >>>> - period seems to be unused now >> >>> Period on the other hand is unused because i somehow missed the subtle >>> change of behavior in one of the patches made by, i think, Gerd. >> >> Uhm, which patch? I think that wasn't intentional, at least I can't >> remember intentionally disabling period. Maybe I missed the subtle >> change of behavior too. > > Could be: > > 39deb1e496de81957167daebf5cf5d1fbd5e47c2 Yes, looks like this one is it. >> I do see the point in having this configurable as this is a cpu overhead >> vs. latency tradeoff which one might want to tweak depending on the use >> case. > > But that would impact a/v sync, or can you report added latency back > to the guest somehow? I imagine audio backend latency should depend on > the configured device buffering/latency, not on an environment tweak. Sure, with lower latency you get better a/v sync too. Guest interfacing is next to impossible I think, simply because real hardware has no need for that and thus the interfaces simply don't exist in the hardware we are emulating. We can try to fix that with a virtio soundcard which has such interfaces, but it could be this simply shifts the issue from the driver/hardware interface to the os-kernel/driver interface. cheers, Gerd
Hi > Guest interfacing is next to impossible I think, simply because real > hardware has no need for that and thus the interfaces simply don't > exist > in the hardware we are emulating. We can try to fix that with a > virtio > soundcard which has such interfaces, but it could be this simply > shifts > the issue from the driver/hardware interface to the os-kernel/driver > interface. btw, the Xen folks just merged a PulseAudio sink/src implementation a few days ago: http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=50a7bf1175eaf07521c00bde8eed2f820e64437f
diff --git a/audio/audio.c b/audio/audio.c index bb94133..c2e6e15 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -30,7 +30,6 @@ #define AUDIO_CAP "audio" #include "audio_int.h" -/* #define DEBUG_PLIVE */ /* #define DEBUG_LIVE */ /* #define DEBUG_OUT */ /* #define DEBUG_CAPTURE */ @@ -62,11 +61,6 @@ struct fixed_settings { static struct { struct fixed_settings fixed_out; struct fixed_settings fixed_in; - union { - int hertz; - int64_t ticks; - } period; - int plive; int log_to_monitor; int try_poll_in; int try_poll_out; @@ -96,8 +90,6 @@ static struct { } }, - .period = { .hertz = 250 }, - .plive = 0, .log_to_monitor = 0, .try_poll_in = 1, .try_poll_out = 1, @@ -1453,9 +1445,6 @@ static void audio_run_out (AudioState *s) while (sw) { sw1 = sw->entries.le_next; if (!sw->active && !sw->callback.fn) { -#ifdef DEBUG_PLIVE - dolog ("Finishing with old voice\n"); -#endif audio_close_out (sw); } sw = sw1; @@ -1642,18 +1631,6 @@ static struct audio_option audio_options[] = { }, /* Misc */ { - .name = "TIMER_PERIOD", - .tag = AUD_OPT_INT, - .valp = &conf.period.hertz, - .descr = "Timer period in HZ (0 - use lowest possible)" - }, - { - .name = "PLIVE", - .tag = AUD_OPT_BOOL, - .valp = &conf.plive, - .descr = "(undocumented)" - }, - { .name = "LOG_TO_MONITOR", .tag = AUD_OPT_BOOL, .valp = &conf.log_to_monitor, @@ -1898,18 +1875,6 @@ static void audio_init (void) } } - if (conf.period.hertz <= 0) { - if (conf.period.hertz < 0) { - dolog ("warning: Timer period is negative - %d " - "treating as zero\n", - conf.period.hertz); - } - conf.period.ticks = 1; - } else { - conf.period.ticks = - muldiv64 (1, get_ticks_per_sec (), conf.period.hertz); - } - e = qemu_add_vm_change_state_handler (audio_vm_change_state_handler, s); if (!e) { dolog ("warning: Could not register change state handler\n" diff --git a/audio/audio_template.h b/audio/audio_template.h index 519432a..4120afb 100644 --- a/audio/audio_template.h +++ b/audio/audio_template.h @@ -433,29 +433,6 @@ SW *glue (AUD_open_, TYPE) ( return sw; } -#ifdef DAC - if (conf.plive && sw && (!sw->active && !sw->empty)) { - live = sw->total_hw_samples_mixed; - -#ifdef DEBUG_PLIVE - dolog ("Replacing voice %s with %d live samples\n", SW_NAME (sw), live); - dolog ("Old %s freq %d, bits %d, channels %d\n", - SW_NAME (sw), sw->info.freq, sw->info.bits, sw->info.nchannels); - dolog ("New %s freq %d, bits %d, channels %d\n", - name, - as->freq, - (as->fmt == AUD_FMT_S16 || as->fmt == AUD_FMT_U16) ? 16 : 8, - as->nchannels); -#endif - - if (live) { - old_sw = sw; - old_sw->callback.fn = NULL; - sw = NULL; - } - } -#endif - if (!glue (conf.fixed_, TYPE).enabled && sw) { glue (AUD_close_, TYPE) (card, sw); sw = NULL; @@ -495,9 +472,6 @@ SW *glue (AUD_open_, TYPE) ( * old_sw->info.bytes_per_second / sw->info.bytes_per_second; -#ifdef DEBUG_PLIVE - dolog ("Silence will be mixed %d\n", mixed); -#endif sw->total_hw_samples_mixed += mixed; } #endif
- period seems to be unused now - plive is very obscure and should either be documented or perhaps removed Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- audio/audio.c | 35 ----------------------------------- audio/audio_template.h | 26 -------------------------- 2 files changed, 0 insertions(+), 61 deletions(-)