[RFCv3.1 weston] WIP protocol: add flags and refresh stream to presentation

[Mario Kleiner]

On 03/07/2014 04:09 PM, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
> This is quick write-up of
> http://cgit.collabora.com/git/user/pq/weston.git/tree/buffer-queue3.txt?h=buffer-queue-spec
>
> How would this idea feel?

Hi Pekka,

looks good to me. Should serve the needs of my type of application.

One possible extension for the queueing flags would be to have a "target 
presentation timestamp is relative to previous queued update" flag. It 
would mean that a given target presentation timestamp is not in absolute 
time, but relative to the realized presentation time of the previous 
queued update. This would allow to retain the relative spacing of 
presented frames for an application that queues a bunch of frames as 
part of an animation, even if some frames target presentation time is 
missed.

E.g., for a movie/animation playing at 30 fps ~ 0.033333 seconds per 
frame, one could queue the start frame of the movie at an absolute 
target time, but then queue all following frames as having a relative 
timestamp of 0.033333 seconds wrt. to presentation of previous frames.

Otoh this adds more complexity, so this is just a thought for a possible 
future extension.

Fwiw, I also took a few hours to go through your RFC v3 patch series and 
"sort of" review patches 6-12 and 15. I'm not familiar enough with 
Weston yet, but i couldn't find obvious bugs and the implementation of 
the bits i do understand looks correct, e.g., the method/implementation 
of picking updates from the queue for presentation, sending presentation 
feedback, the drm/kms stuff etc. To me this looks good so far.

Only patch 9/15 "compositor: set and use presentation clock everywhere " 
seems to have a small bug in the Rasperry Pi backend:
...

  static void
-rpi_output_update_complete(struct rpi_output *output, uint64_t time)
+rpi_output_update_complete(struct rpi_output *output,
+			   const struct timespec *stamp)
  {
-	DBG("frame update complete(%" PRIu64 ")\n", time);
+	DBG("frame update complete(%d.%09d)\n", stamp.tv_sec, stamp.tv_nsec);

--> In DBG() "stamp" is a pointer, so stamp.tv_sec --> stamp->tv_sec

	 rpi_renderer_finish_frame(&output->base);
-	weston_output_finish_frame(&output->base, time);
+	weston_output_finish_frame(&output->base, stamp);
  }

...

Thanks for your nice work on this,
-mario


>
> Thanks,
> pq
> ---
>   protocol/presentation_timing.xml | 66 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 66 insertions(+)
>
> diff --git a/protocol/presentation_timing.xml b/protocol/presentation_timing.xml
> index e191b87..7da735b 100644
> --- a/protocol/presentation_timing.xml
> +++ b/protocol/presentation_timing.xml
> @@ -235,6 +235,24 @@
>              summary="new feedback object"/>
>       </request>
>   
> +    <enum name="queue">
> +      <description summary="queueing flags">
> +        The no-skip flag will cause the compositor to never discard the
> +        flagged update. Instead, it will postpone following updates to the
> +        next output refresh cycle if needed to ensure, that this update
> +        will be on screen for at least one refresh cycle.
> +
> +        Normally, the compositor will try to match the given target timestamp
> +        to the closest refresh cycle, which means that the update may hit
> +        the screen slightly earlier than requested. Not-before flag guarantees
> +        that the update is never shown before the given target time.
> +      </description>
> +
> +      <entry name="no_skip" value="1" summary="never skip this update"/>
> +      <entry name="not_before" value="2"
> +             summary="do not show the update before the target time"/>
> +    </enum>
> +
>       <request name="queue">
>         <description summary="queue the buffer instead of immediate presentation">
>           This request changes the behaviour of the very next
> @@ -265,6 +283,7 @@
>              summary="low 32 bits of the seconds part of the target timestamp"/>
>         <arg name="tv_nsec" type="uint"
>              summary="nanoseconds part of the target timestamp"/>
> +      <arg name="flags" type="uint" summary="queueing flags"/>
>       </request>
>   
>       <request name="discard_queue">
> @@ -288,6 +307,16 @@
>              summary="target surface"/>
>       </request>
>   
> +    <request name="open_refresh_stream">
> +      <description summary="">
> +      </description>
> +
> +      <arg name="surface" type="object" interface="wl_surface"
> +           summary="target surface"/>
> +      <arg name="callback" type="new_id" interface="refresh_stream"
> +           summary="new refresh stream delivering events"/>
> +    </request>
> +
>       <event name="clock_id">
>         <description summary="clock ID for timestamps">
>           This event tells the client, in which clock domain the
> @@ -366,6 +395,19 @@
>              summary="presentation output"/>
>       </event>
>   
> +    <enum name="kind">
> +      <description summary="bitmask of flags in presented event">
> +      </description>
> +
> +      <entry name="vsync" value="1" summary="presentation was vsync'd"/>
> +      <entry name="user_clock" value="2"
> +             summary="timestamp was retrieved by user space reading a clock"/>
> +      <entry name="hw_completion" value="4"
> +             summary="hardware signalled the completion of the presentation"/>
> +      <entry name="copy" value="8"
> +             summary="final presentation was done as a copy, not a flip"/>
> +    </enum>
> +
>       <event name="presented">
>         <description summary="the content update was displayed">
>           The associated content update was displayed to the user at the
> @@ -422,6 +464,7 @@
>              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">
> @@ -431,4 +474,27 @@
>       </event>
>     </interface>
>   
> +  <interface name="refresh_stream" version="1">
> +    <description summary="delivers refresh events">
> +    </description>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="close the refresh stream">
> +      </description>
> +    </request>
> +
> +    <event name="refresh">
> +      <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="seq_hi" type="uint"
> +           summary="high 32 bits of refresh counter"/>
> +      <arg name="seq_lo" type="uint"
> +           summary="low 32 bits of refresh counter"/>
> +    </event>
> +  </interface>
> +
>   </protocol>