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

Wed Dec 14 15:34:07 UTC 2016

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.

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).

Cheers,
  toso

>
> >
> > On Wed, Dec 14, 2016 at 04:10:55AM +0100, Francois Gouget wrote:
> > > > Client says that prefers vp8, maybe due hw decoding capability.
> > >
> > > So let's build a proper case for your proposal:
> > >
> > > There are two cases where the codec used to encode the video really
> > > matters to the client:
> > >
> > > 1) Mobile devices such as smartphones and tablets
> > >
> > >    These usually have processors that would struggle to keep up with
> > >    software decoding. Furthermore software decoding is more power
> > >    inefficient and would quickly run down their battery. For these two
> > >    reasons it is important that they be able to get video in a format
> > >    compatible with their hardware decoding capabilities whenever
> > >    possible.
> > >
> > >    I would also note that there is no client for these platforms (that
> > >    I know of), but the Spice protocol should not be an obstacle to
> > >    implementing a good client for these platforms.
> > >
> > > 2) Laptops operating on battery power
> > >
> > >    Laptops usually have processors that are powerful enough to perform
> > >    software decoding without trouble. That may change as new codecs
> > >    are added that require more processing power for decoding although
> > >    if past history is any indicator processors quickly catch up (but
> > >    it may be unwise to bet against a plateauing of CPU speeds).
> > >
> > >    Another factor is that for laptops too hardware decoding is more
> > >    power efficient. So even if they could do software decoding,
> > >    ensuring they get video in a format compatible with their hardware
> > >    capabilities would help increase their battery life.
> > 
> > Power consumption and decoding efficiency, valid reasons.
> > 
> > Each codec might have different overall quality and bandwidth usage and
> > client might prefer one to another because of that too (which might or
> > not be related to decoding efficiency).
> > 
> > > So how can the client get a say in the selection of the video codec
> > > while at the same time letting the server administrator be in charge
> > > too?
> > >
> > > There are two requirements:
> > >
> > > 1) The client must have a way of expressing preferences.
> > >    The current capability based system does not allow that.
> > >
> > > 2) Since the server preferences override those of the client, they
> > >    must be able to give the same priority to multiple codecs so the
> > >    client preferences can be used to break these ties.
> > >
> > >
> > > And now, with the rationale firmly established, we can get into the
> > > implementation details.
> > >
> > >
> > > Server side
> > > -----------
> > >
> > > On the server side the simplest solution is to extend the current
> > > preferences system by adding a rank, as you proposed, so we have a
> > > semi-colon separated list of encoder:codec:rank triplets. The higher the
> > > rank the lower the preference for the codec.
> > >
> > > There are a few details to decide on however:
> > >
> > > 1) What should the highest rank be: zero or one?
> > >
> > >    Under the current system any encoder:codec that is not in the
> > >    preferences list will not be used and for backward compatibility this
> > >    should be preserved.
> > >
> > >    In your proposal an encoder:codec with a rank of 0 is to be disabled
> > >    which is the same as not putting it in the list in the first place.
> > >    This seems redundant and I usually consider redundancy to be bad as
> > >    it typically means unnecessary complexity.
> > 
> > The spice-server API is spice_server_set_video_codecs() and so we
> > consider that any encoder:codec pair that are not present should be
> > disabled.
> > 
> > I would prefer that encoder:codec pair that are not present would
> > receive the default rank/priority value which would mean *not* disabled,
> > but less likely to be used; To disable a encoder:codec, it should be
> > explicit required to be said so.
> > 
> > The reason for that is that application implementing spice-server will
> > need to be changed every time we include a new video-codec capability if
> > the application wants to support it otherwise it will be disabled as
> > its no present in the input string of spice_server_set_video_codecs()
> > 
> > >    Do we have a scenario where we would need to be able to put an
> > >    encoder:codec in the preferences string but still want it to not be
> > >    used?
> > 
> > Internally we use a GArray of encoder/video-codecs and as a detail of
> > implementation, I think it is much better to have a GArray with all
> > video-codecs with their ranks set accordingly.
> > 
> > >
> > >    If not then should the highest rank be 0 rather than 1?
> > 
> > I got a bit confused with "What should the highest rank be: zero or one"
> > I first read I thought you mean s/highest/lowest a 0 < 9 :) - But I'll
> > keep commenting following the nomination you gave.
> > 
> > I think this redundancy is not a bad one so I would go for 0 to
> > disabled, 1 being the default/highest rank.
> > 
> > > 2) What should the lowest rank be?
> > >
> > >    In you proposal you argue for the rank to be a single digit. This
> > >    saves us from having to deal with crazy inputs such as
> > >    "12345678901234567890". However a single digit seems like a pretty
> > >    small space and it's not really much easier to parse than the
> > >    multiple digits case.
> > >
> > >    Should we just say it's any integer we can parse? Or a two digit
> > >    number?
> > 
> > One digit means 10 values. User can use 9 values to set their preference
> > in the host... I would say it is more then enough and very easy to
> > extend if necessary.
> > 
> > >
> > > 3) If one sets the preferences as "gstreamer:h264:2;spice:mjpeg", what
> > >    should the rank of "spice:mjpeg" be?
> > >
> > >    I argue that it should be the lowest rank (i.e. something like 9 or
> > >    99 or whichever value we pick in point 2).
> > 
> > 9 would be higher priority then 2. So, the default should be 1.
> > 
> > > 4) Should wa call this a rank or a priority?
> > 
> > Either is fine but one important point to make is that the value
> > attached to them and how they matter, my proposal is:
> > 
> > 0: disabled
> > 1: default - (lowest priority possible)
> > 2: higher priority then 1
> > ...
> > 9: highest priority.
> > 
> > >    The problem with calling it a rank is that the entry with the highest
> > >    numerical value is the one least likely to be used. This can be
> > >    confusing.
> > 
> > Yeah, I think we both are confused at this point :)
> > 
> > I will use the priority term as it seems easier to understand:
> > - 9 has higher priority then 1
> > - 9 has higher rank then 1
> > 
> > >    As proof that it is confusing, I initially used the "highest rank"
> > >    term for both point 1 (which is about 0 vs. 1) and point 2 (which is
> > >    about the other end: 9, 99 or more).
> > >
> > >    Would calling it a priority be better? That would imply flipping
> > >    things over so 99 actually has a higher priority than 2. Then
> > >    "spice:mjpeg" could get priority 0, i.e. the lowest priority.
> > >
> > >
> > > Client side
> > > -----------
> > >
> > > On the client side what we need is:
> > >
> > > 1) A new Spice message to send the client preferences since the
> > >    capabilities system has no way of ordering capabilities.
> > >
> > > 2) A format to express the client preferences.
> > >
> > > The format could be a simple semi-colon separated list of codecs since
> > > there is no need for ranks on the client side. Or it could be an array
> > > of strings to save on parsing if that easy to handle within the Spice
> > > protocol.
> > 
> > As we already have SpiceVideoCodecType [0] and we only want to state the
> > order of preference, I would go with an array as I did with these enum
> > as value.
> > 
> > [0] https://cgit.freedesktop.org/spice/spice-protocol/tree/spice/enums.h#n147
> > 
> > In the server we will simply sort the existing GArray in a new array. It
> > is also easier to validate too; Using string does not reduce the need of
> > parsing IMHO.
> > 
> > > It could also be some format that includes ranks if that helps us reuse
> > > code. However this would mean exposing implementation details on the
> > > client side which we could get stuck with. So it may be best to avoid.
> > 
> > Yes :) - My first approach was using ranks in the client side but it was
> > bad.
> > 
> > > Can the client send a message expresing its codec preferences to a
> > > server that does not support said message? Does this require adding a
> > > server capability?
> > 
> > It requires adding capability
> > 
> > > Then what should we do with the current video capabilities?
> > >
> > > It seems like we have to keep them for backward compatibility but
> > > presumably we will not add new capabilities for VP9, h265, etc?
> > 
> > The video-codec preference message and capability does not exclude the
> > need of per video-codec capability. We will keep including video-codec
> > capability in server and client.
> > 
> > I think we will be able to keep backward compatibility for everything
> > related to the video-codec preference :)
> > 
> > Cheers,
> >   toso
> 
> 
> 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 

-------------- 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/20161214/aecd2383/attachment-0001.sig>