Hey,
On Tue, May 02, 2017 at 01:05:45PM +0200, Francois Gouget wrote:
> On Mon, 24 Apr 2017, Christophe Fergeau wrote:
> [...]
> > > + c->streams[op->id] = display_stream_create(channel, op->surface_id, op->flags, op->codec_type);
> > > + if (c->streams[op->id] == NULL) {
> > > + spice_printerr("could not create the %u video stream", op->id);
> > > destroy_stream(channel, op->id);
> > > report_invalid_stream(channel, op->id);
> > > + } else {
> > > + c->streams[op->id]->dest = op->dest;
> > > + c->streams[op->id]->clip = op->clip;
> >
> > Any reason why you initialize everything in display_stream_create()
> > except these SpiceRect/SpiceClip instances? I'd just squash this in:
>
> There's no big reason.
> * Concerning dest, setting it is required for the stream to work so
> it could make sense to set it up in display_stream_create().
> * But one does not need to set an explicit clip region to use the
> stream and as is it defaults to SPICE_CLIP_TYPE_NONE which I think
> makes sense. So I'd leave setting it up to the caller so it is
> optional.
>
> So I'd propose another variant where display_stream_create() takes a
> rect parameter but leaves clip to its default. See the attached patch.
> Feel free to pick any of the three variants.
In the current code, clip is not really optional as it's unconditionally
set after calling display_stream_create(). I'll push with my variant,
and we can revise how this all works and make this optional when/if the
code you alude to gets public :)
Thanks,
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel