On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote: > 2017-02-23 12:01 GMT+01:00 <marcel-hollerb...@t-online.de>: > > > First of all to the technical side: > > > > The commit i pushed removes the code that fetches the volume from pa > > when the drag of a slider is done. Reason for that was that we might > > set a old volume from pulseaudio which is not the last one the user > > dragged to. This can happen bacause the volume is sometimes not up to > > date when we are ending the drag, if pa gives us a new volume that is > > not exact to the one we had when the drag stopped (for example some > > clamping of the volume happened inside pulseaudio) then we are getting > > a callback where we are setting the volume again in the slider. So > > that removing of code does not really remove a feature. We are getting > > notified later about a new volume. > > > > No we aren't notified, because the volume doesn't change. If the volume > is setted to 100 as example and you set it to 105. emix won't change the > volume. The volume is still setted to 100 not 105 ...
Well depends on what you are doing. If you are dragging then it should work, if you do it with clicking/keybindings then it will clamp at 100% (BARRIER feature),. Can you elaborate a bit on your usecase? > > > > > > On Thu, Feb 23, 2017 at 11:30:18AM +0100, Michaël Bouchaud wrote: > > > Sorry for the miss of comments about this revert. > > > > > > I explained to you, this commit remove something needed to get proper > > value > > > setted to the volume slider. > > > > What is that something ? As explained above, if something else changes > > the volume again, we are getting a callback that results in a update of > > the slider. > > > > > When you have done this revert you have posted this video > > > https://omicron.homeip.net/filedump/mixer_gadget_bug.ogv. We can see > > that > > > emixer works great but not the enlightenment mixer module. Both share > > about > > > 90 > > > percents of the code to talk to pulseaudio. So no need to talk with > > > pulseaudio > > > guys, we got something weird into our enlightenment volume mixer. I > > analyze > > > code of two and see the enlightenment module make some alloc to set the > > > volume > > > and emixer doesn't. I fix that in the previous commit > > > (dbdf411b488fd4d3f37a26d8cb142b25aba784d6). I've asked morlenxus to > > check my > > > branch, but he never came back to say if this work. I don't see why this > > > wouldn't working because emixer and enlightenment module are the same > > code > > > now. > > > > While analyzing the code of emixer.c and the module you probebly should > > have noticed that emixer doesnt do exactly THAT. it does not set the > > volume again on drag end. thats NOT done. The only place where this is > > done is on a input volume slider, not the sink or sink input. > > And the bug in the video is clearly happening for sink / sink input. > > > > ??? Sorry but the code is in the revert ... > Your commit (fba185798cf75eaeaba4a95d2be25fb2fea6ef1a) brings back the resetting of the volume on drag stop. Show me where this is done in emixer.c for a slider of the sink volume. > > > > > The weirdness part in our enlightenment codebase here is that we > > sometimes get into the state where we are getting updates from > > pulseaudio very slowly (Can be caused by havy work load, etc. etc.). > > > > Absolutely not, we are just rereading the value we have setted to > pulseaudio. > Pulseaudio isn't solicited at this time. > Its not like this, read the pulse code of emix. if we are setting a volume to pulseaudio it just translates it to a pulseaudio volume struct and sets it for that sink. It never attaches the same volume struct to the sink. The volume in the sink is only updated when pa sends the callback of "hey volume has changed" (pulse.c:215) But now, lets assume you are right. That means that we are setting EXACTLY the same value we have just gotten from our slider, back to our slider. Why is that needed? > > > > > > And more important, not getting this commit into our code just hidde us > > the > > > bug > > > and give us no chance to spot it. > > > > Thats a part i dont understand :) > > > > > > > > > > > 2017-02-23 9:09 GMT+01:00 Marcel Hollerbach < > > marcel-hollerb...@t-online.de>: > > > > > > > bu5hm4n pushed a commit to branch master. > > > > > > > > http://git.enlightenment.org/core/enlightenment.git/commit/?id= > > > > 9745890a37036091d5dec320fecda7ed4c6bdb6c > > > > > > > > commit 9745890a37036091d5dec320fecda7ed4c6bdb6c > > > > Author: Marcel Hollerbach <marcel-hollerb...@t-online.de> > > > > Date: Thu Feb 23 09:08:24 2017 +0100 > > > > > > > > Revert "Revert "mixer: do not set back the value from emix once the > > > > drag is finished"" > > > > > > > > This reverts commit fba185798cf75eaeaba4a95d2be25fb2fea6ef1a. > > > > > > > > There is not even a description why you reverted it. This is a > > bugfix > > > > that fixed a bug. So talk to me what the issue is, but please stop > > > > reverting commits silently. > > > > --- > > > > src/modules/mixer/e_mod_main.c | 11 ----------- > > > > src/modules/mixer/emixer.c | 13 ------------- > > > > 2 files changed, 24 deletions(-) > > > > > > > > diff --git a/src/modules/mixer/e_mod_main.c b/src/modules/mixer/e_mod_ > > > > main.c > > > > index ac805cc..2c86915 100644 > > > > --- a/src/modules/mixer/e_mod_main.c > > > > +++ b/src/modules/mixer/e_mod_main.c > > > > @@ -481,16 +481,6 @@ _slider_changed_cb(void *data EINA_UNUSED, > > > > Evas_Object *obj, > > > > } > > > > > > > > static void > > > > -_slider_drag_stop_cb(void *data EINA_UNUSED, Evas_Object *obj, > > > > - void *event EINA_UNUSED) > > > > -{ > > > > - EINA_SAFETY_ON_NULL_RETURN(mixer_context->sink_default); > > > > - Emix_Sink *s = (Emix_Sink *)mixer_context->sink_default; > > > > - int val = s->volume.volumes[0]; > > > > - elm_slider_value_set(obj, val); > > > > -} > > > > - > > > > -static void > > > > _sink_selected_cb(void *data, Evas_Object *obj EINA_UNUSED, void > > > > *event_info EINA_UNUSED) > > > > { > > > > Emix_Sink *s = data; > > > > @@ -554,7 +544,6 @@ _popup_new(Instance *inst) > > > > evas_object_show(slider); > > > > elm_slider_min_max_set(slider, 0.0, emix_max_volume_get()); > > > > evas_object_smart_callback_add(slider, "changed", > > _slider_changed_cb, > > > > NULL); > > > > - evas_object_smart_callback_add(slider, "slider,drag,stop", > > > > _slider_drag_stop_cb, NULL); > > > > elm_slider_value_set(slider, volume); > > > > elm_box_pack_end(bx, slider); > > > > evas_object_show(slider); > > > > diff --git a/src/modules/mixer/emixer.c b/src/modules/mixer/emixer.c > > > > index 5cde881..1bcd96c 100644 > > > > --- a/src/modules/mixer/emixer.c > > > > +++ b/src/modules/mixer/emixer.c > > > > @@ -49,17 +49,6 @@ _cb_sink_volume_change(void *data, > > > > } > > > > > > > > static void > > > > -_cb_sink_volume_drag_stop(void *data, > > > > - Evas_Object *obj, > > > > - void *event EINA_UNUSED) > > > > -{ > > > > - Evas_Object *bxv = data; > > > > - Emix_Sink *sink = evas_object_data_get(bxv, "sink"); > > > > - int vol = sink->volume.volumes[0]; > > > > - elm_slider_value_set(obj, vol); > > > > -} > > > > - > > > > -static void > > > > _cb_sink_mute_change(void *data, > > > > Evas_Object *obj, > > > > void *event_info EINA_UNUSED) > > > > @@ -134,8 +123,6 @@ _emix_sink_add(Emix_Sink *sink) > > > > elm_box_pack_end(bx, sl); > > > > evas_object_show(sl); > > > > evas_object_smart_callback_add(sl, "changed", > > _cb_sink_volume_change, > > > > bxv); > > > > - evas_object_smart_callback_add(sl, "slider,drag,stop", > > > > - _cb_sink_volume_drag_stop, bxv); > > > > > > > > ck = elm_check_add(win); > > > > evas_object_data_set(bxv, "mute", ck); > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > -- > > > Michaël Bouchaud (yoz) <y...@efl.so> > > > ------------------------------------------------------------ > > ------------------ > > > Check out the vibrant tech community on one of the world's most > > > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > > > _______________________________________________ > > > enlightenment-devel mailing list > > > enlightenment-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > ------------------------------------------------------------ > > ------------------ > > Check out the vibrant tech community on one of the world's most > > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > > _______________________________________________ > > enlightenment-devel mailing list > > enlightenment-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > > > > > > -- > Michaël Bouchaud > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
