[PATCH wayland-protocols 1/4] stable: add presentation-time draft
Bryce Harrington
bryce at osg.samsung.com
Tue Feb 23 18:21:44 UTC 2016
On Tue, Feb 23, 2016 at 12:27:23PM +0200, Pekka Paalanen wrote:
> On Mon, 22 Feb 2016 15:39:18 -0800
> Bryce Harrington <bryce at osg.samsung.com> wrote:
>
> > On Mon, Feb 22, 2016 at 03:34:40PM +0200, Pekka Paalanen wrote:
> > > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > >
> > > This XML file has been copied verbatim from Weston 1.10.0 release,
> > > protocol/presentation_timing.xml. The last behavioral change to that
> > > file was in December 2014, so the behaviour is considered stable.
> > >
> > > Interfaces still need to be renamed according wayland-protocols policy.
> > > That will be done in a follow-up patch to clearly show the changes.
> > >
> > > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >
> > Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>
> >
> > Presumably the API is fine, below comments focus on grammatical
> > suggestions. Nothing terribly critical, but there are a few clunky
> > passages which might be worth revisiting now that this is becoming
> > official.
>
> Hi,
>
> cool.
>
> > > ---
> > > stable/presentation-time/README | 5 +
> > > stable/presentation-time/presentation-time.xml | 274 +++++++++++++++++++++++++
> > > 2 files changed, 279 insertions(+)
> > > create mode 100644 stable/presentation-time/README
> > > create mode 100644 stable/presentation-time/presentation-time.xml
> > >
> > > diff --git a/stable/presentation-time/README b/stable/presentation-time/README
> > > new file mode 100644
> > > index 0000000..c7781ea
> > > --- /dev/null
> > > +++ b/stable/presentation-time/README
> > > @@ -0,0 +1,5 @@
> > > +Presentation time protocol
> > > +
> > > +Maintainers:
> > > +Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > +
> > > diff --git a/stable/presentation-time/presentation-time.xml b/stable/presentation-time/presentation-time.xml
> > > new file mode 100644
> > > index 0000000..10a5f14
> > > --- /dev/null
> > > +++ b/stable/presentation-time/presentation-time.xml
> > > @@ -0,0 +1,274 @@
> > > +<?xml version="1.0" encoding="UTF-8"?>
> > > +<protocol name="presentation_timing">
> > > +<!-- wrap:70 -->
> > > +
> > > + <copyright>
> > > + Copyright © 2013-2014 Collabora, Ltd.
> >
> > Probably extend the copyright date to now, unless there's a specific
> > reason not to?
>
> 2014 is the last time any significant (copyrightable) changes were
> made, so I don't see a reason to bump it. Unless we make non-trivial
> changes now, of course.
I would think the protocol becoming officially stable would be a
suitably non-trivial change. But whatever, it's your copyright. ;-)
> > > + Permission is hereby granted, free of charge, to any person obtaining a
> > > + copy of this software and associated documentation files (the "Software"),
> > > + to deal in the Software without restriction, including without limitation
> > > + the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > > + and/or sell copies of the Software, and to permit persons to whom the
> > > + Software is furnished to do so, subject to the following conditions:
> > > +
> > > + The above copyright notice and this permission notice (including the next
> > > + paragraph) shall be included in all copies or substantial portions of the
> > > + Software.
> > > +
> > > + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > > + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > > + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > > + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > > + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > > + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > + DEALINGS IN THE SOFTWARE.
> > > + </copyright>
> > > +
> > > + <interface name="presentation" version="1">
> > > + <description summary="timed presentation related wl_surface requests">
> > > +
> > > +<!-- Introduction -->
> > > +
> > > + The main feature of this interface is accurate presentation
> > > + timing feedback to ensure smooth video playback while maintaining
> > > + audio/video synchronization. Some features use the concept of a
> > > + presentation clock, which is defined in presentation.clock_id
> > > + event.
> >
> > "defined in the"
>
> Yup.
>
> > > + Request 'feedback' can be regarded as an additional wl_surface
> > > + method. It is part of the double-buffered surface state update
> > > + mechanism, where other requests first set up the state and then
> > > + wl_surface.commit atomically applies the state into use. In
> > > + other words, wl_surface.commit submits a content update.
> >
> > Could this paragraph be restated to be more clear?
>
> Perhaps, but I have hard time finding the words. The key points are
> that feedback is double-buffered, part of a commit as all
> double-buffered state is, and it defines the term "content update" used
> later.
I would suggest a rephrasing myself but I'm not sure I comprehend your
description adequately enough to do so. But I can point out some
phrasings that I think are problematic: 'can be regarded' seems a bit
nebulous; are there other ways it could be regarded? 'applies the state
into use' seems an awkward phrasing. 'In other words' makes me wonder
if your simpler final sentence summarizing the thing would be better
placed at the start of the paragraph.
> > > +<!-- Completing presentation -->
> > > +
> > > + When the final realized presentation time is available, e.g.
> > > + after a framebuffer flip completes, the requested
> > > + presentation_feedback.presented events are sent. The final
> > > + presentation time can differ from the compositor's predicted
> > > + display update time and the update's target time, especially
> > > + when the compositor misses its target vertical blanking period.
> > > + </description>
> > > +
> > > + <enum name="error">
> > > + <description summary="fatal presentation errors">
> > > + These fatal protocol errors may be emitted in response to
> > > + illegal presentation requests.
> > > + </description>
> > > + <entry name="invalid_timestamp" value="0"
> > > + summary="invalid value in tv_nsec"/>
> > > + <entry name="invalid_flag" value="1"
> > > + summary="invalid flag"/>
> > > + </enum>
> > > +
> > > + <request name="destroy" type="destructor">
> > > + <description summary="unbind from the presentation interface">
> > > + Informs the server that the client will not be using this
> > > + protocol object anymore. This does not affect any existing
> > > + objects created by this interface.
> >
> > Slightly rephrased:
> >
> > Informs the server that the client will no longer be using this
> > protocol object. Existing objects created by this interface are
> > not affected.
>
> That's better, though I'll replace "interface" with "object".
>
> > > + </description>
> > > + </request>
> > > +
> > > + <request name="feedback">
> > > + <description summary="request presentation feedback information">
> > > + Request presentation feedback for the current content submission
> > > + on the given surface. This creates a new presentation_feedback
> > > + object, which will deliver the feedback information once. If
> > > + multiple presentation_feedback objects are created for the same
> > > + submission, they will all deliver the same information.
> > > +
> > > + For details on what information is returned, see
> > > + presentation_feedback interface.
> > > + </description>
> > > +
> > > + <arg name="surface" type="object" interface="wl_surface"
> > > + summary="target surface"/>
> > > + <arg name="callback" type="new_id" interface="presentation_feedback"
> > > + summary="new feedback object"/>
> > > + </request>
> > > +
> > > + <event name="clock_id">
> > > + <description summary="clock ID for timestamps">
> > > + This event tells the client in which clock domain the
> > > + compositor interprets the timestamps used by the presentation
> > > + extension. This clock is called the presentation clock.
> > > +
> > > + The compositor sends this event when the client binds to the
> > > + presentation interface. The presentation clock does not change
> > > + during the lifetime of the client connection.
> > > +
> > > + The clock identifier is platform dependent. Clients must be
> > > + able to query the current clock value directly, not by asking
> > > + the compositor.
> > > +
> > > + On Linux/glibc, the identifier value is one of the clockid_t
> > > + values accepted by clock_gettime(). clock_gettime() is defined
> > > + by POSIX.1-2001.
> > > +
> > > + Compositors should prefer a clock which does not jump and is
> > > + not slewed e.g. by NTP. The absolute value of the clock is
> > > + irrelevant. Precision of one millisecond or better is
> > > + recommended.
> > > +
> > > + Timestamps in this clock domain are expressed as tv_sec_hi,
> > > + tv_sec_lo, tv_nsec triples, each component being an unsigned
> > > + 32-bit value. Whole seconds are in tv_sec which is a 64-bit
> > > + value combined from tv_sec_hi and tv_sec_lo, and the
> > > + additional fractional part in tv_nsec as nanoseconds. Hence,
> > > + for valid timestamps tv_nsec must be in [0, 999999999].
> > > +
> > > + Note that clock_id applies only to the presentation clock,
> > > + and implies nothing about e.g. the timestamps used in the
> > > + Wayland core protocol input events.
> >
> > The above docs seem fine but fwiw I have a feeling that with some
> > copyediting it could be condensed down better. Maybe it should start
> > out something like,
> >
> > Defines the 'presentation clock' that the compositor will be using to
> > express timestamp data for the client. The presentation clock is a
> > platform-dependent blah blah...
> >
> > I'd also suggest organizing it such that the discussion that is
> > pertinent to clients is towards the beginning, and the compositor
> > implementation notes are at the end.
>
> Ok, I'm not quite sure how you'd change it, but I'll think about it.
>
> > > + </description>
> > > +
> > > + <arg name="clk_id" type="uint" summary="platform clock identifier"/>
> > > + </event>
> > > +
> > > + </interface>
> > > +
> > > + <interface name="presentation_feedback" version="1">
> > > + <description summary="presentation time feedback event">
> > > + A presentation_feedback object returns an indication that a
> > > + wl_surface content update has become visible to the user.
> > > + One object corresponds to one content update submission
> > > + (wl_surface.commit). There are two possible outcomes: the
> > > + content update is presented to the user, and a presentation
> > > + timestamp delivered; or, the user did not see the content
> > > + update because it was superseded or its surface destroyed,
> > > + and the content update is discarded.
> > > +
> > > + Once a presentation_feedback object has delivered an 'presented'
> > > + or 'discarded' event it is automatically destroyed.
> >
> > delivered a 'presented'
>
> Done.
>
> > > + </description>
> > > +
> > > + <event name="sync_output">
> > > + <description summary="presentation synchronized to this output">
> > > + As presentation can be synchronized to only one output at a
> > > + time, this event tells which output it was. This event is only
> > > + sent prior to the presented event.
> > > +
> > > + As clients may bind to the same global wl_output multiple
> > > + times, this event is sent for each bound instance that matches
> > > + the synchronized output. If a client has not bound to the
> > > + right wl_output global at all, this event is not sent.
> > > + </description>
> > > +
> > > + <arg name="output" type="object" interface="wl_output"
> > > + summary="presentation output"/>
> > > + </event>
> > > +
> > > + <enum name="kind">
> > > + <description summary="bitmask of flags in presented event">
> > > + These flags provide information about how the presentation of
> > > + the related content update was done. The intent is to help
> > > + clients assess the reliability of the feedback and the visual
> > > + quality with respect to possible tearing and timings. The
> > > + flags are:
> > > +
> > > + VSYNC:
> > > + The presentation was synchronized to the "vertical retrace" by
> > > + the display hardware such that tearing does not happen.
> > > + Relying on user space scheduling is not acceptable for this
> > > + flag. If presentation is done by a copy to the active
> > > + frontbuffer, then it must guarantee that tearing cannot
> > > + happen.
> > > +
> > > + HW_CLOCK:
> > > + The display hardware provided measurements that the hardware
> > > + driver converted into a presentation timestamp. Sampling a
> > > + clock in user space is not acceptable for this flag.
> > > +
> > > + HW_COMPLETION:
> > > + The display hardware signalled that it started using the new
> > > + image content. The opposite of this is e.g. a timer being used
> > > + to guess when the display hardware has switched to the new
> > > + image content.
> > > +
> > > + ZERO_COPY:
> > > + The presentation of this update was done zero-copy. This means
> > > + the buffer from the client was given to display hardware as
> > > + is, without copying it. Compositing with OpenGL counts as
> > > + copying, even if textured directly from the client buffer.
> > > + Possible zero-copy cases include direct scanout of a
> > > + fullscreen surface and a surface on a hardware overlay.
> > > + </description>
> > > +
> > > + <entry name="vsync" value="0x1" summary="presentation was vsync'd"/>
> > > + <entry name="hw_clock" value="0x2"
> > > + summary="hardware provided the presentation timestamp"/>
> > > + <entry name="hw_completion" value="0x4"
> > > + summary="hardware signalled the start of the presentation"/>
> > > + <entry name="zero_copy" value="0x8"
> > > + summary="presentation was done zero-copy"/>
> >
> > Can <entry>'s include <description> sections? (Looks like the xdg-shell
> > protocol does this for its state enum entries.)
>
> xdg-shell does, but wayland-scanner ignores them. The above at least get into
> the generated comments.
Ah, well that's a shame.
> > > + </enum>
> > > +
> > > + <event name="presented">
> > > + <description summary="the content update was displayed">
> > > + The associated content update was displayed to the user at the
> > > + indicated time (tv_sec_hi/lo, tv_nsec). For the interpretation of
> > > + the timestamp, see presentation.clock_id event.
> > > +
> > > + The timestamp corresponds to the time when the content update
> > > + turned into light the first time on the surface's main output.
> > > + Compositors may approximate this from the framebuffer flip
> > > + completion events from the system, and the latency of the
> > > + physical display path if known.
> > > +
> > > + This event is preceded by all related sync_output events
> > > + telling which output's refresh cycle the feedback corresponds
> > > + to, i.e. the main output for the surface. Compositors are
> > > + recommended to choose the output containing the largest part
> > > + of the wl_surface, or keeping the output they previously
> > > + chose. Having a stable presentation output association helps
> > > + clients predict future output refreshes (vblank).
> > > +
> > > + Argument 'refresh' gives the compositor's prediction of how
> >
> > "The 'refresh' argument gives"
>
> Done.
>
> > > + many nanoseconds after tv_sec, tv_nsec the very next output
> > > + refresh may occur. This is to further aid clients in
> > > + predicting future refreshes, i.e., estimating the timestamps
> > > + targeting the next few vblanks. If such prediction cannot
> > > + usefully be done, the argument is zero.
> > > +
> > > + The 64-bit value combined from seq_hi and seq_lo is the value
> > > + of the output's vertical retrace counter when the content
> > > + update was first scanned out to the display. This value must
> > > + be compatible with the definition of MSC in
> > > + GLX_OML_sync_control specification. Note, that if the display
> > > + path has a non-zero latency, the time instant specified by
> > > + this counter may differ from the timestamp's.
> > > +
> > > + If the output does not have a constant refresh rate, explicit
> > > + video mode switches excluded, then the refresh argument must
> > > + be zero.
> > > +
> > > + If the output does not have a concept of vertical retrace or a
> > > + refresh cycle, or the output device is self-refreshing without
> > > + a way to query the refresh count, then the arguments seq_hi
> > > + and seq_lo must be zero.
> > > + </description>
> > > +
> > > + <arg name="tv_sec_hi" type="uint"
> > > + summary="high 32 bits of the seconds part of the presentation timestamp"/>
> > > + <arg name="tv_sec_lo" type="uint"
> > > + summary="low 32 bits of the seconds part of the presentation timestamp"/>
> > > + <arg name="tv_nsec" type="uint"
> > > + summary="nanoseconds part of the presentation timestamp"/>
> > > + <arg name="refresh" type="uint" summary="nanoseconds till next refresh"/>
> > > + <arg name="seq_hi" type="uint"
> > > + summary="high 32 bits of refresh counter"/>
> > > + <arg name="seq_lo" type="uint"
> > > + summary="low 32 bits of refresh counter"/>
> > > + <arg name="flags" type="uint" summary="combination of 'kind' values"/>
> > > + </event>
> > > +
> > > + <event name="discarded">
> > > + <description summary="the content update was not displayed">
> > > + The content update was never displayed to the user.
> > > + </description>
> > > + </event>
> > > + </interface>
> > > +
> > > +</protocol>
> > > --
> > > 2.4.10
>
>
> Thanks,
> pq
Bryce
More information about the wayland-devel
mailing list