[Spice-devel] [spice v15 00/21] Add GStreamer support for video streaming

[Christophe Fergeau]

Hey,

Not going to answer to each patch as I don't have a lot to add at this
point, it looks mostly good to me.
One thing to fix (I believe) which was introduced in these patches is
"is_chunk_stride_aligned", the name was changed, but its return value
should be inverted too, currently it returns FALSE when it is actually
aligned. See the 'fixup' commits for
'streaming: Add a GStreamer 1.0 MJPEG video encoder and use it by
default' and 'streaming: Avoid copying the input frame in the GStreamer
encoder' in https://cgit.freedesktop.org/~teuf/spice/log/?h=gst

There are also a few more cosmetic changes in there (setting some fields
to NULL in some struct after unref'ing them, and the removal of a xx?a:b
instance)

Apart from this, I'm still really unconvinced about 'streaming: Use the
optimal number of threads for VP8 encoding', maybe I'll keep it out
initially.
I'd also like to ask GStreamer people their thoughts about the problem
you are solving with "streaming: Handle and recover from GStreamer
encoding errors" (I'm not very satisfied with spice-server having to
more or less duplicate what gstappsink is already doing internally). I
will do that on Monday.

Then I'll run this code a bit (haven't done that recently), and we will
be good to go upstream at long long last!

Christophe


On Thu, May 26, 2016 at 05:14:42PM +0200, Francois Gouget wrote:
> 
> This patch series adds support for using GStreamer to encode the video 
> streams, adding support for VP8 and h264 codecs.
> 
> As before the patches can also be grabbed from the gst branch of the 
> repositories below. Note that this time around the required support is 
> already present in the client.
> 
> spice:          https://github.com/fgouget/spice
> xf86-video-qxl: https://github.com/fgouget/xf86-video-qxl
> spice-protocol: https://github.com/fgouget/spice-protocol
> 
> 
> There's also 'extras' branches with more experimental/future patches for 
> the curious, and gst-vNN branches to help comparing the various 
> revisions.
> 
> Note that to test GStreamer support with QEMU you probably need to grab 
> the patch below and set the video-codecs option to something like 
> gstreamer:h264 or gstreamer:vp8.
> 
> https://lists.freedesktop.org/archives/spice-devel/2015-May/019771.html
> 
> 
> Changes from v14:
> 
> * In this round the width / height removal is back because I think it 
>   is relevant to the discussion around the gstreamer-encoder code for 
>   copying the frames.
> 
> * I also split that change in two parts : one removing the use of the
>   width / height parameters in the MJPEG encoder, and another removing 
>   them from the video-encoder API.
> 
> * Added a comment and tweaked the warning in 
>   is_chunk_stride_aligned().
> 
> * Added a comment in line_copy() wrt 0-byte chunks.
> 
> * Renamed max_mem to max_block_count in zero_copy().
> 
> * I also changed the patchset prefix to 'streaming' as suggested by 
>   Pavel Grunt.
> 
> 
> Changes from v13:
> 
> Rebased + some changes suggested by Christophe Fergeau.
> 
> -- 
> 2.8.1
> _______________________________________________
> 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/20160527/b98699b6/attachment.sig>