[Spice-devel] [PATCH spice 4/11] server: Add a GStreamer 0.10 MJPEG video encoder and use it by default.

[Francois Gouget]

On Tue, 19 May 2015, Marc-André Lureau wrote:
[...]
> I think this should target at least gstreamer 1.0. Do you have good reasons
> to want 0.10 support?

I need to target CentOS 6.5 which does not have GStreamer 1.0. I have 
started work on adding GStreamer 1.0 support anyway but that's not 
working yet.


> > +    encoder->pipeline = gst_parse_launch_full("appsrc name=src is-live=1
> > ! ffmpegcolorspace ! ffenc_mjpeg name=encoder ! appsink name=sink", NULL,
> > GST_PARSE_FLAG_FATAL_ERRORS, &err);
> >
> 
> Any reason to pick ffmpegcolorspace instead of autoconvert ?

Using autovideoconvert works, I'll use that instead.


> I would rather see an encodebin with a mjpeg profile or caps. However, I am
> not sure it will be possible to adjust bitrate easily. That could be done
> later.

There's also another issue which is that VP8 encoder does inter-frame 
compression. Currently that does not mesh well with neither Spice's 
video_encoder_encode_frame() API which expects to get one compressed 
buffer for each input frame, nor the current gstreamer_encoder 
implementation.

So to get VP8 support we configure the encoder to force it to only issue 
keyframes. But I'm not sure that would be possible through encodebin 
profiles, particularly in an encoder-independent way (even if just 
across VP8 encoder implementations).


> > +    /* We could create a bunch of GstMemory objects, one per line, to avoid
> > +     * copying the raw frame. But this may run into alignment problems and it's
> > +     * unclear that it would be any faster, particularly if we're unable to
> > +     * cache these objects.
> > +     */
>
> It would probably help when encoding fullscreen.

Maybe. It's a GStreamer 1.0 feature so I have not had a deep look into 
it.


[...]
> Although it is much less important than the memcpy on the src, I could
> imagine the memcpy could be removed eventually, perhaps a FIXME is worth
> here.

It probably requires setting up our own buffer allocator so when the 
encoder asks for a buffer we can give it the Spice output buffer. 
That may be making too many assumptions about the encoder though, 
particularly in GStreamer 1.0. I'll add a FIXME though.


-- 
Francois Gouget <fgouget at codeweavers.com>