Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

And now it comes to me the idea to add some settings for this timer
(sorry mike).
If this timer is called, we could display a message to the user.
A dialog to warn him the pulseaudio is slow, and let it choose
to increase the timer.

2017-02-24 19:21 GMT+01:00 michael bouchaud :

> No, current is the current value setted. Don't need to add a field for
> that.
> Instead to only be changed by pulseaudio the field is updated by a set,
> like you do but without duplicating it.
> And a timer is launched for the use case we have no update from pulseaudio
> and sync the volume level.
> No bouncing and no wrong value (or for a short couple of times).
>
> All case are covered and I don't forget where pulse stay at 40%.
>
> 2017-02-24 18:36 GMT+01:00 :
>
>> I dont see why...
>>
>> Lets keep the sample 20% -> 40% -> 60%
>>
>> then there are 2 calls,
>>
>> first:  emix_sink_volume_set(sink, vol) where volume is set to 40%
>> second: emix_sink_volume_set(sink, vol) where volume is set to 60%
>>
>> So at each time the set_value points to the value that got set last
>> time. And we are always jumping to the correct value back when the drag
>> ends...
>>
>> And "if pulse stays at 40%", then my scenario wont hit ... but i think
>> we should not search for a scenario where it works :P
>>
>> On Fri, Feb 24, 2017 at 06:25:07PM +0100, michael bouchaud wrote:
>> > Sorry but with your solution the comportment will be the same...
>> > Or you will save and display wrong value
>> >
>> > 2017-02-24 18:23 GMT+01:00 michael bouchaud > >:
>> >
>> > > Ok and if pulse stay at 40% ?
>> > >
>> > > 2017-02-24 17:15 GMT+01:00 :
>> > >
>> > >> On Fri, Feb 24, 2017 at 01:42:00PM +0100, michael bouchaud wrote:
>> > >> > 2017-02-24 13:31 GMT+01:00 michael bouchaud <
>> michael.bouch...@gmail.com
>> > >> >:
>> > >> >
>> > >> > > 2017-02-23 22:13 GMT+01:00 Simon Lees :
>> > >> > >
>> > >> > >>
>> > >> > >>
>> > >> > >> On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
>> > >> > >> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud
>> wrote:
>> > >> > >> >> 2017-02-23 14:11 GMT+01:00 :
>> > >> > >> >> The "current" is the current value of pulseaudio, and if
>> > >> pulseaudio
>> > >> > >> change
>> > >> > >> >> this
>> > >> > >> >> value we will got an event who update the value and the
>> slider.
>> > >> > >> >
>> > >> > >> >> The user don't complain about bouncing slider, but because
>> he got
>> > >> wrong
>> > >> > >> >> value displayed by the slider.
>> > >> > >> >
>> > >> > >> > The video obviously shows how the slider bounces back and
>> forth ...
>> > >> > >> > so the user complained about the bouncing slider. Just read
>> the
>> > >> commit
>> > >> > >> > message you reverted, and you get to the video where you see
>> the
>> > >> bug.
>> > >> > >> >
>> > >> > >>
>> > >> > >> While in most cases i'd agree that showing the actual volume is
>> > >> better,
>> > >> > >> i've experienced the bouncing around and it basically makes the
>> > >> slider
>> > >> > >> unusable so unfortunately in this case I don't think we can
>> show the
>> > >> > >> actual volume, pulse will catch up soon enough.
>> > >> > >>
>> > >> > >>
>> > >> > > I understand boucing slider could be annoying for users,  we
>> need to
>> > >> take
>> > >> > > this into account, I agreed. Marcel I don't think adding a field
>> is
>> > >> the
>> > >> > > best way to resolve
>> > >> > > this. If we do like that we need to add a field for sinks,
>> > >> sink_inputs and
>> > >> > > main volume.
>> > >> > > And now the problem go to e_client_volume too, so we need to do
>> the
>> > >> same
>> > >> > > with these
>> > >> > > sliders (I really dislike it). This only grow the memory usage
>> in my
>> > >> point
>> > >> > > of view without
>> > >> > > really fixing it. We just moving the problem, as I say before, in
>> > >> other
>> > >> > > place (here higher
>> > >> > > level).
>> > >> > >
>> > >> >
>> > >> > Ok I see your commit you don't move the problem in higher level,
>> but
>> > >> memory
>> > >> > still grow.
>> > >>
>> > >> I dont think thats too bad. Thats a few bytes per sink and channel.
>> > >> The reason i dont like the timeout mechanic is that pulse could
>> update
>> > >> to a "old" value.
>> > >>
>> > >> Lets say you rise from 20% to 40% to 60%. Now you wait for pulse to
>> > >> update. Pulse first updates to 40 (you display it) and then pulse
>> > >> updates to 60%. And you have a jumping slider again.
>> > >>
>> > >> I think that saving our own volume somewhere is here the easiest
>> > >> solution, with the smallest number of corner cases.
>> > >>
>> > >> >
>> > >> >
>> > >> > >
>> > >> > > Why not adding a timeout mechanics when a volume set is done. We
>> set
>> > >> the
>> > >> > > volume
>> > >> > > and update current volume field. If pulse don't change it in a
>> delay
>> > >> of

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

No, current is the current value setted. Don't need to add a field for that.
Instead to only be changed by pulseaudio the field is updated by a set,
like you do but without duplicating it.
And a timer is launched for the use case we have no update from pulseaudio
and sync the volume level.
No bouncing and no wrong value (or for a short couple of times).

All case are covered and I don't forget where pulse stay at 40%.

2017-02-24 18:36 GMT+01:00 :

> I dont see why...
>
> Lets keep the sample 20% -> 40% -> 60%
>
> then there are 2 calls,
>
> first:  emix_sink_volume_set(sink, vol) where volume is set to 40%
> second: emix_sink_volume_set(sink, vol) where volume is set to 60%
>
> So at each time the set_value points to the value that got set last
> time. And we are always jumping to the correct value back when the drag
> ends...
>
> And "if pulse stays at 40%", then my scenario wont hit ... but i think
> we should not search for a scenario where it works :P
>
> On Fri, Feb 24, 2017 at 06:25:07PM +0100, michael bouchaud wrote:
> > Sorry but with your solution the comportment will be the same...
> > Or you will save and display wrong value
> >
> > 2017-02-24 18:23 GMT+01:00 michael bouchaud  >:
> >
> > > Ok and if pulse stay at 40% ?
> > >
> > > 2017-02-24 17:15 GMT+01:00 :
> > >
> > >> On Fri, Feb 24, 2017 at 01:42:00PM +0100, michael bouchaud wrote:
> > >> > 2017-02-24 13:31 GMT+01:00 michael bouchaud <
> michael.bouch...@gmail.com
> > >> >:
> > >> >
> > >> > > 2017-02-23 22:13 GMT+01:00 Simon Lees :
> > >> > >
> > >> > >>
> > >> > >>
> > >> > >> On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
> > >> > >> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud
> wrote:
> > >> > >> >> 2017-02-23 14:11 GMT+01:00 :
> > >> > >> >> The "current" is the current value of pulseaudio, and if
> > >> pulseaudio
> > >> > >> change
> > >> > >> >> this
> > >> > >> >> value we will got an event who update the value and the
> slider.
> > >> > >> >
> > >> > >> >> The user don't complain about bouncing slider, but because he
> got
> > >> wrong
> > >> > >> >> value displayed by the slider.
> > >> > >> >
> > >> > >> > The video obviously shows how the slider bounces back and
> forth ...
> > >> > >> > so the user complained about the bouncing slider. Just read the
> > >> commit
> > >> > >> > message you reverted, and you get to the video where you see
> the
> > >> bug.
> > >> > >> >
> > >> > >>
> > >> > >> While in most cases i'd agree that showing the actual volume is
> > >> better,
> > >> > >> i've experienced the bouncing around and it basically makes the
> > >> slider
> > >> > >> unusable so unfortunately in this case I don't think we can show
> the
> > >> > >> actual volume, pulse will catch up soon enough.
> > >> > >>
> > >> > >>
> > >> > > I understand boucing slider could be annoying for users,  we need
> to
> > >> take
> > >> > > this into account, I agreed. Marcel I don't think adding a field
> is
> > >> the
> > >> > > best way to resolve
> > >> > > this. If we do like that we need to add a field for sinks,
> > >> sink_inputs and
> > >> > > main volume.
> > >> > > And now the problem go to e_client_volume too, so we need to do
> the
> > >> same
> > >> > > with these
> > >> > > sliders (I really dislike it). This only grow the memory usage in
> my
> > >> point
> > >> > > of view without
> > >> > > really fixing it. We just moving the problem, as I say before, in
> > >> other
> > >> > > place (here higher
> > >> > > level).
> > >> > >
> > >> >
> > >> > Ok I see your commit you don't move the problem in higher level, but
> > >> memory
> > >> > still grow.
> > >>
> > >> I dont think thats too bad. Thats a few bytes per sink and channel.
> > >> The reason i dont like the timeout mechanic is that pulse could update
> > >> to a "old" value.
> > >>
> > >> Lets say you rise from 20% to 40% to 60%. Now you wait for pulse to
> > >> update. Pulse first updates to 40 (you display it) and then pulse
> > >> updates to 60%. And you have a jumping slider again.
> > >>
> > >> I think that saving our own volume somewhere is here the easiest
> > >> solution, with the smallest number of corner cases.
> > >>
> > >> >
> > >> >
> > >> > >
> > >> > > Why not adding a timeout mechanics when a volume set is done. We
> set
> > >> the
> > >> > > volume
> > >> > > and update current volume field. If pulse don't change it in a
> delay
> > >> of
> > >> > > time (maybe 1 s),
> > >> > > we go to read the current volume.
> > >> > > If the volume isn't changed current field differ from the readed
> > >> value, so
> > >> > > we could send
> > >> > > the volume change event.
> > >> > >
> > >> > > This fix all places without more memory allocated. What do you
> think ?
> > >> > >
> > >> > >
> > >> > >> --
> > >> > >>
> > >> > >> Simon Lees (Simotek)
> http://simotek.net
> > >> > >>
> > >> > >> 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

I dont see why...

Lets keep the sample 20% -> 40% -> 60%

then there are 2 calls, 

first:  emix_sink_volume_set(sink, vol) where volume is set to 40%
second: emix_sink_volume_set(sink, vol) where volume is set to 60%

So at each time the set_value points to the value that got set last
time. And we are always jumping to the correct value back when the drag
ends...

And "if pulse stays at 40%", then my scenario wont hit ... but i think
we should not search for a scenario where it works :P

On Fri, Feb 24, 2017 at 06:25:07PM +0100, michael bouchaud wrote:
> Sorry but with your solution the comportment will be the same...
> Or you will save and display wrong value
> 
> 2017-02-24 18:23 GMT+01:00 michael bouchaud :
> 
> > Ok and if pulse stay at 40% ?
> >
> > 2017-02-24 17:15 GMT+01:00 :
> >
> >> On Fri, Feb 24, 2017 at 01:42:00PM +0100, michael bouchaud wrote:
> >> > 2017-02-24 13:31 GMT+01:00 michael bouchaud  >> >:
> >> >
> >> > > 2017-02-23 22:13 GMT+01:00 Simon Lees :
> >> > >
> >> > >>
> >> > >>
> >> > >> On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
> >> > >> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
> >> > >> >> 2017-02-23 14:11 GMT+01:00 :
> >> > >> >> The "current" is the current value of pulseaudio, and if
> >> pulseaudio
> >> > >> change
> >> > >> >> this
> >> > >> >> value we will got an event who update the value and the slider.
> >> > >> >
> >> > >> >> The user don't complain about bouncing slider, but because he got
> >> wrong
> >> > >> >> value displayed by the slider.
> >> > >> >
> >> > >> > The video obviously shows how the slider bounces back and forth ...
> >> > >> > so the user complained about the bouncing slider. Just read the
> >> commit
> >> > >> > message you reverted, and you get to the video where you see the
> >> bug.
> >> > >> >
> >> > >>
> >> > >> While in most cases i'd agree that showing the actual volume is
> >> better,
> >> > >> i've experienced the bouncing around and it basically makes the
> >> slider
> >> > >> unusable so unfortunately in this case I don't think we can show the
> >> > >> actual volume, pulse will catch up soon enough.
> >> > >>
> >> > >>
> >> > > I understand boucing slider could be annoying for users,  we need to
> >> take
> >> > > this into account, I agreed. Marcel I don't think adding a field is
> >> the
> >> > > best way to resolve
> >> > > this. If we do like that we need to add a field for sinks,
> >> sink_inputs and
> >> > > main volume.
> >> > > And now the problem go to e_client_volume too, so we need to do the
> >> same
> >> > > with these
> >> > > sliders (I really dislike it). This only grow the memory usage in my
> >> point
> >> > > of view without
> >> > > really fixing it. We just moving the problem, as I say before, in
> >> other
> >> > > place (here higher
> >> > > level).
> >> > >
> >> >
> >> > Ok I see your commit you don't move the problem in higher level, but
> >> memory
> >> > still grow.
> >>
> >> I dont think thats too bad. Thats a few bytes per sink and channel.
> >> The reason i dont like the timeout mechanic is that pulse could update
> >> to a "old" value.
> >>
> >> Lets say you rise from 20% to 40% to 60%. Now you wait for pulse to
> >> update. Pulse first updates to 40 (you display it) and then pulse
> >> updates to 60%. And you have a jumping slider again.
> >>
> >> I think that saving our own volume somewhere is here the easiest
> >> solution, with the smallest number of corner cases.
> >>
> >> >
> >> >
> >> > >
> >> > > Why not adding a timeout mechanics when a volume set is done. We set
> >> the
> >> > > volume
> >> > > and update current volume field. If pulse don't change it in a delay
> >> of
> >> > > time (maybe 1 s),
> >> > > we go to read the current volume.
> >> > > If the volume isn't changed current field differ from the readed
> >> value, so
> >> > > we could send
> >> > > the volume change event.
> >> > >
> >> > > This fix all places without more memory allocated. What do you think ?
> >> > >
> >> > >
> >> > >> --
> >> > >>
> >> > >> Simon Lees (Simotek)http://simotek.net
> >> > >>
> >> > >> Emergency Update Team   keybase.io/simotek
> >> > >> SUSE Linux   Adelaide Australia, UTC+10:30
> >> > >> GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B
> >> > >>
> >> > >>
> >> > >> 
> >> > >> --
> >> > >> 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
> >> > >>
> >> > 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

Sorry but with your solution the comportment will be the same...
Or you will save and display wrong value

2017-02-24 18:23 GMT+01:00 michael bouchaud :

> Ok and if pulse stay at 40% ?
>
> 2017-02-24 17:15 GMT+01:00 :
>
>> On Fri, Feb 24, 2017 at 01:42:00PM +0100, michael bouchaud wrote:
>> > 2017-02-24 13:31 GMT+01:00 michael bouchaud > >:
>> >
>> > > 2017-02-23 22:13 GMT+01:00 Simon Lees :
>> > >
>> > >>
>> > >>
>> > >> On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
>> > >> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
>> > >> >> 2017-02-23 14:11 GMT+01:00 :
>> > >> >> The "current" is the current value of pulseaudio, and if
>> pulseaudio
>> > >> change
>> > >> >> this
>> > >> >> value we will got an event who update the value and the slider.
>> > >> >
>> > >> >> The user don't complain about bouncing slider, but because he got
>> wrong
>> > >> >> value displayed by the slider.
>> > >> >
>> > >> > The video obviously shows how the slider bounces back and forth ...
>> > >> > so the user complained about the bouncing slider. Just read the
>> commit
>> > >> > message you reverted, and you get to the video where you see the
>> bug.
>> > >> >
>> > >>
>> > >> While in most cases i'd agree that showing the actual volume is
>> better,
>> > >> i've experienced the bouncing around and it basically makes the
>> slider
>> > >> unusable so unfortunately in this case I don't think we can show the
>> > >> actual volume, pulse will catch up soon enough.
>> > >>
>> > >>
>> > > I understand boucing slider could be annoying for users,  we need to
>> take
>> > > this into account, I agreed. Marcel I don't think adding a field is
>> the
>> > > best way to resolve
>> > > this. If we do like that we need to add a field for sinks,
>> sink_inputs and
>> > > main volume.
>> > > And now the problem go to e_client_volume too, so we need to do the
>> same
>> > > with these
>> > > sliders (I really dislike it). This only grow the memory usage in my
>> point
>> > > of view without
>> > > really fixing it. We just moving the problem, as I say before, in
>> other
>> > > place (here higher
>> > > level).
>> > >
>> >
>> > Ok I see your commit you don't move the problem in higher level, but
>> memory
>> > still grow.
>>
>> I dont think thats too bad. Thats a few bytes per sink and channel.
>> The reason i dont like the timeout mechanic is that pulse could update
>> to a "old" value.
>>
>> Lets say you rise from 20% to 40% to 60%. Now you wait for pulse to
>> update. Pulse first updates to 40 (you display it) and then pulse
>> updates to 60%. And you have a jumping slider again.
>>
>> I think that saving our own volume somewhere is here the easiest
>> solution, with the smallest number of corner cases.
>>
>> >
>> >
>> > >
>> > > Why not adding a timeout mechanics when a volume set is done. We set
>> the
>> > > volume
>> > > and update current volume field. If pulse don't change it in a delay
>> of
>> > > time (maybe 1 s),
>> > > we go to read the current volume.
>> > > If the volume isn't changed current field differ from the readed
>> value, so
>> > > we could send
>> > > the volume change event.
>> > >
>> > > This fix all places without more memory allocated. What do you think ?
>> > >
>> > >
>> > >> --
>> > >>
>> > >> Simon Lees (Simotek)http://simotek.net
>> > >>
>> > >> Emergency Update Team   keybase.io/simotek
>> > >> SUSE Linux   Adelaide Australia, UTC+10:30
>> > >> GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B
>> > >>
>> > >>
>> > >> 
>> > >> --
>> > >> 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
>> > >
>> >
>> >
>> >
>> > --
>> > 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
>> 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

Ok and if pulse stay at 40% ?

2017-02-24 17:15 GMT+01:00 :

> On Fri, Feb 24, 2017 at 01:42:00PM +0100, michael bouchaud wrote:
> > 2017-02-24 13:31 GMT+01:00 michael bouchaud  >:
> >
> > > 2017-02-23 22:13 GMT+01:00 Simon Lees :
> > >
> > >>
> > >>
> > >> On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
> > >> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
> > >> >> 2017-02-23 14:11 GMT+01:00 :
> > >> >> The "current" is the current value of pulseaudio, and if pulseaudio
> > >> change
> > >> >> this
> > >> >> value we will got an event who update the value and the slider.
> > >> >
> > >> >> The user don't complain about bouncing slider, but because he got
> wrong
> > >> >> value displayed by the slider.
> > >> >
> > >> > The video obviously shows how the slider bounces back and forth ...
> > >> > so the user complained about the bouncing slider. Just read the
> commit
> > >> > message you reverted, and you get to the video where you see the
> bug.
> > >> >
> > >>
> > >> While in most cases i'd agree that showing the actual volume is
> better,
> > >> i've experienced the bouncing around and it basically makes the slider
> > >> unusable so unfortunately in this case I don't think we can show the
> > >> actual volume, pulse will catch up soon enough.
> > >>
> > >>
> > > I understand boucing slider could be annoying for users,  we need to
> take
> > > this into account, I agreed. Marcel I don't think adding a field is the
> > > best way to resolve
> > > this. If we do like that we need to add a field for sinks, sink_inputs
> and
> > > main volume.
> > > And now the problem go to e_client_volume too, so we need to do the
> same
> > > with these
> > > sliders (I really dislike it). This only grow the memory usage in my
> point
> > > of view without
> > > really fixing it. We just moving the problem, as I say before, in other
> > > place (here higher
> > > level).
> > >
> >
> > Ok I see your commit you don't move the problem in higher level, but
> memory
> > still grow.
>
> I dont think thats too bad. Thats a few bytes per sink and channel.
> The reason i dont like the timeout mechanic is that pulse could update
> to a "old" value.
>
> Lets say you rise from 20% to 40% to 60%. Now you wait for pulse to
> update. Pulse first updates to 40 (you display it) and then pulse
> updates to 60%. And you have a jumping slider again.
>
> I think that saving our own volume somewhere is here the easiest
> solution, with the smallest number of corner cases.
>
> >
> >
> > >
> > > Why not adding a timeout mechanics when a volume set is done. We set
> the
> > > volume
> > > and update current volume field. If pulse don't change it in a delay of
> > > time (maybe 1 s),
> > > we go to read the current volume.
> > > If the volume isn't changed current field differ from the readed
> value, so
> > > we could send
> > > the volume change event.
> > >
> > > This fix all places without more memory allocated. What do you think ?
> > >
> > >
> > >> --
> > >>
> > >> Simon Lees (Simotek)http://simotek.net
> > >>
> > >> Emergency Update Team   keybase.io/simotek
> > >> SUSE Linux   Adelaide Australia, UTC+10:30
> > >> GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B
> > >>
> > >>
> > >> 
> > >> --
> > >> 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
> > >
> >
> >
> >
> > --
> > 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
>



-- 
Michaël Bouchaud
--
Check out the vibrant tech community on one of the world's most

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

On Fri, Feb 24, 2017 at 01:42:00PM +0100, michael bouchaud wrote:
> 2017-02-24 13:31 GMT+01:00 michael bouchaud :
> 
> > 2017-02-23 22:13 GMT+01:00 Simon Lees :
> >
> >>
> >>
> >> On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
> >> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
> >> >> 2017-02-23 14:11 GMT+01:00 :
> >> >> The "current" is the current value of pulseaudio, and if pulseaudio
> >> change
> >> >> this
> >> >> value we will got an event who update the value and the slider.
> >> >
> >> >> The user don't complain about bouncing slider, but because he got wrong
> >> >> value displayed by the slider.
> >> >
> >> > The video obviously shows how the slider bounces back and forth ...
> >> > so the user complained about the bouncing slider. Just read the commit
> >> > message you reverted, and you get to the video where you see the bug.
> >> >
> >>
> >> While in most cases i'd agree that showing the actual volume is better,
> >> i've experienced the bouncing around and it basically makes the slider
> >> unusable so unfortunately in this case I don't think we can show the
> >> actual volume, pulse will catch up soon enough.
> >>
> >>
> > I understand boucing slider could be annoying for users,  we need to take
> > this into account, I agreed. Marcel I don't think adding a field is the
> > best way to resolve
> > this. If we do like that we need to add a field for sinks, sink_inputs and
> > main volume.
> > And now the problem go to e_client_volume too, so we need to do the same
> > with these
> > sliders (I really dislike it). This only grow the memory usage in my point
> > of view without
> > really fixing it. We just moving the problem, as I say before, in other
> > place (here higher
> > level).
> >
> 
> Ok I see your commit you don't move the problem in higher level, but memory
> still grow.

I dont think thats too bad. Thats a few bytes per sink and channel.
The reason i dont like the timeout mechanic is that pulse could update
to a "old" value.

Lets say you rise from 20% to 40% to 60%. Now you wait for pulse to
update. Pulse first updates to 40 (you display it) and then pulse
updates to 60%. And you have a jumping slider again.

I think that saving our own volume somewhere is here the easiest
solution, with the smallest number of corner cases.

> 
> 
> >
> > Why not adding a timeout mechanics when a volume set is done. We set the
> > volume
> > and update current volume field. If pulse don't change it in a delay of
> > time (maybe 1 s),
> > we go to read the current volume.
> > If the volume isn't changed current field differ from the readed value, so
> > we could send
> > the volume change event.
> >
> > This fix all places without more memory allocated. What do you think ?
> >
> >
> >> --
> >>
> >> Simon Lees (Simotek)http://simotek.net
> >>
> >> Emergency Update Team   keybase.io/simotek
> >> SUSE Linux   Adelaide Australia, UTC+10:30
> >> GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B
> >>
> >>
> >> 
> >> --
> >> 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
> >
> 
> 
> 
> -- 
> 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

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-24 13:31 GMT+01:00 michael bouchaud :

> 2017-02-23 22:13 GMT+01:00 Simon Lees :
>
>>
>>
>> On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
>> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
>> >> 2017-02-23 14:11 GMT+01:00 :
>> >> The "current" is the current value of pulseaudio, and if pulseaudio
>> change
>> >> this
>> >> value we will got an event who update the value and the slider.
>> >
>> >> The user don't complain about bouncing slider, but because he got wrong
>> >> value displayed by the slider.
>> >
>> > The video obviously shows how the slider bounces back and forth ...
>> > so the user complained about the bouncing slider. Just read the commit
>> > message you reverted, and you get to the video where you see the bug.
>> >
>>
>> While in most cases i'd agree that showing the actual volume is better,
>> i've experienced the bouncing around and it basically makes the slider
>> unusable so unfortunately in this case I don't think we can show the
>> actual volume, pulse will catch up soon enough.
>>
>>
> I understand boucing slider could be annoying for users,  we need to take
> this into account, I agreed. Marcel I don't think adding a field is the
> best way to resolve
> this. If we do like that we need to add a field for sinks, sink_inputs and
> main volume.
> And now the problem go to e_client_volume too, so we need to do the same
> with these
> sliders (I really dislike it). This only grow the memory usage in my point
> of view without
> really fixing it. We just moving the problem, as I say before, in other
> place (here higher
> level).
>

Ok I see your commit you don't move the problem in higher level, but memory
still grow.


>
> Why not adding a timeout mechanics when a volume set is done. We set the
> volume
> and update current volume field. If pulse don't change it in a delay of
> time (maybe 1 s),
> we go to read the current volume.
> If the volume isn't changed current field differ from the readed value, so
> we could send
> the volume change event.
>
> This fix all places without more memory allocated. What do you think ?
>
>
>> --
>>
>> Simon Lees (Simotek)http://simotek.net
>>
>> Emergency Update Team   keybase.io/simotek
>> SUSE Linux   Adelaide Australia, UTC+10:30
>> GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B
>>
>>
>> 
>> --
>> 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
>



-- 
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

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 22:13 GMT+01:00 Simon Lees :

>
>
> On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
> >> 2017-02-23 14:11 GMT+01:00 :
> >> The "current" is the current value of pulseaudio, and if pulseaudio
> change
> >> this
> >> value we will got an event who update the value and the slider.
> >
> >> The user don't complain about bouncing slider, but because he got wrong
> >> value displayed by the slider.
> >
> > The video obviously shows how the slider bounces back and forth ...
> > so the user complained about the bouncing slider. Just read the commit
> > message you reverted, and you get to the video where you see the bug.
> >
>
> While in most cases i'd agree that showing the actual volume is better,
> i've experienced the bouncing around and it basically makes the slider
> unusable so unfortunately in this case I don't think we can show the
> actual volume, pulse will catch up soon enough.
>
>
I understand boucing slider could be annoying for users,  we need to take
this into account, I agreed. Marcel I don't think adding a field is the
best way to resolve
this. If we do like that we need to add a field for sinks, sink_inputs and
main volume.
And now the problem go to e_client_volume too, so we need to do the same
with these
sliders (I really dislike it). This only grow the memory usage in my point
of view without
really fixing it. We just moving the problem, as I say before, in other
place (here higher
level).

Why not adding a timeout mechanics when a volume set is done. We set the
volume
and update current volume field. If pulse don't change it in a delay of
time (maybe 1 s),
we go to read the current volume.
If the volume isn't changed current field differ from the readed value, so
we could send
the volume change event.

This fix all places without more memory allocated. What do you think ?


> --
>
> Simon Lees (Simotek)http://simotek.net
>
> Emergency Update Team   keybase.io/simotek
> SUSE Linux   Adelaide Australia, UTC+10:30
> GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B
>
>
> 
> --
> 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

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[Simon Lees]

On 02/24/2017 01:03 AM, marcel-hollerb...@t-online.de wrote:
> On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
>> 2017-02-23 14:11 GMT+01:00 :
>> The "current" is the current value of pulseaudio, and if pulseaudio change
>> this
>> value we will got an event who update the value and the slider.
> 
>> The user don't complain about bouncing slider, but because he got wrong
>> value displayed by the slider.
> 
> The video obviously shows how the slider bounces back and forth ...
> so the user complained about the bouncing slider. Just read the commit
> message you reverted, and you get to the video where you see the bug.
> 

While in most cases i'd agree that showing the actual volume is better,
i've experienced the bouncing around and it basically makes the slider
unusable so unfortunately in this case I don't think we can show the
actual volume, pulse will catch up soon enough.

-- 

Simon Lees (Simotek)http://simotek.net

Emergency Update Team   keybase.io/simotek
SUSE Linux   Adelaide Australia, UTC+10:30
GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B



signature.asc
Description: OpenPGP digital signature
--
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

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

On Thu, Feb 23, 2017 at 04:05:17PM +0100, michael bouchaud wrote:
> 2017-02-23 15:33 GMT+01:00 :
> 
> > On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
> > > 2017-02-23 14:11 GMT+01:00 :
> > >
> > > > > [...]
> > > > > The current volume is the current volume not old. Pulse audio notify
> > us
> > > > > when changing the volume and this field is updated at this time
> > > > > as you said.
> > > >
> > > > The "current" volume of the sink is the last reported volume we got
> > from
> > > > pulseaudio, and as we say in the video, there are cases where we are
> > > > having delays. So better NOT setting it when the drag ends, and just
> > > > synchronize the UI representation when we are setting the volume to
> > > > emix.
> > > >
> > > >
> > > Ho common, I don't remove any feature. I just want to improve user
> > > experience.
> > > I think having wrong value displayed on the slider is a bug. If I have
> > > hacked
> > > your commit it's because I got some bugs. I don't care about code who
> > works
> > > great.
> >
> > And with your revert you introduced another commit ;) we can now
> > continue to revert the revert of reverts, or we are just going to think
> > about what the issue in the past was and how it could get fixed, or at
> > least, start to communicate :)
> >
> 
> 
> Start to communicate ? stop please don't say that when you remove this stuff
> you never telling me you will do that. You have choosen on your own and you
> know I'm working on it.

Yep, i did not tell you in advance about the commit thats true. The only
difference is my commit does have a commit message and a discription
what it does fix.

Anyway can you checkout f87331badc5d910a8110d411ec1ef2009f9f668e ?
This fixes for me the case where the spinner is jumping arround.

> 
> 
> > > The "current" is the current value of pulseaudio, and if pulseaudio
> > change
> > > this
> > > value we will got an event who update the value and the slider.
> >
> > > The user don't complain about bouncing slider, but because he got wrong
> > > value displayed by the slider.
> >
> > The video obviously shows how the slider bounces back and forth ...
> > so the user complained about the bouncing slider. Just read the commit
> > message you reverted, and you get to the video where you see the bug.
> >
> 
> I see the video and talked with morlenxus.
> 
> 
> >
> 
> > You never talked to anyone when you choose to not set the slider after a
> > > drag stop. I'm not the author of this code, raster is. So to sum up why
> > > raster use
> > > drag stop event and not doing it as you say.
> > >
> > > First :
> > >  Calling elm_slider_value_set into the changed callback when user drag
> > it,
> > > won't work. It works only when clicked. If dragged, the value stay on
> > what
> > > user
> > > set not the fixed one.
> > >
> > > Second :
> > >   If we set the slider to a theorical good value not the current one,
> > and we
> > > don't get any events from pulse. This value stay only theorical not the
> > > current one.
> > >
> > > So if you think my patch is only to remove your stuff, you're wrong.
> > > My patch FIX an issue you introduced.
> > >
> >
> > Well, before than blidly reverting my commit without even telling why
> > (btw. thats the second time ;)) you might want to contanct my and ask my
> > to take again a look on the fix. For example save the volume that gets
> > set to a sink in a seperated field, and just use that to clamp when the
> > drag is done, so we do not depend on a volume that is not updated by
> > pulseaudio.
> >
> >
> The second time on the same part of code. I explained you month ago
> why this is needed, and reexplain here. You still don't want to understand
> it.
> I have tell you,I will make some test with morlenxus. I do, but he never
> came back to tell me if my branch work. I'm not against you and I try my
> best
> to explain why this is needed and why it works as intended.
> Adding more mechanics in my point of view, isn't a fix but just report the
> problem in other place.
> 
> > >
> > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > 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
> > > > > > > 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 15:33 GMT+01:00 :

> On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
> > 2017-02-23 14:11 GMT+01:00 :
> >
> > > > [...]
> > > > The current volume is the current volume not old. Pulse audio notify
> us
> > > > when changing the volume and this field is updated at this time
> > > > as you said.
> > >
> > > The "current" volume of the sink is the last reported volume we got
> from
> > > pulseaudio, and as we say in the video, there are cases where we are
> > > having delays. So better NOT setting it when the drag ends, and just
> > > synchronize the UI representation when we are setting the volume to
> > > emix.
> > >
> > >
> > Ho common, I don't remove any feature. I just want to improve user
> > experience.
> > I think having wrong value displayed on the slider is a bug. If I have
> > hacked
> > your commit it's because I got some bugs. I don't care about code who
> works
> > great.
>
> And with your revert you introduced another commit ;) we can now
> continue to revert the revert of reverts, or we are just going to think
> about what the issue in the past was and how it could get fixed, or at
> least, start to communicate :)
>


Start to communicate ? stop please don't say that when you remove this stuff
you never telling me you will do that. You have choosen on your own and you
know I'm working on it.


> > The "current" is the current value of pulseaudio, and if pulseaudio
> change
> > this
> > value we will got an event who update the value and the slider.
>
> > The user don't complain about bouncing slider, but because he got wrong
> > value displayed by the slider.
>
> The video obviously shows how the slider bounces back and forth ...
> so the user complained about the bouncing slider. Just read the commit
> message you reverted, and you get to the video where you see the bug.
>

I see the video and talked with morlenxus.


>

> You never talked to anyone when you choose to not set the slider after a
> > drag stop. I'm not the author of this code, raster is. So to sum up why
> > raster use
> > drag stop event and not doing it as you say.
> >
> > First :
> >  Calling elm_slider_value_set into the changed callback when user drag
> it,
> > won't work. It works only when clicked. If dragged, the value stay on
> what
> > user
> > set not the fixed one.
> >
> > Second :
> >   If we set the slider to a theorical good value not the current one,
> and we
> > don't get any events from pulse. This value stay only theorical not the
> > current one.
> >
> > So if you think my patch is only to remove your stuff, you're wrong.
> > My patch FIX an issue you introduced.
> >
>
> Well, before than blidly reverting my commit without even telling why
> (btw. thats the second time ;)) you might want to contanct my and ask my
> to take again a look on the fix. For example save the volume that gets
> set to a sink in a seperated field, and just use that to clamp when the
> drag is done, so we do not depend on a volume that is not updated by
> pulseaudio.
>
>
The second time on the same part of code. I explained you month ago
why this is needed, and reexplain here. You still don't want to understand
it.
I have tell you,I will make some test with morlenxus. I do, but he never
came back to tell me if my branch work. I'm not against you and I try my
best
to explain why this is needed and why it works as intended.
Adding more mechanics in my point of view, isn't a fix but just report the
problem in other place.

> >
> > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > 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 :)
> > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

On Thu, Feb 23, 2017 at 03:13:18PM +0100, michael bouchaud wrote:
> 2017-02-23 14:11 GMT+01:00 :
> 
> > > [...]
> > > The current volume is the current volume not old. Pulse audio notify us
> > > when changing the volume and this field is updated at this time
> > > as you said.
> >
> > The "current" volume of the sink is the last reported volume we got from
> > pulseaudio, and as we say in the video, there are cases where we are
> > having delays. So better NOT setting it when the drag ends, and just
> > synchronize the UI representation when we are setting the volume to
> > emix.
> >
> >
> Ho common, I don't remove any feature. I just want to improve user
> experience.
> I think having wrong value displayed on the slider is a bug. If I have
> hacked
> your commit it's because I got some bugs. I don't care about code who works
> great.

And with your revert you introduced another commit ;) we can now
continue to revert the revert of reverts, or we are just going to think
about what the issue in the past was and how it could get fixed, or at
least, start to communicate :)

> The "current" is the current value of pulseaudio, and if pulseaudio change
> this
> value we will got an event who update the value and the slider.

> The user don't complain about bouncing slider, but because he got wrong
> value displayed by the slider.

The video obviously shows how the slider bounces back and forth ...
so the user complained about the bouncing slider. Just read the commit
message you reverted, and you get to the video where you see the bug.

> You never talked to anyone when you choose to not set the slider after a
> drag stop. I'm not the author of this code, raster is. So to sum up why
> raster use
> drag stop event and not doing it as you say.
> 
> First :
>  Calling elm_slider_value_set into the changed callback when user drag it,
> won't work. It works only when clicked. If dragged, the value stay on what
> user
> set not the fixed one.
> 
> Second :
>   If we set the slider to a theorical good value not the current one, and we
> don't get any events from pulse. This value stay only theorical not the
> current one.
> 
> So if you think my patch is only to remove your stuff, you're wrong.
> My patch FIX an issue you introduced.
> 

Well, before than blidly reverting my commit without even telling why
(btw. thats the second time ;)) you might want to contanct my and ask my
to take again a look on the fix. For example save the volume that gets
set to a sink in a seperated field, and just use that to clamp when the
drag is done, so we do not depend on a volume that is not updated by
pulseaudio. 

> >
> > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > 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 <
> > > > > > > > > > > > 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""
> > > > > > > > > > > > > >
> > 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 14:11 GMT+01:00 :

> > [...]
> > The current volume is the current volume not old. Pulse audio notify us
> > when changing the volume and this field is updated at this time
> > as you said.
>
> The "current" volume of the sink is the last reported volume we got from
> pulseaudio, and as we say in the video, there are cases where we are
> having delays. So better NOT setting it when the drag ends, and just
> synchronize the UI representation when we are setting the volume to
> emix.
>
>
Ho common, I don't remove any feature. I just want to improve user
experience.
I think having wrong value displayed on the slider is a bug. If I have
hacked
your commit it's because I got some bugs. I don't care about code who works
great.
The "current" is the current value of pulseaudio, and if pulseaudio change
this
value we will got an event who update the value and the slider.
The user don't complain about bouncing slider, but because he got wrong
value displayed by the slider.
You never talked to anyone when you choose to not set the slider after a
drag stop. I'm not the author of this code, raster is. So to sum up why
raster use
drag stop event and not doing it as you say.

First :
 Calling elm_slider_value_set into the changed callback when user drag it,
won't work. It works only when clicked. If dragged, the value stay on what
user
set not the fixed one.

Second :
  If we set the slider to a theorical good value not the current one, and we
don't get any events from pulse. This value stay only theorical not the
current one.

So if you think my patch is only to remove your stuff, you're wrong.
My patch FIX an issue you introduced.

>
> > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > 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 <
> > > > > > > > > > > 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
> fba185798cf75eaeaba4a95d2be25f
> > > > > > > b2fea6ef1a.
> > > > > > > > > > > > >
> > > > > > > > > > > > > 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
> > > > > > > > > > > > > @@ 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

> [...]
> The current volume is the current volume not old. Pulse audio notify us
> when changing the volume and this field is updated at this time
> as you said.

The "current" volume of the sink is the last reported volume we got from
pulseaudio, and as we say in the video, there are cases where we are
having delays. So better NOT setting it when the drag ends, and just
synchronize the UI representation when we are setting the volume to
emix.

> 
> >
> > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > 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 <
> > > > > > > > > > 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 fba185798cf75eaeaba4a95d2be25f
> > > > > > b2fea6ef1a.
> > > > > > > > > > > >
> > > > > > > > > > > > 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);
> > > > > > > > > > > > -   

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

On Thu, Feb 23, 2017 at 01:45:26PM +0100, michael bouchaud wrote:
> 2017-02-23 13:38 GMT+01:00 michael bouchaud :
> 
> > 2017-02-23 13:23 GMT+01:00 :
> >
> >> On Thu, Feb 23, 2017 at 01:18:35PM +0100, michael bouchaud wrote:
> >> > 2017-02-23 13:13 GMT+01:00 :
> >> >
> >> > > On Thu, Feb 23, 2017 at 01:00:10PM +0100, michael bouchaud wrote:
> >> > > > 2017-02-23 12:37 GMT+01:00 :
> >> > > >
> >> > > > > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
> >> > > > > > 2017-02-23 12:01 GMT+01:00 :
> >> > > > > >
> >> > > > > > > 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.
> >> > >
> >> > > It did before commit dbdf411b488fd4d3f37a26d8cb142b25aba784d6. Reread
> >> > > the patch, you are removing there a elm_slider_value_set, this ensured
> >> > > that the value stayed at the value returned by the barrier feature.
> >>
> >
> I remove it because it isn't in a good shape and all other place use VOLSET
> macro.
> So for consistency and reduce code duplication I use emix as other place do

Yeah ... use VOLSET as you like, but you have to keep
elm_slider_volume_set, since thats part of the barrier feature that
brings back the slider to the value where we are clamping at ...
Otherwise a user doesnt even see where his volume is going ... 

Basically your complete argumentation of why my bugfix is wrong is based
on your own commit that removed a part of a feature. Could we just sum
up for the future that we might just talk about something before it gets
pushed?  

> 
> 
> > > >
> >> > > >
> >> > > >
> >> > > > > >
> >> > > > > >
> >> > > > > > >
> >> > > > > > > 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 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 13:49 GMT+01:00 :

> On Thu, Feb 23, 2017 at 01:38:31PM +0100, michael bouchaud wrote:
> > 2017-02-23 13:23 GMT+01:00 :
> >
> > > On Thu, Feb 23, 2017 at 01:18:35PM +0100, michael bouchaud wrote:
> > > > 2017-02-23 13:13 GMT+01:00 :
> > > >
> > > > > On Thu, Feb 23, 2017 at 01:00:10PM +0100, michael bouchaud wrote:
> > > > > > 2017-02-23 12:37 GMT+01:00 :
> > > > > >
> > > > > > > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud
> wrote:
> > > > > > > > 2017-02-23 12:01 GMT+01:00 :
> > > > > > > >
> > > > > > > > > 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.
> > > > >
> > > > > It did before commit dbdf411b488fd4d3f37a26d8cb142b25aba784d6.
> Reread
> > > > > the patch, you are removing there a elm_slider_value_set, this
> ensured
> > > > > that the value stayed at the value returned by the barrier feature.
> > > > >
> > > > > >
> > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > 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 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

On Thu, Feb 23, 2017 at 01:38:31PM +0100, michael bouchaud wrote:
> 2017-02-23 13:23 GMT+01:00 :
> 
> > On Thu, Feb 23, 2017 at 01:18:35PM +0100, michael bouchaud wrote:
> > > 2017-02-23 13:13 GMT+01:00 :
> > >
> > > > On Thu, Feb 23, 2017 at 01:00:10PM +0100, michael bouchaud wrote:
> > > > > 2017-02-23 12:37 GMT+01:00 :
> > > > >
> > > > > > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
> > > > > > > 2017-02-23 12:01 GMT+01:00 :
> > > > > > >
> > > > > > > > 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.
> > > >
> > > > It did before commit dbdf411b488fd4d3f37a26d8cb142b25aba784d6. Reread
> > > > the patch, you are removing there a elm_slider_value_set, this ensured
> > > > that the value stayed at the value returned by the barrier feature.
> > > >
> > > > >
> > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > 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 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 13:38 GMT+01:00 michael bouchaud :

> 2017-02-23 13:23 GMT+01:00 :
>
>> On Thu, Feb 23, 2017 at 01:18:35PM +0100, michael bouchaud wrote:
>> > 2017-02-23 13:13 GMT+01:00 :
>> >
>> > > On Thu, Feb 23, 2017 at 01:00:10PM +0100, michael bouchaud wrote:
>> > > > 2017-02-23 12:37 GMT+01:00 :
>> > > >
>> > > > > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
>> > > > > > 2017-02-23 12:01 GMT+01:00 :
>> > > > > >
>> > > > > > > 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.
>> > >
>> > > It did before commit dbdf411b488fd4d3f37a26d8cb142b25aba784d6. Reread
>> > > the patch, you are removing there a elm_slider_value_set, this ensured
>> > > that the value stayed at the value returned by the barrier feature.
>>
>
I remove it because it isn't in a good shape and all other place use VOLSET
macro.
So for consistency and reduce code duplication I use emix as other place do.


> > >
>> > > >
>> > > >
>> > > > > >
>> > > > > >
>> > > > > > >
>> > > > > > > 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 ...
>> > > 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 13:23 GMT+01:00 :

> On Thu, Feb 23, 2017 at 01:18:35PM +0100, michael bouchaud wrote:
> > 2017-02-23 13:13 GMT+01:00 :
> >
> > > On Thu, Feb 23, 2017 at 01:00:10PM +0100, michael bouchaud wrote:
> > > > 2017-02-23 12:37 GMT+01:00 :
> > > >
> > > > > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
> > > > > > 2017-02-23 12:01 GMT+01:00 :
> > > > > >
> > > > > > > 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.
> > >
> > > It did before commit dbdf411b488fd4d3f37a26d8cb142b25aba784d6. Reread
> > > the patch, you are removing there a elm_slider_value_set, this ensured
> > > that the value stayed at the value returned by the barrier feature.
> > >
> > > >
> > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > 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 :)
> > >
> > > yeah. in e_mod_main.c but since you are comparing emixer with the
> module
> > > you 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

On Thu, Feb 23, 2017 at 01:18:35PM +0100, michael bouchaud wrote:
> 2017-02-23 13:13 GMT+01:00 :
> 
> > On Thu, Feb 23, 2017 at 01:00:10PM +0100, michael bouchaud wrote:
> > > 2017-02-23 12:37 GMT+01:00 :
> > >
> > > > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
> > > > > 2017-02-23 12:01 GMT+01:00 :
> > > > >
> > > > > > 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.
> >
> > It did before commit dbdf411b488fd4d3f37a26d8cb142b25aba784d6. Reread
> > the patch, you are removing there a elm_slider_value_set, this ensured
> > that the value stayed at the value returned by the barrier feature.
> >
> > >
> > >
> > > > >
> > > > >
> > > > > >
> > > > > > 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 :)
> >
> > yeah. in e_mod_main.c but since you are comparing emixer with the module
> > you should also have the code you want in somewhere in emixer.c show me
> > where it is.
> >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > 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 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 13:13 GMT+01:00 :

> On Thu, Feb 23, 2017 at 01:00:10PM +0100, michael bouchaud wrote:
> > 2017-02-23 12:37 GMT+01:00 :
> >
> > > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
> > > > 2017-02-23 12:01 GMT+01:00 :
> > > >
> > > > > 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.
>
> It did before commit dbdf411b488fd4d3f37a26d8cb142b25aba784d6. Reread
> the patch, you are removing there a elm_slider_value_set, this ensured
> that the value stayed at the value returned by the barrier feature.
>
> >
> >
> > > >
> > > >
> > > > >
> > > > > 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 :)
>
> yeah. in e_mod_main.c but since you are comparing emixer with the module
> you should also have the code you want in somewhere in emixer.c show me
> where it is.
>
> >
> > >
> > > >
> > > > >
> > > > > 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 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

On Thu, Feb 23, 2017 at 01:00:10PM +0100, michael bouchaud wrote:
> 2017-02-23 12:37 GMT+01:00 :
> 
> > On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
> > > 2017-02-23 12:01 GMT+01:00 :
> > >
> > > > 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.

It did before commit dbdf411b488fd4d3f37a26d8cb142b25aba784d6. Reread
the patch, you are removing there a elm_slider_value_set, this ensured
that the value stayed at the value returned by the barrier feature. 

> 
> 
> > >
> > >
> > > >
> > > > 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 :)

yeah. in e_mod_main.c but since you are comparing emixer with the module
you should also have the code you want in somewhere in emixer.c show me
where it is.

> 
> >
> > >
> > > >
> > > > 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.
> 
>  

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 12:37 GMT+01:00 :

> On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
> > 2017-02-23 12:01 GMT+01:00 :
> >
> > > 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. 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

On Thu, Feb 23, 2017 at 12:14:05PM +0100, michael bouchaud wrote:
> 2017-02-23 12:01 GMT+01:00 :
> 
> > 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 
> > > > 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 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[michael bouchaud]

2017-02-23 12:01 GMT+01:00 :

> 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 ...


>
> 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 ...


>
> 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.


>
> > 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 
> > > 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 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[marcel-hollerbach]

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.

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. 

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.).

> 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 :
> 
> > bu5hm4n pushed a commit to branch master.
> >
> > http://git.enlightenment.org/core/enlightenment.git/commit/?id=
> > 9745890a37036091d5dec320fecda7ed4c6bdb6c
> >
> > commit 9745890a37036091d5dec320fecda7ed4c6bdb6c
> > Author: Marcel Hollerbach 
> > 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 

Re: [E-devel] [EGIT] [core/enlightenment] master 01/01: Revert "Revert "mixer: do not set back the value from emix once the drag is finished""

[Michaël Bouchaud]

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.
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.
And more important, not getting this commit into our code just hidde us the
bug
and give us no chance to spot it.


2017-02-23 9:09 GMT+01:00 Marcel Hollerbach :

> bu5hm4n pushed a commit to branch master.
>
> http://git.enlightenment.org/core/enlightenment.git/commit/?id=
> 9745890a37036091d5dec320fecda7ed4c6bdb6c
>
> commit 9745890a37036091d5dec320fecda7ed4c6bdb6c
> Author: Marcel Hollerbach 
> 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) 
--
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