2017-02-23 12:37 GMT+01:00 <[email protected]>: > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote: > > 2017-02-23 12:01 GMT+01:00 <[email protected]>: > > > > > 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? >
you have 100 setted and you drag to 105, as you say BARRIER feature won't change the volume. But without this commit the slider stay at 105. > > > > > > > > > > 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. > You remove it ... so :) > > > > > > > > > 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) > yes but the code here don't call any emix or pulse stuff. Just reread the value. /* retrieve data */ EINA_SAFETY_ON_NULL_RETURN(mixer_context->sink_default); Emix_Sink *s = (Emix_Sink *)mixer_context->sink_default; /* Read it */ int val = s->volume.volumes[0]; /* Set it */ elm_slider_value_set(obj, val); Please if I'm wrong, spot me where we call pulse stuff. > > 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? > To be sure we display the correct value setted to pulse (BARRIER feature could not change the volume or set it to a different value). If you want to see why we need this. Open emixer and move the enlightenment module slider in different value. You will see we don't get the same value on both in some case. > > > > > > > > > > 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 < > > > [email protected]>: > > > > > > > > > bu5hm4n pushed a commit to branch master. > > > > > > > > > > http://git.enlightenment.org/core/enlightenment.git/commit/?id= > > > > > 9745890a37036091d5dec320fecda7ed4c6bdb6c > > > > > > > > > > commit 9745890a37036091d5dec320fecda7ed4c6bdb6c > > > > > Author: Marcel Hollerbach <[email protected]> > > > > > 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) <[email protected]> > > > > ------------------------------------------------------------ > > > ------------------ > > > > 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 > > > > [email protected] > > > > 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 > > > [email protected] > > > 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 > > [email protected] > > 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 > [email protected] > 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 [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
