[PATCH] compositor: fix overflow issue when calculate msecs of vblank
Wang, Quanxian
quanxian.wang at intel.com
Wed Sep 5 19:36:17 PDT 2012
> -----Original Message-----
> From: Kristian Høgsberg [mailto:hoegsberg at gmail.com]
> Sent: Thursday, September 06, 2012 10:31 AM
> To: Wang, Quanxian
> Cc: wayland-devel at lists.freedesktop.org; Guo, Chaohong; Bradford, Robert
> Subject: Re: [PATCH] compositor: fix overflow issue when calculate msecs of
> vblank
>
> On Wed, Sep 5, 2012 at 9:28 PM, Wang, Quanxian <quanxian.wang at intel.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Kristian Høgsberg [mailto:hoegsberg at gmail.com]
> >> Sent: Thursday, September 06, 2012 3:28 AM
> >> To: Wang, Quanxian
> >> Cc: wayland-devel at lists.freedesktop.org; Guo, Chaohong; Bradford,
> >> Robert
> >> Subject: Re: [PATCH] compositor: fix overflow issue when calculate
> >> msecs of vblank
> >>
> >> On Wed, Sep 05, 2012 at 03:35:32AM +0000, Wang, Quanxian wrote:
> >> > From "Quanxian Wang"
> >> > <quanxian.wang at intel.com<mailto:quanxian.wang at intel.com>>
> >> >
> >> > commit f7b156cef1ed8081df6f25cc0e66c8d7fbf414fc
> >> > Author: Wang Quanxian <quanxian.wang at intel.com>
> >> > Date: Wed Sep 5 11:30:38 2012 +0800
> >> >
> >> > Change int to long to avoid the overflow when calculation
> >> > vblank msecs
> >> >
> >> > secs and nsecs are from vblank event, the number of secs is
> >> > very
> >> big(134xxxxxxx),
> >> > when turns secs to msecs(multiple 1000), it will exceed uint32
> >> > max value.
> >> >
> >> > Signed-Off-By Quanxian Wang <quanxian.wang at intel.com>
> >>
> >> No, the overflow is itentional. The timestamp is just a milisecond
> >> value with an unspecified epoch, only differences between timestamps
> >> are useful. It may overflow and that's expected. If you use
> >> uint32_t math, you can subtract timestamps before and after an
> >> overflow and still get the number of miliseconds elapsed.
> > [Wang, Quanxian] I agree your point partly. My different point is for example,
> take 100 as the limit, you have 101 and 99 number, the diff is 2. To 101, after
> conversion, the number is 01 and to 99 is 99. The diff turns to be 98 and
> compared with previous 2 it turns to bigger. In the limit, it is the safe, cross the
> border is not safe.
>
> No unsigned integer math in C is defined so that this actually works.
> Assume four bit integers (that is, our range is 0-15). Suppose you have
> timestamps 14 and 17. The difference is 17 - 14 = 3. But with 4 bit unsigned
> integers, the values are truncated to 14 and 1. However,
> 1 - 14 still gives the right result.
[Wang, Quanxian] yes. It is minus. I see. Thanks
>
> Kristian
>
> >>
> >> Kristian
> >>
> >> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c index
> >> > df81aba..8b6285c 100644
> >> > --- a/src/compositor-drm.c
> >> > +++ b/src/compositor-drm.c
> >> > @@ -447,7 +447,7 @@ vblank_handler(int fd, unsigned int frame,
> >> > unsigned
> >> int sec, unsigned int usec,
> >> > struct drm_sprite *s = (struct drm_sprite *)data;
> >> > struct drm_compositor *c = s->compositor;
> >> > struct drm_output *output = s->output;
> >> > - uint32_t msecs;
> >> > + uint64_t msecs = 0;
> >> >
> >> > output->vblank_pending = 0;
> >> >
> >> > @@ -480,7 +480,7 @@ page_flip_handler(int fd, unsigned int frame,
> >> > unsigned int sec, unsigned int usec, void *data) {
> >> > struct drm_output *output = (struct drm_output *) data;
> >> > - uint32_t msecs;
> >> > + uint64_t msecs=0;
> >> >
> >> > output->page_flip_pending = 0;
> >> >
> >> > @@ -496,7 +496,7 @@ page_flip_handler(int fd, unsigned int frame,
> >> > output->next = NULL;
> >> >
> >> > if (!output->vblank_pending) {
> >> > - msecs = sec * 1000 + usec / 1000;
> >> > + msecs = (uint64_t)sec * 1000 + (uint64_t)usec /
> >> > + 1000;
> >> > weston_output_finish_frame(&output->base, msecs);
> >> > }
> >> > }
> >> > diff --git a/src/compositor.c b/src/compositor.c index
> >> > f4263ac..37c52bc 100644
> >> > --- a/src/compositor.c
> >> > +++ b/src/compositor.c
> >> > @@ -1026,7 +1026,7 @@ weston_output_damage(struct weston_output
> >> > *output)
> >> >
> >> > static void
> >> > fade_frame(struct weston_animation *animation,
> >> > - struct weston_output *output, uint32_t msecs)
> >> > + struct weston_output *output, uint64_t msecs)
> >> > {
> >> > struct weston_compositor *compositor =
> >> > container_of(animation, @@ -1129,7 +1129,7 @@
> >> > surface_accumulate_damage(struct weston_surface *surface, }
> >> >
> >> > static void
> >> > -weston_output_repaint(struct weston_output *output, uint32_t
> >> > msecs)
> >> > +weston_output_repaint(struct weston_output *output, uint64_t
> >> > +msecs)
> >> > {
> >> > struct weston_compositor *ec = output->compositor;
> >> > struct weston_surface *es;
> >> > @@ -1222,7 +1222,7 @@ weston_compositor_read_input(int fd,
> uint32_t
> >> > mask, void *data) }
> >> >
> >> > WL_EXPORT void
> >> > -weston_output_finish_frame(struct weston_output *output, uint32_t
> >> > msecs)
> >> > +weston_output_finish_frame(struct weston_output *output, uint64_t
> >> > +msecs)
> >> > {
> >> > struct weston_compositor *compositor = output->compositor;
> >> > struct wl_event_loop *loop = diff --git a/src/compositor.h
> >> > b/src/compositor.h index
> >> > 7a8058e..f49da84 100644
> >> > --- a/src/compositor.h
> >> > +++ b/src/compositor.h
> >> > @@ -109,7 +109,7 @@ struct weston_spring {
> >> > double current;
> >> > double target;
> >> > double previous;
> >> > - uint32_t timestamp;
> >> > + uint64_t timestamp;
> >> > };
> >> >
> >> > enum {
> >> > @@ -164,7 +164,7 @@ struct weston_output {
> >> > struct weston_output_zoom zoom;
> >> > int dirty;
> >> > struct wl_signal frame_signal;
> >> > - uint32_t frame_time;
> >> > + uint64_t frame_time;
> >> > int disable_planes;
> >> >
> >> > char *make, *model;
> >> > @@ -493,7 +493,7 @@ void
> >> > weston_spring_init(struct weston_spring *spring,
> >> > double k, double current, double target); void
> >> > -weston_spring_update(struct weston_spring *spring, uint32_t msec);
> >> > +weston_spring_update(struct weston_spring *spring, uint64_t msec);
> >> > int
> >> > weston_spring_done(struct weston_spring *spring);
> >> >
> >> > @@ -543,7 +543,7 @@ void
> >> > weston_plane_release(struct weston_plane *plane);
> >> >
> >> > void
> >> > -weston_output_finish_frame(struct weston_output *output, uint32_t
> >> > msecs);
> >> > +weston_output_finish_frame(struct weston_output *output, uint64_t
> >> > +msecs);
> >> > void
> >> > weston_output_schedule_repaint(struct weston_output *output);
> >> > void diff --git a/src/util.c b/src/util.c index 4ff451a..34b140e
> >> > 100644
> >> > --- a/src/util.c
> >> > +++ b/src/util.c
> >> > @@ -42,7 +42,7 @@ weston_spring_init(struct weston_spring *spring,
> >> > }
> >> >
> >> > WL_EXPORT void
> >> > -weston_spring_update(struct weston_spring *spring, uint32_t msec)
> >> > +weston_spring_update(struct weston_spring *spring, uint64_t msec)
> >> > {
> >> > double force, v, current, step;
> >> >
> >> >
> >>
> >> > _______________________________________________
> >> > wayland-devel mailing list
> >> > wayland-devel at lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
More information about the wayland-devel
mailing list