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

[Pekka Paalanen]

On Tue, 25 Mar 2014 19:47:21 +0100
Mario Kleiner <mario.kleiner.de at gmail.com> wrote:

> 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.

Hi Mario,

nice to hear that. :-)

> 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.

What should happen if we still need to discard queued updates? E.g. if
previous realized presentation time + delta from the next queued update
is already out of reach, maybe due to too small delta or determining the
realized timestamp too late. Should the delta be defined as the minimum
time difference and imply no_skip?

Yeah, this can very well be left for the future in case a use case
comes up, it would not need changes to existing protocol, just
additions AFAICT.

> 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.

Thank you! Very much! :-)
Would you like me to add a reviewed-by tag for you to some patches?

> 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

Aahaha, embarassing. :-D
Obviously I never even compile-tested this with DBG enabled.


Thanks,
pq