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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]