[Spice-devel] [spice v1 2/2] display-channel: Make video-codecs readwrite

[Victor Toso]

Hi,

On Fri, Dec 02, 2016 at 01:09:54PM -0500, Frediano Ziglio wrote:
> > Indeed, it seems that it can be improved but I'm not sure how it should
> > be designed?
> >
> > Nowadays:
> >
> > host calls reds:spice_server_set_video_codecs() that calls
> > reds_on_vc_change() which end up sending
> > RED_WORKER_MESSAGE_SET_VIDEO_CODECS, meaning
> > display_channel_set_video_codecs() (from the first patch!) so stream.c
> > can use it in its helper function to call the best video_codec->create()
> >
> > Bugs me that reds keeps the original GArray and each display-channel
> > refs it (They should weak-reference it from somewhere else, no?)
> >
>
> reds (SpiceServer) has to keep for the possible future DisplayChannels.

Ah, right.

> The RED_WORKER_MESSAGE_SET_VIDEO_CODECS is required to avoid race conditions.
> But as far SpiceServer and DisplayChannel should not care about it.
>
> Why weak reference? They all hold the array, both SpiceServer and
> DisplayChannel.

Ah, it should not be a big deal. I was thinking if host sets a new
video-codecs preference, each display would be using the wrong
video-codecs array till it gets updated.

As WeakRef I mean that at the time the new GArray is set, the old
GArray reference would be NULL and while there is no update, there would
be no stream.

>
> > Let me know if I can help with something here, I don't understand all
> > the design in spice-server.
> >
>
> Nobody does, that's the funny bit!
>
> DisplayChannel is a good entry point for SpiceServer but basically the
> DisplayChannel structure contains every fields needed by DisplayChannel,
> Stream, StreamAgent and DisplayChannelClient, if you looks at the includes
> currently all these objects know the implementation (even private structure)
> of each others. I manage do disentangle image encoding and I tried to start
> with Stream but looks like more harder.

Ah, I see what you mean now.

>
> OT: I'm still wondering if it's a good decision to have *Channel and
> *ChannelClient separate. Basically Channel store a SC state while ChannelClient
> store a Sc state and send a SC-Sc update to the client so surely ChannelClient
> has to know some state of Channel while Channel has to know when specifically
> inform ChannelClient of any change and manage the various updates. Also noting
> that some of the callbacks for Channel are managing ChannelClient requests.
>
> Frediano

Thanks for your feedback and comments :)

  toso
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161205/d26409cd/attachment.sig>