Hi, this looks mostly very good. There is one mistake, and some things that need more thought or better wording.
On Fri, 08 Nov 2013 06:59:38 -0500 Frederic Plourde <[email protected]> wrote: > Hi, > > I have gathered comments and suggestions from colleagues and wayland-devel > reviewers and here is RFC v.2 of our buffer queue + presentation feedback > protocol extension. > > Notice the following changes : > ----------------------------------------- > - it's changed name to make it more general. (comments/ideas on the protocol > and interfaces names are welcome). We believe this wl_surface extension > really is down to adding a timed buffer queue, hence the wl_buffer_queue > interface name. > > - it's been extended to include presentation time feedback > (wl_presentation_time) > > - Extensive comments were added. > > please, see the code below: > > > > <?xml version="1.0" encoding="UTF-8"?> > <protocol name="presentation_timing"> > > <copyright> > Copyright © 2012-2013 Collabora, Ltd. > > Permission to use, copy, modify, distribute, and sell this > software and its documentation for any purpose is hereby granted > without fee, provided that the above copyright notice appear in > all copies and that both that copyright notice and this permission > notice appear in supporting documentation, and that the name of > the copyright holders not be used in advertising or publicity > pertaining to distribution of the software without specific, > written prior permission. The copyright holders make no > representations about the suitability of this software for any > purpose. It is provided "as is" without express or implied > warranty. > > THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS > SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND > FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY > SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN > AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, > ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF > THIS SOFTWARE. > </copyright> > > <interface name="wl_presentation" version="1"> > <description summary="buffer queues with presentation timing information/ > for surfaces"> > The global factory interface exposing timestamped buffer queuing > composi- > ting capabilities. > > The aim of presentation timing is to support streaming video generally > coming from videosink clients that typically need to queue video buffers > with presentation timestamps in order to accurately synchronize video > and audio streams. > > This global interface allows for the creation of timestamped buffer > queues for wl_surface objects. > </description> > > <request name="destroy" type="destructor"> > <description summary="unbind from the wl_presentation interface"> > Tell the server that the client will not be using this > protocol object anymore. This does not affect any other > objects, wl_buffer_queue objects included. > </description> > </request> > > <enum name="error"> > <entry name="buffer_queue_exists" value="0" > summary="the surface already has a buffer_queue object/ > associated to it"/> > </enum> > > <event name="clock_id", type="uint"> > <description summary="clock ID to use"> > Tell the client which clock ID is the compositor going to use for > time- > stamps and presentation feedback. > > Compositor sends that event once to a client right after it binds to > the global interface. > </description> > </event> > > <request name="create_queue"> > <description summary="add buffer queueing capabilities to a wl_surface"> > Create and attach a wl_buffer_queue interface to the given wl_sur- > face. This effectively adds buffer queueing and presentation timing > feedback capabilities to the surface through the use of buffer queues > and presentation callbacks. > > If the given wl_surface already has a wl_buffer_queue object > associated > to it, the buffer_queue_exists protocol error is raised. > > Here, clock_id is needed to negociate time domain with the compositor > as a common basis when specifing timestamps and presentation > callbacks. > clock_id is an implementation-specific integer representing the clock > identification. > </description> > > <arg name="id" type="new_id" interface="wl_buffer_queue" > summary="the new buffer_queue object id"/> > <arg name="surface" type="object" interface="wl_surface" > summary="the surface to be turned intio a buffer_queue surface"/> > <arg name="clock_id" type="uint" summary="clock ID to be used for/ > timestamps"> The clock_id argument should not be here. There is only one clock id to be used, and that is the one sent by the compositor. > </request> > </interface> > > <interface name="wl_buffer_queue" version="1"> > <description summary="buffer queue interface to a wl_surface"> > An additional interface to a wl_surface object to enable buffer queueing > capabilities. A buffer_queue-enabled surface contains a buffer queue and > exposes protocol that allows for queuing timestamped wl_buffer objects > into the queue and requesting presentation time feedback. > > Buffer_queue-enabled surfaces do not declare any state in addition to > the > ones wl_surfaces already use by default. Thus, once a plain wl_surface > has > been turned into a buffer_queue-enabled surface, buffer queuing hap- > pens through the "queue" request. This means that queuing up > buffers does *not* require any surface.attach nor surface.commit. More- > over, such an attach + commit sequence will clear the buffer queue in > order to exclusively consider the newly attached buffer. Hence, > surface.attach and surface.commit should *not* be used when specifying > streaming surface content via buffer_queue. Also, queuing up a wl_buffer > makes it "reserved" in the compositor just like attach + commit does. > > Besides, it is perfectly acceptable to commit (without attach) some > states on a buffer_queue-enabled surface, as it will not clear the > buffer > queue nor interfere with any presentation timing mechanism brought by > the presentation_timing interfaces. > > The compositor will attempt to repaint so that each queued buffer gets > presented at the requested target time. However, this may not always be > possible e.g. when a requested presentation time cannot be met > accurately > or when the target time has completely passed already and been replaced > by > another buffer by the time the compositor can repaint again. Therefore, > > At every repaint cycle, enqueued buffers with past-timestamps, if any, > will be considered and the compositor will present the most recent one > among them. At that point, buffers with older timestamps will be removed > from the queue and released (a BUFFER_RELEASE event will be fired for > every removed one). Also, at every repaint cycle, future-timestamped > buffers will be kept untouched in the queue. Well, buffer releases are fired as appropriate, i.e. the buffer really is free. Also, send the presentation feedback events, specifically discarded. One thing we forgot to specify is whether the applied buffer keeps the requested presentation timestamp and if buffers queued afterwards are compared against that. I think there is no need to compare the currently on-screen buffer's timestamp to those in the queue, or can someone think of a case where it would be useful? OTOH, Axel's use case for xwayland does not want it, so that is a point for not considering the current buffer. > > When applying a buffer from the queue, the compositor implies full > surface damage. If the client manages to attach + damage + commit in the > meantime, then the damage is what it explicitely specified (and the > buffer_queue gets cleared) Some observations: Since the buffer queue does not manipulate the "pending state" of a wl_surface, and wl_surface.commit atomically applies the pending state, applying a queued buffer in the middle of updating a surface state will not affect what gets committed in the end. So, there is no race. And as attach + commit clears the queue, the surface content is guaranteed to become the attached buffer until further requests, and previously queued buffers will not overwrite that. So no race in that case either. > </description> > > <request name="destroy" type="destructor"> > <description summary="remove buffer_queue interface"> > The buffer_queue interface is removed from the buffer_queue-enabled > surface. This could also mention, that the queue is emptied first, and release and presentation feedback events are emitted as usual. Can describe it as an implicit "clear" request. > </description> > </request> > > <request name="queue"> > <description summary="enqueue a buffer into the surface's buffer queue"> > Queue a buffer along with its timestamp into the surface's internal > buffer queue. This timestamp is the intended presentation time. While > clients generally want to queue posterior timestamps (future), > queueing > up anterior timestamps (in the past) is allowed and does not raise any > protocol error. > > The speficied timestamp is two-part. tv_sec is the number of seconds > and tv_nsec defines the number of nanoseconds spent after tv_sec. > > Calling queue() will create a wl_presentation_time object that is ex- > clusively associated with the provided wl_buffer lifetime in the queue > at the moment the object was created. This means that presentation > feedback concerning this wl_buffer will be provided through wl_presen- > tation_time events, but only once. As soon as the buffer is presented > and removed from the queue by the compositor, it's internal link to > the wl_presentation_time object is broken and associated presentation > events will never be called again. So a client, right after having > recei- > ved a wl_presentation_time event will generally destroy that object. > On the other hand, if a client does not want feedback, it may destroy > presentation_time object right after queuing a buffer. Might also say that the wl_presentation_time object refers to this particular submission of this wl_buffer. A client may queue the same buffer more than once, and each time it gets a unique time object. You could also use the term "inert" to describe the wl_presentation_time object after it has fired. The wl_presentation_time object is basically one-shot, just like wl_callback. Both events kill it. We could make wl_presentation_time self-destructing like wl_callback is, to avoid sending the destroy request over the wire, but... the way it is now is more familiar. Should we? It is unlikely we ever need to add any requests to wl_presentation_time, and adding more (even non-self-destructing) events is possible. > </description> > > <arg name="buffer" type="object" interface="wl_buffer" > summary="the buffer to queue"/> > <arg name="id" type="new_id" interface="wl_presentation_time" > summary="the new presentation_time object id"/> > <arg name="tv_sec" type="uint" > summary="seconds value of the buffer target timestamp"/> > <arg name="tv_nsec" type="uint" > summary="nanoseconds value of the buffer target timestamp"/> > </request> > > <request name="clear"> > <description summary="clear the surface's buffer queue"> > Clear the surface's buffer queue. All buffers are removed from the > queue and released as appropriate. The requested presentation times > are > discarded. The surface retains the contents it currently has, at the > time the compositor processes this request. The compositor may not Oops, s/may not/might not/. > release the buffer(s) currently being displayed if it is needed for > repainting or scanout, as usual. Also send the wl_presentation_time events. > </description> > </request> > </interface> > > <interface name="wl_presentation_time" version="1"> > <description summary="presentation time feedback event"> > The wl_presentation_time object encapsulates presentation time events > sent as feedback by the compositor to a client that previously queued > a wl_buffer. wl_presentation_time objects are created and returned when > a client enqueues a buffer through the wl_buffer_queue.queue() request. Strike "and returned". Returning is just C API convenience, in the protocol there is no concept of returning something. > Note that presentation time only tells client when the compositor > presen- > ted the buffer to display hardware, not when the buffer was turned into > light (actually displayed on screen) by this hardware. As there could > be anything displaying those buffers, from very fast, low-latency > computer monitors to slow, hi-latency HDMI TV screens, it is the > client's > responsibility to make sure it knows what display hardware is currently > connected and what is its latency. Do people agree with this definition? I assume it means when the gfx card starts to emit the pixels to the physical wire. Or should we allow the compositor to factor in the possible information from the monitor about its latency? I'm kind of thinking that we should. If a monitor does not tell its latency, and the user can see it, the compositor could allow the user to configure an assumed latency. Perhaps even measured by the user. Or is that something that should be (or even already is?) configured in the drivers, so they already give out the "turns into light" time? Also, the presentation timestamp can only be given wrt. one output. If the surface spans several outputs, the compositor chooses one to sync to. > </description> > > <request name="destroy" type="destructor"> > <description summary="destroy presentation time request"> > The presentation_time object is destroyed and none of its events will > ever be called again. > </description> > </request> > > <event name="presented"> > <description summary="buffer was displayed"> > Tell the client that a buffer was presented at time T=tv_sec+tv_nsec. > This event only pertains to the one wl_buffer that was passed in when > calling the buffer_queue.queue Thus, a particular presentation time > feeback event is always associated with specific wl_buffer lifetime > in > the buffer queue. Or in other words, this is related to a specific submission of a wl_buffer. > </description> > <arg name="tv_sec" type="uint" > summary="seconds value of the buffer presentation timestamp"/> > <arg name="tv_nsec" type="uint" > summary="nanoseconds value of the buffer presentation timestamp"/> > </event> > > <event name="discarded"> > <description summary="buffer was not displayed"> > The buffer scheduled for presentation was not displayed and was > removed from the buffer_queue. > </description> > </event> > </interface> > </protocol> > Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
