[Spice-devel] [client v2 5/5] streaming: Separate the network code from the display_stream management

[Francois Gouget]

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.


I have a use case here where I need to create a stream before I have 
received the rect and where I don't use clipping at all which is why I 
naturally made these optional. But that code is not public (yet anyway) 
and one could argue that the rect should be known at stream creation 
time. In any case it would be trivial to initialize local variables to 
dummy values and pass them to display_stream_create() so any variant 
would be easy for me to adjust to.

(And I apologize for the delay, my Internet connection broke down last 
week and then I was away for the extended week-end)

-- 
Francois Gouget <fgouget at codeweavers.com>              
-------------- next part --------------
A non-text attachment was scrubbed...
Name: separate.diff
Type: text/x-diff
Size: 7731 bytes
Desc: separate.diff
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170502/2588ab3f/attachment.diff>