[pulseaudio-discuss] [PATCH] virtual-surround: check if resampled memblock is not equal to input
Since commit e32a408b3cdd46857fdf12210c1bf5bdbf3a96f8, we silence the input memblock in order to give the resampler enough input samples, if necessary. But if there is no need to resample the hrir, the resampled memblock is actually the same as the input memblock. Thus, we have to make sure that we do not silence it in this case. --- src/modules/module-virtual-surround-sink.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/modules/module-virtual-surround-sink.c b/src/modules/module-virtual-surround-sink.c index 4915278..adaa58f 100644 --- a/src/modules/module-virtual-surround-sink.c +++ b/src/modules/module-virtual-surround-sink.c @@ -738,8 +738,10 @@ int pa__init(pa_module*m) { /* add silence to the hrir until we get enough samples out of the resampler */ while (hrir_copied_length < hrir_total_length) { pa_resampler_run(resampler, &hrir_temp_chunk, &hrir_temp_chunk_resampled); -/* Silence input block */ -pa_silence_memblock(hrir_temp_chunk.memblock, &hrir_temp_ss); +if (hrir_temp_chunk.memblock != hrir_temp_chunk_resampled.memblock) { +/* Silence input block */ +pa_silence_memblock(hrir_temp_chunk.memblock, &hrir_temp_ss); +} if (hrir_temp_chunk_resampled.memblock) { /* Copy hrir data */ -- 1.8.0 ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH] virtual-surround: check if resampled memblock is not equal to input
On Sat, 2012-11-24 at 12:32 +0100, Niels Ole Salscheider wrote: > Since commit e32a408b3cdd46857fdf12210c1bf5bdbf3a96f8, we silence the > input memblock in order to give the resampler enough input samples, if > necessary. > But if there is no need to resample the hrir, the resampled memblock is > actually the same as the input memblock. Thus, we have to make sure that > we do not silence it in this case. Thanks, applied to my "next" branch. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] jackdbus module, pulse fails to conform on device reservation API
On Wed, 2012-11-21 at 03:04 -0500, Ian Malone wrote: > On 20 November 2012 14:01, Tanu Kaskinen wrote: > > On Tue, 2012-11-20 at 20:43 +0200, Tanu Kaskinen wrote: > >> On Tue, 2012-11-20 at 17:59 +, Ian Malone wrote: > >> > Thanks, I'll give it a go. I think handling the already_owner case in > >> > reserve.c as well might be worthwhile since there may be other ways to > >> > get to that state. > >> > >> I think it's a bug in the application if it calls rd_acquire for a > >> device that it already owns. An assertion might be the way to go. If not > >> an assertion, then at least an error. > > > > When writing that, I didn't realize that the current code already > > returns an error if dbus_bus_request_name() returns > > DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER. I think that's a fine way of > > handling it, so in my opinion nothing needs to be done here. > > > > Okay I agree there is probably a more serious bug somewhere. I'll just > point out that the current response does not result in an error in > verbose output and that encountering that response results in removing > the reserve method and handlers, meaning if the service isn't broken > before it happens then it certainly is afterwards. Yes, if that error happens, the device reservation won't work, but the problem is not that the error is handled badly, the problem is that the error happens. Btw, error from rd_acquire() does cause a log message to be printed in reserve-wrap.c: pa_log_debug("Failed to acquire reservation lock on device '%s': %s", device_name, pa_cstrerror(-k)); That's probably not very useful when debugging this bug, though. When debugging this, I'd like you to add this assertion to rd_acquire(): assert(k != DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER); Then run pulseaudio in gdb and get a backtrace. Also, would it be possible for you try 2.99.2 (with the patch for reserve.c added)? > Moving on: 0001-reserve-Call-change_cb-only-if-there-actually-was-a-.patch > I think we might be getting somewhere. This doesn't actually fix the > problem, the service without a reservedevice1/audioN method still gets > produced (this is the use case of trying KDE audio properties), but > playback doesn't work, so it may be doing something related to the > problem: > > pulseaudio -v output, no patch: http://pastebin.com/yfGEu1qx > pulseaudio -v output, patched: http://pastebin.com/HkjSST4p Note that you can add another "v" to get even more verbose output. I'm not sure what I should look for in those logs. You said that "playback doesn't work" - is that a good or a bad thing? Usually it's a bad thing, but is the playback supposed to not work in this case due to the device reservation? It looks like the alsa sink doesn't get unsuspended when the last stream is created, so it looks like the device reservation is doing its job (partially). -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] jackdbus module, pulse fails to conform on device reservation API
On Mon, 2012-11-19 at 21:45 +0100, Brendan Jones wrote: > I'm seeing this in virtual box under KDE (also Fedora 18 / pulseaudio 2.1). I guess it should be quite easy to reproduce this if I'd try this with virtual box with the same setup as you. I've never used virtual box (or any other virtual machine), but I think I should learn to do that. After installing virtual box, I guess I need some OS image - could you point me to the one that you were using when you reproduced this? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 2/2] sinkwidget: Move format selection options to 'Advanced' expander
On Tue, 2012-11-20 at 20:08 +0530, Arun Raghavan wrote: > There's no reason to present this for all S/PDIF and HDMI cases. The > user can select it when required. This change only modifies the glade file. I think the logic for enabling and disabling the advanced options needs to be updated too. Otherwise the advanced options may be disabled even when the encodings are supposed to be editable. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 1/2 pavucontrol] mainwindow: Show the availability of the ports and profiles.
On Fri, 2012-11-16 at 00:12 +0100, poljar (Damir Jelić) wrote: > diff --git a/src/mainwindow.cc b/src/mainwindow.cc > index 1041eab..63e02e8 100644 > --- a/src/mainwindow.cc > +++ b/src/mainwindow.cc > @@ -254,12 +254,31 @@ static void set_icon_name_fallback(Gtk::Image *i, const > char *name, Gtk::IconSiz > > static void updatePorts(DeviceWidget *w, std::map > &ports) { > std::map::iterator it; > +PortInfo p; > + > +for (uint32_t i = 0; i < w->ports.size(); i++) { > +Glib::ustring desc; > +it = ports.find(w->ports[i].first); > + > +if (it == ports.end()) > +continue; > + > +p = (*it).second; Out of curiosity, is the reason for using (*it).second instead of it->second that you think it looks better, or something else? -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 pavucontrol] mainwindow: Show the availability of the ports and profiles.
On Fri, 2012-11-16 at 00:12 +0100, poljar (Damir Jelić) wrote: > Hi. Here comes the second version of this patch. > > This version fixes the issues pointed out by Tanu: > - don't add all ports of a card to the sink/source > - make the strings translatable > > I also attached a second patch which changes the logic to only > show if a profile is unplugged. These two can be squashed together or the > second patch > can be dropped depending on the desired policy. Squashed and pushed. Thanks! -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 1/2 pavucontrol] mainwindow: Show the availability of the ports and profiles.
On Sat, Nov 24, 2012 at 04:29:05PM +0200, Tanu Kaskinen wrote: > On Fri, 2012-11-16 at 00:12 +0100, poljar (Damir Jelić) wrote: > > diff --git a/src/mainwindow.cc b/src/mainwindow.cc > > index 1041eab..63e02e8 100644 > > --- a/src/mainwindow.cc > > +++ b/src/mainwindow.cc > > @@ -254,12 +254,31 @@ static void set_icon_name_fallback(Gtk::Image *i, > > const char *name, Gtk::IconSiz > > > > static void updatePorts(DeviceWidget *w, std::map > > &ports) { > > std::map::iterator it; > > +PortInfo p; > > + > > +for (uint32_t i = 0; i < w->ports.size(); i++) { > > +Glib::ustring desc; > > +it = ports.find(w->ports[i].first); > > + > > +if (it == ports.end()) > > +continue; > > + > > +p = (*it).second; > > Out of curiosity, is the reason for using (*it).second instead of > it->second that you think it looks better, or something else? > I think I saw that somewhere else so I tried to be consistent, but I'm not completely sure anymore. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 1/2 pavucontrol] mainwindow: Show the availability of the ports and profiles.
On Sat, 2012-11-24 at 16:06 +0100, Damir Jelić wrote: > On Sat, Nov 24, 2012 at 04:29:05PM +0200, Tanu Kaskinen wrote: > > On Fri, 2012-11-16 at 00:12 +0100, poljar (Damir Jelić) wrote: > > > diff --git a/src/mainwindow.cc b/src/mainwindow.cc > > > index 1041eab..63e02e8 100644 > > > --- a/src/mainwindow.cc > > > +++ b/src/mainwindow.cc > > > @@ -254,12 +254,31 @@ static void set_icon_name_fallback(Gtk::Image *i, > > > const char *name, Gtk::IconSiz > > > > > > static void updatePorts(DeviceWidget *w, std::map > > PortInfo> &ports) { > > > std::map::iterator it; > > > +PortInfo p; > > > + > > > +for (uint32_t i = 0; i < w->ports.size(); i++) { > > > +Glib::ustring desc; > > > +it = ports.find(w->ports[i].first); > > > + > > > +if (it == ports.end()) > > > +continue; > > > + > > > +p = (*it).second; > > > > Out of curiosity, is the reason for using (*it).second instead of > > it->second that you think it looks better, or something else? > > > I think I saw that somewhere else so I tried to be consistent, but I'm > not completely sure anymore. Yes, I noticed myself too (after sending that message) that there were also other places where that style was used. I committed a separate patch that changes all occurrences of (*it).second to it->seconds. -- Tanu ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH 1/3] sink_input: new volume_factor system
On Wed, 2012-11-14 at 15:19 -0200, Flavio Ceolin wrote: > This patch makes possible to set more than one volume factor. The > real value of the volume_factor will be the multiplication of these > values. Please avoid breaking compilation between patches. Patches 1/3 and 2/3 should be squashed, otherwise module-position-event-sounds doesn't compile after this patch. One missing thing is remapping the volume_factor_sink entries in pa_sink_input_start_move(). > src/pulsecore/sink-input.c | 172 > ++--- > src/pulsecore/sink-input.h | 19 +++-- > 2 files changed, 158 insertions(+), 33 deletions(-) > > diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c > index 7a7575a..22a462f 100644 > --- a/src/pulsecore/sink-input.c > +++ b/src/pulsecore/sink-input.c > @@ -48,6 +48,34 @@ > > PA_DEFINE_PUBLIC_CLASS(pa_sink_input, pa_msgobject); > > +struct volume_factor_entry { > +char *key; > +pa_cvolume *volume; The volume field can be just plain pa_cvolume instead of a pointer. > +}; > + > +static struct volume_factor_entry *volume_factor_entry_new(const char *key, > const pa_cvolume *volume) { > +struct volume_factor_entry *entry; > + Here should be pa_assert(key) and pa_assert(volume). > +entry = pa_xnew(struct volume_factor_entry, 1); > +entry->key = pa_xstrdup(key); > + > +entry->volume = pa_xnew(pa_cvolume, 1); > +*entry->volume = *volume; > + > +return entry; > +} > + > +static void volume_factor_entry_free(struct volume_factor_entry > *volume_entry) { Pointer assertion missing from here too. > +pa_xfree(volume_entry->key); > +pa_xfree(volume_entry->volume); > + > +pa_xfree(volume_entry); > +} > + > +static void volume_factor_entry_free2(struct volume_factor_entry > *volume_entry, void *userdarta) { > +volume_factor_entry_free(volume_entry); > +} I would personally do an exception here and not have an assertion for volume_entry, so this function doesn't need changing. > + > static void sink_input_free(pa_object *o); > static void set_real_ratio(pa_sink_input *i, const pa_cvolume *v); > > @@ -74,6 +102,9 @@ pa_sink_input_new_data* > pa_sink_input_new_data_init(pa_sink_input_new_data *data > data->proplist = pa_proplist_new(); > data->volume_writable = TRUE; > > +data->volume_factor_items = pa_hashmap_new(pa_idxset_string_hash_func, > pa_idxset_string_compare_func); > +data->volume_factor_sink_items = > pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func); > + > return data; > } > > @@ -111,28 +142,28 @@ void > pa_sink_input_new_data_set_volume(pa_sink_input_new_data *data, const pa_cv > data->volume = *volume; > } > > -void pa_sink_input_new_data_apply_volume_factor(pa_sink_input_new_data > *data, const pa_cvolume *volume_factor) { > +void pa_sink_input_new_data_add_volume_factor(pa_sink_input_new_data *data, > const pa_cvolume *volume_factor, const char *key) { > +struct volume_factor_entry *v; > + > pa_assert(data); > +pa_assert(key); > pa_assert(volume_factor); > > -if (data->volume_factor_is_set) > -pa_sw_cvolume_multiply(&data->volume_factor, &data->volume_factor, > volume_factor); > -else { > -data->volume_factor_is_set = TRUE; > -data->volume_factor = *volume_factor; > -} > +v = volume_factor_entry_new(key, volume_factor); > +if (pa_hashmap_put(data->volume_factor_items, key, v) < 0) > +volume_factor_entry_free(v); The key that is inserted to the hashmap has to be v->key, because we don't have guarantees for the lifetime of the key that is passed as the function argument. Also, if pa_hashmap_put() fails here, that's a bug, so you can replace the if with pa_assert_se(pa_hashmap_put(data->volume_factor_items, v->key, v) >= 0); > } > > -void pa_sink_input_new_data_apply_volume_factor_sink(pa_sink_input_new_data > *data, const pa_cvolume *volume_factor) { > +void pa_sink_input_new_data_add_volume_factor_sink(pa_sink_input_new_data > *data, const pa_cvolume *volume_factor, const char *key) { > +struct volume_factor_entry *v; > + > pa_assert(data); > +pa_assert(key); > pa_assert(volume_factor); > > -if (data->volume_factor_sink_is_set) > -pa_sw_cvolume_multiply(&data->volume_factor_sink, > &data->volume_factor_sink, volume_factor); > -else { > -data->volume_factor_sink_is_set = TRUE; > -data->volume_factor_sink = *volume_factor; > -} > +v = volume_factor_entry_new(key, volume_factor); > +if (pa_hashmap_put(data->volume_factor_sink_items, key, v) < 0) > +volume_factor_entry_free(v); Same comment as above. > } > > void pa_sink_input_new_data_set_muted(pa_sink_input_new_data *data, > pa_bool_t mute) { > @@ -204,6 +235,16 @@ void pa_sink_input_new_data_done(pa_sink_input_new_data > *data) { > if (data->format) > pa_format_info_free(data->format); > >
Re: [pulseaudio-discuss] jackdbus module, pulse fails to conform on device reservation API
On 24 November 2012 13:01, Tanu Kaskinen wrote: > Note that you can add another "v" to get even more verbose output. I'm > not sure what I should look for in those logs. You said that "playback Neither am I, but then I'm not a developer for this project. > doesn't work" - is that a good or a bad thing? Usually it's a bad thing, > but is the playback supposed to not work in this case due to the device > reservation? It looks like the alsa sink doesn't get unsuspended when > the last stream is created, so it looks like the device reservation is > doing its job (partially). Given that it's pulse reserving the device lack of playback is not a feature. Right, will run it again with 2.99.2. Assuming I can get it built and packaged. -- imalone http://ibmalone.blogspot.co.uk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss