On Sat, 9 Feb 2013 16:34:17 +0530 Arvind R <arvin...@gmail.com> said:
> On Fri, Feb 8, 2013 at 3:18 PM, Carsten Haitzler <ras...@rasterman.com> wrote:
> > On Fri, 1 Feb 2013 22:29:56 +0530 Arvind R <arvin...@gmail.com> said:
> >
> > ummm i'm just testing with slider directly... and i ONLY get change
> > callbacks when the user drags it around, not when code sets the value... i
> > added tests in rev83775. i dont get any printfs for changes unless i drag
> > myself. the +.1 -.1 0 and 1 buttons that explicitly set the slider to these
> > values produce no changed callback...
> >
> > ? fyi - i'm testing svn here...
> I tested by putting a 'printf' in elc_player.c _update position()
> which should be called only when the user drags - but I found it being
> called once awhile. The problem may not lie with the slider, but in
> the dynamics of the movie being played and the call to
> elm_video_play_position_set(). The minimal change needed to avoid the
> effect is to track the video_position in a instance variable and
> change video_position in update_position only if it differs by 0.1
> (sec). Testing with r83733.
well i added tests to elm-slider and its not coming from the slider... :/ well
not in my tests, not when i set the value in the elm slider api and actually
change it to a different value. :( so we have an issue here on the source of
the issue.
your patch does a whole list of things.
1. changes code to only set disabled state to !seekable IF seekable and
disabled state disagree - i am not sure this is useful as disabled_set should
already be checking disabled state of the slider too... ?
2. adds code to check length changed THEN set slider min/max... problem is
_elm_slider_min_max_set() already does this check so i'm not sure if this is
helpful other than adding more code. :( UNLESS we have a precision problem. ie
stored val_max (or min) will be just slightly different to the provided min/max
as they may end up register passed and on x86 fp vals in registers CAN be 80bit
(not 64bit) and thus contain more precision while in a register... - is this
the real core of the problem? if so i dont think your check will help either. :(
3. indeed there is value to "not updating the slider as much" (every frame
decode), but doing it only every 30 frames is going to create problems. imho
what u want is this (kind of similar to your code:
if ((fabs(sd->pos - pos) > 0.5) || (fabs(ecore_loop_time_get() -
sd->last_update_time) > 0.5)
{
sd->last_update_time = ecore_loop_time_get();
if (sd->deferred_update) ecore_timer_del(sd->deferred_update);
sd->deferred_update = NULL;
//...
}
else
{
if (sd->deferred_update) ecore_timer_del(sd->deferred_update);
sd->deferred_update = ecore_timer_add(0.75, _update_anyway, obj);
}
ie - limit updates to every 0.5 sec in between and if frame updates come SLOWLY
- still update every 0.5 sec... and... if we got an update like 0.4 sec after
the last one and then no more because play paused/stopped.. defer an update
anyway so that if we get no more updates then we ultimately update anyway -=
just with a 0.75 sec delay.
this should avoid the "missing updates" corner cases your patch will create :
( i'd of course make these values tunable and stored in the elc_player smart
data struct (sd)... :)
4. setting disabled by default? hmmm...?
5. setting min/max to 0->1 by default. why?
so as such i dont think your patch is going to help without creating new "hurt"
too. :( i am tempted to just do what i suggested above with a rate limiting of
udpates to the slider to every N sec (where N is maybe 0.25 or 0.5 or something
by default).
> >
> >> Hi,
> >>
> >> The slider emits a "changed" signal when its internal representation
> >> of value really changes. It does not know whether the user dragged it
> >> or internal updates caused the value change. Most internal incremental
> >> updates when trimmed to the current resolution of the slider, do not
> >> cause a value change and the signal is not emitted on every "set".
> >>
> >> This behaviour causes a 'feedback-loop' in that the "changed" signal
> >> from an internal update, being undifferentiated from a "user-drag",
> >> causes a delayed reposition command to the emotion engine. The result
> >> is a nice 'track-jump-repeat' which in some extreme cases, a
> >> 'stuck-in-a-groove' effect.
> >>
> >> The foll. patch is a solution to this problem; and also reduces the
> >> frequency of slider updates. An alternate solution would be to use
> >> _smart_callback_del() before actually updating slider in
> >> elc_player.c:_update_slider(), and re-installing it back - but this
> >> could open the door to race conditions.
> >>
> >> Arvind
> >> ---
> >>
> >> diff -uprN a/src/lib/elc_player.c b/src/lib/elc_player.c
> >> --- a/src/lib/elc_player.c 2013-02-01 21:59:39.000000000 +0530
> >> +++ b/src/lib/elc_player.c 2013-02-01 21:59:54.000000000 +0530
> >> @@ -163,12 +163,35 @@ _update_slider(void *data,
> >> ELM_PLAYER_DATA_GET(data, sd);
> >>
> >> seekable = elm_video_is_seekable_get(sd->video);
> >> + if (seekable == sd->disabled)
> >> + elm_object_disabled_set(sd->slider, !seekable);
> >> +
> >> length = elm_video_play_length_get(sd->video);
> >> - pos = elm_video_play_position_get(sd->video);
> >>
> >> - elm_object_disabled_set(sd->slider, !seekable);
> >> - elm_slider_min_max_set(sd->slider, 0, length);
> >> - elm_slider_value_set(sd->slider, pos);
> >> + if (sd->length != length) {
> >> + sd->length = length;
> >> + elm_slider_min_max_set(sd->slider, 0, length);
> >> + }
> >> +
> >> + if (sd->frames < 0) {
> >> + pos = elm_video_play_position_get(sd->video);
> >> + if (fabs(sd->pos - pos) > 0.5) {
> >> + sd->pos = pos;
> >> + sd->frames = 30;
> >> + elm_slider_value_set(sd->slider, pos);
> >> + }
> >> + }
> >> +}
> >> +
> >> +static void
> >> +_frame_decoded_cb(void *data,
> >> + Evas_Object *obj __UNUSED__,
> >> + void *event_info __UNUSED__)
> >> +{
> >> + ELM_PLAYER_DATA_GET(data, sd);
> >> + /* do not update slider on every frame */
> >> + if (--sd->frames < 0)
> >> + _update_slider(data, obj, event_info);
> >> }
> >>
> >> static void
> >> @@ -176,9 +199,24 @@ _update_position(void *data,
> >> Evas_Object *obj __UNUSED__,
> >> void *event_info __UNUSED__)
> >> {
> >> + /*
> >> + * called wnenever the value of the slider is changed.
> >> + * - irespective of user drags
> >> + * or
> >> + * internal increments larger than the slider resolution.
> >> + * the 2nd case causes an inadvertent repositioning of stream
> >> + * which some engines respond to (xine).
> >> + * This is a feedback loop that needs to be broken.
> >> + */
> >> + double pos;
> >> +
> >> ELM_PLAYER_DATA_GET(data, sd);
> >>
> >> - elm_video_play_position_set(sd->video, elm_slider_value_get
> >> (sd->slider));
> >> + pos = elm_slider_value_get(sd->slider);
> >> + if (fabs(sd->pos - pos) > 0.25) {
> >> + elm_video_play_position_set(sd->video, pos);
> >> + sd->pos = pos;
> >> + }
> >> }
> >>
> >> static void
> >> @@ -455,7 +493,7 @@ _elm_player_smart_content_set(Eo *obj, v
> >> else elm_layout_signal_emit(obj, "elm,player,pause", "elm");
> >>
> >> evas_object_smart_callback_add(sd->emotion, "frame_decode",
> >> - _update_slider, obj);
> >> + _frame_decoded_cb, obj);
> >> evas_object_smart_callback_add(sd->emotion, "frame_resize",
> >> _update_slider, obj);
> >> evas_object_smart_callback_add(sd->emotion, "length_change",
> >> @@ -495,9 +533,13 @@ _elm_player_smart_add(Eo *obj, void *_pd
> >> (priv->slider, _double_to_time, _str_free);
> >> elm_slider_units_format_function_set
> >> (priv->slider, _double_to_time, _str_free);
> >> - elm_slider_min_max_set(priv->slider, 0, 0);
> >> + elm_slider_min_max_set(priv->slider, 0, 1);
> >> elm_slider_value_set(priv->slider, 0);
> >> elm_object_disabled_set(priv->slider, EINA_TRUE);
> >> + priv->disabled = EINA_TRUE;
> >> + priv->length = 0.0;
> >> + priv->frames = 0;
> >> + priv->pos = 0.0;
> >> evas_object_size_hint_align_set(priv->slider, EVAS_HINT_FILL, 0.5);
> >> evas_object_size_hint_weight_set
> >> (priv->slider, EVAS_HINT_EXPAND, EVAS_HINT_EXPAND);
> >> diff -uprN a/src/lib/elm_widget_player.h b/src/lib/elm_widget_player.h
> >> --- a/src/lib/elm_widget_player.h 2012-11-26 12:02:53.000000000 +0530
> >> +++ b/src/lib/elm_widget_player.h 2013-02-01 21:59:54.000000000 +0530
> >> @@ -33,6 +33,11 @@ struct _Elm_Player_Smart_Data
> >> Evas_Object *rewind;
> >> Evas_Object *stop;
> >> Evas_Object *slider;
> >> +
> >> + Eina_Bool disabled; /* used for only-when-must changes */
> >> + double length; /* --- ditto --- */
> >> + short frames; /* used to reduce update frequency */
> >> + double pos; /* inter-callback communication to break
> >> slider feedback loop */
> >> };
> >>
> >> /**
> >>
> >> ------------------------------------------------------------------------------
> >> Everyone hates slow websites. So do we.
> >> Make your web apps faster with AppDynamics
> >> Download AppDynamics Lite for free today:
> >> http://p.sf.net/sfu/appdyn_d2d_jan
> >> _______________________________________________
> >> enlightenment-users mailing list
> >> enlightenment-users@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/enlightenment-users
> >>
> >
> >
> > --
> > ------------- Codito, ergo sum - "I code, therefore I am" --------------
> > The Rasterman (Carsten Haitzler) ras...@rasterman.com
> >
>
--
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler) ras...@rasterman.com
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
enlightenment-users mailing list
enlightenment-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-users
