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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel