Hi Axel, I agree with the clock comments, in that we need to know which clock to use, and 32 bits may be too short. If we follow struct timespec, uint32_t seconds + uint32_t nanoseconds should be fine. We don't have 64-bit integers in the protocol.
How about adding an event which tells the client, which clock the compositor is using? We cannot simply require CLOCK_MONOTONIC_RAW or CLOCK_MONOTONIC_COARSE, because they are Linux-specific and introduced in 2.6.28 and 2.6.32, respectively. The clock name should probably be platform-dependant, if POSIX values are not enough, so e.g. weston could use CLOCK_MONOTONIC_RAW, or whatever is the most appropriate, and on other OSs use what's appropriate there. More below... On Sun, 3 Nov 2013 00:39:58 +0100 Axel Davy <[email protected]> wrote: > These two new requests are designed to help video players > to synchronize what is displayed on the screen and the audio, > and to implement the X Present extension in XWayland. > --- > protocol/wayland.xml | 70 > +++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 67 insertions(+), 3 deletions(-) > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml > index a1df007..553af61 100644 > --- a/protocol/wayland.xml > +++ b/protocol/wayland.xml > @@ -959,7 +959,7 @@ > </event> > </interface> > > - <interface name="wl_surface" version="3"> > + <interface name="wl_surface" version="4"> > <description summary="an onscreen surface"> > A surface is a rectangular area that is displayed on the screen. > It has a location, size and pixel contents. > @@ -1057,8 +1057,9 @@ > > <request name="frame"> > <description summary="request repaint feedback"> > - Request notification when the next frame is displayed. Useful > - for throttling redrawing operations, and driving animations. > + Request notification when the next frame is used for the first time > + by the compositor. Useful for throttling redrawing operations, and > + driving animations. "For the first time"? Does that match what happens if a client sends frame,commit without an attach, and the current wl_buffer is already on screen? TBF, this may have been a little vague to begin with. > The frame request will take effect on the next wl_surface.commit. > The notification will only be posted for one frame unless > requested again. > @@ -1235,6 +1236,69 @@ > </request> > </interface> > > + <!-- Version 4 additions --> > + > + <request name="presentation_time" since="4"> > + <description summary="request the frame of next commit to hit the > screen at a specific time"> > + This request is used to indicate the compositor at which ust time the > + client wish the frame of next commit to hit the screen. > + > + The request will take effect on the next wl_surface.commit. > + The ust time indicated is in Milliseconds. Explain "ust"? > + > + If the ust time requested has already happened, then the next commit > + will be processed as any other commit. Could you be more specific in "as any other commit"? What would happen in the following scenario: - current time is T seconds - client commits buffer D with ust = T+6 - client commits buffer C with ust = T+1 - client commits buffer B with ust = T-4 - client commits buffer A with ust = T-9 As buffer A is committed last with ust in the past, the commit will be executed immediately, leaving buffer A on screen instead of B which would be preferred based on timestamps. This needs some more explanation as to what should actually happen. The intent is probably clear, but following the wording produces a wrong result, IMHO. > + > + The client can do another commit request without cancelling > + a commit associated to a requested presentation time that has not > + already happened. Is this saying, that one can queue frames in advance, and the queue can hold multiple frames? In that case you should explicitly say, that there is a queue involved. How can a client cancel queued frames, or wipe the queue? A client may have queued several frames' worth, when it gets an input event, which requires it to cancel and perhaps redo already queued frames. What state is queued, and what state is not? Is all double-buffered state, that is latched in on commit, queued? If so, how will the client be aware, which set of state is current at any time, when e.g. input events come? How does this interact with the sub-surface protocol, especially - if this surface is a sub-surface in synchronized mode? - if this surface is a parent surface with synchronized sub-surfaces? > + > + The compositor can choose to ignore the indicated ust time, and for > + example, if the client has queued too much buffers, it can choose to > + treat some past commit with a future ust time, as commits with no > + ust time indicated. It's good to specify what happens when there are too many buffers in a queue, but what does that mean really? Is the compositor applying all the queued state changes up to a point where the queue has a sane length again? Shouldn't this be defined to take and process the commits with the oldest ust first? Not in the order they actually were committed? Or, could we simply assume that any sane queue length will be much smaller than some arbitrary implementation defined constant, and if that number is ever reached, the client is misbehaving and deserves a protocol error, disconnecting it? That would be a lot simpler, and simply queueing a 1000 frames should not be enough to let a compositor hit OOM - after all the client has had to have allocated the buffers in the first place, so the client is susceptible to hit OOM much much earlier; or the client is re-using buffers before they have been released, so it is not a good citizen anyway. > + Calling a second time presentation_time on a wl_surface without doing > + a commit will replace the last ust time indicated. > + </description> > + > + <arg name="ust" type="uint"/> > + </request> > + > + <request name="hit" since="4"> > + <description summary="request hit feedback"> > + Request notification when the next frame hits a physical screen. > + This notification, which should happen after the frame notification, > + can be used to synchronize video and audio better. The time given > + will be ust time, in Milliseconds. Shouldn't this be combined with presentation_time request? If a client cares about the presentation time, it probably also wants to know how it succeeded, right? > + If, for any reason, the compositor determines that the frame will > + never hit the screen, then the callback is called with 0 as argument, > + instead of the ust time. One possible reason is that the compositor > + choose to use a newer frame sent by the client. 0 as a special invalid timestamp... that is a bit sketchy, since I don't think anything defines 0 as an invalid timestamp. > + The client is not supposed to throttle its drawing to this > + notification, but to the frame notification. > + > + If the frame hits multiple physical screens, only the first time it > + hits a screen triggers the hit callback. Maybe better to define it as "only one of them will trigger the callback", so that compositors are free to trigger it when the output they synchronize the surface to updates. For instance, if the surface is mostly on one output, and bit on the other. > + The hit request will take effect on the next wl_surface.commit. > + The notification will only be posted for one frame unless > + requested again. > + > + A client can request a hit callback even without an attach, > + damage, or any other state changes, since wl_surface.commit triggers a > + display update. Triggering a display update is a compositor implementation detail, and should not be mentioned in the protocol like this. It is perfectly possible for the commit to not change anything visible on screen, and therefore the compositor will not repaint. If you want to allow requesting for the hit timestamp after the fact, you should define it so, and compositors can reply with the original timestamp when the buffer hit the screen the first time (on its main output). It think that would be more explicit. > + The object returned by this request will be destroyed by the > + compositor after the callback is fired and as such the client must not > + attempt to use it after that point. > + </description> > + > + <arg name="callback" type="new_id" interface="wl_callback"/> > + </request> > + > <interface name="wl_seat" version="3"> > <description summary="group of input devices"> > A seat is a group of keyboards, pointer and touch devices. This So, this proposal requires *all* compositors to implement presentation timestamping and frame queues for wl_surfaces. And I really mean ALL compositors, including any session switchers, system compositors, embedded compositors, compositors nested in web-brosers for each-tab-is-a-separate-process... I think that is a bit much. Extending wl_surface like this does not allow a compositor to not implement all this. This is also a bit risky in case we ever need to change this interface. Therefore I would really prefer the separate interface design, where you would also have a natural place to put the what-clock-to-use event. Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
