[Spice-devel] [spice v10 08/27] server: Add VP8 support to the GStreamer video encoder

[Christophe Fergeau]

On Thu, Mar 03, 2016 at 09:50:26AM -0500, Frediano Ziglio wrote:
> > diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> > index aae47c7..3dfbdfb 100644
> > --- a/server/gstreamer-encoder.c
> > +++ b/server/gstreamer-encoder.c
> > @@ -188,14 +188,70 @@ static void set_appsrc_caps(SpiceGstEncoder *encoder)
> >      g_object_set(G_OBJECT(encoder->appsrc), "caps", encoder->src_caps,
> >      NULL);
> >  }
> >  
> > +static int physical_core_count = 0;
> > +static int get_physical_core_count(void)
> > +{
> > +    if (!physical_core_count) {
> > +#ifdef HAVE_G_GET_NUMPROCESSORS
> > +        physical_core_count = g_get_num_processors();
> > +#endif
> 
> you could use sysconf if g_get_num_processors is not detected.
> 
> > +        if (system("egrep -l '^flags\\b.*: .*\\bht\\b' /proc/cpuinfo
> > >/dev/null 2>&1") == 0) {
> > +            /* Hyperthreading is enabled so divide by two to get the number
> > +             * of physical cores.
> > +             */
> > +            physical_core_count = physical_core_count / 2;
> > +        }
> 
> Or as you are using /proc/cpuinfo detect from it.
> 
> I'm not sure we should consider compatibility with other processors (like ARM).

In general, I don't think we want to silently use as many cores as
possible for cpu intensive work. This will break down as soon as 2 VMs
trigger the streaming code at the same time, they both will be trying to
use all the available physical cores. Even with a single VM, it could
use 4 vcpus and do CPU intensive tasks on some of them, thus the vp8
encoder would fight for these resources.
I understand in some controlled setups you may want to give more threads
to the encoder, but I think doing it automatically is going to cause us
some issues. I'm not sure what is the best way of making this
configurable is though :( Can you split this patch for now so that the
rest of the vp8 support can go in?

Christophe
-------------- 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/20160303/4e6be956/attachment-0001.sig>