On Wed, Sep 5, 2012 at 9:28 PM, Wang, Quanxian <[email protected]> wrote:
>
>
>> -----Original Message-----
>> From: Kristian Høgsberg [mailto:[email protected]]
>> Sent: Thursday, September 06, 2012 3:28 AM
>> To: Wang, Quanxian
>> Cc: [email protected]; 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"
>> > <[email protected]<mailto:[email protected]>>
>> >
>> > commit f7b156cef1ed8081df6f25cc0e66c8d7fbf414fc
>> > Author: Wang Quanxian <[email protected]>
>> > 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 <[email protected]>
>>
>> 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
>> > [email protected]
>> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
