[Spice-devel] [spice-server v2 8/9] reds: add support to ranks for video codecs

[Victor Toso]

Hi,

On Wed, Dec 14, 2016 at 05:23:28PM +0100, Christophe Fergeau wrote:
> On Wed, Dec 14, 2016 at 04:34:07PM +0100, Victor Toso wrote:
> > On Wed, Dec 14, 2016 at 02:32:09PM +0100, Christophe Fergeau wrote:
> > > On Wed, Dec 14, 2016 at 08:53:49AM +0100, Victor Toso wrote:
> > > > Hi,
> > > >
> > > > You rock. I'll use this as reference for future proposals, many thanks!
> > >
> > > While this is polished, I'd say this patch is not directly related to
> > > the 'preferred-video-codec' series? Ie we could at first not have a way
> > > for the server admin to say "these various codecs have the same
> > > priority", and only use a strictly ordered list of codecs for now?
> > >
> > > Christophe
> >
> > If we introduce the client side preferred video codec without the
> > priority patches, the preferred video codec for the client would be
> > always picked and the main issue with that is host can't do anything
> > besides disabling video codecs it does not want.
>
> Hmm true that depending on how you consider the current codec list
> server side (either soft preference, in which case the client preferred
> codec will always be picked, or strong preference in which case the
> client preferred codec will never be used)

Yes

>
> >
> > If we split this in two:
> > 1) Introduce priority
> > 2) Introduce preferred-video-codec
> >
> > I would think it is okay as (2) would not mess with (1). But we are only
> > one patch short in doing that here [0] - but it was recommended to send
> > them together as the need for changes would make more sense.
> >
> > [0] 3 patches, starting at:
> > https://lists.freedesktop.org/archives/spice-devel/2016-December/034445.html
> >
> > I would oppose in doing (2) before (1).
>
> My main problem with introducing the priority now is that we have
> potential uses for it in the future, but I don't think we have users for
> it right now. So maybe it will be the right thing to do when these
> potential uses materialize, maybe we will want to approach things
> differently.

I agree that we might want to handle things differently in the future
but I don't see why this would limit it to have it now and improve it
later.

The concept of priority for video-codec is not new in spice code as the
order they were set in spice_server_set_video_codecs() define their
priority.

What is new is the concept of client-side saying their video-codecs
preference. So we need to handle this in spice-server appropriately. In
the future, with different needs we might want to change how a
video-codec is picked to do the stream.

> Given the discussion about rank VS priority, 0 meaning
> disabled or being redundant, ..., I was suggesting keeping it on the
> backburner for now so that we can merge the rest of the patches which
> seem more straightforward.

I would prefer trying to discuss improvements to be made... This two
examples are quite minor IMHO.

Also, I don't see what else can be changed (as interface) besides
enabled/disabled and order:
- If admin sets some priority in host, we would need to follow anyway
- If admin sets nothing, that would mean 'default' which we could
  improve in the future in case of hardware encoding capabilities

Cheers,
  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/20161220/2dc5c306/attachment.sig>