[PATCH] compositor: fix overflow issue when calculate msecs of vblank
Kristian Høgsberg
hoegsberg at gmail.com
Wed Sep 5 19:31:06 PDT 2012
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.
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