Forgot to add
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
(with the small fixes you did below).
On Wed, Mar 02, 2016 at 07:46:26PM +0100, Francois Gouget wrote:
> On Wed, 2 Mar 2016, Christophe Fergeau wrote:
> [...]
> > > +if test "x$enable_gstreamer" != "xno"; then
> > > + SPICE_CHECK_GSTREAMER(GSTREAMER_1_0, 1.0, [gstreamer-1.0 gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-video-1.0],
> > > + [enable_gstreamer="yes"
> > > + SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gst-plugins-base 1.0], [appsrc videoconvert appsink])
> > > + SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gstreamer-libav 1.0], [avenc_mjpeg])
> > > + SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gst-plugins-good 1.0], [vp8enc])
> > > + SPICE_CHECK_GSTREAMER_ELEMENTS($GST_INSPECT_1_0, [gst-plugins-ugly 1.0], [x264enc])
> >
> > Maybe the addition of the tests for vp8enc/x264enc should be moved to
> > the respective commits making use of these?
>
> Whoops, I missed those. Moved.
>
>
> > However, I don't think we can go with these compile-time tests and
> > disable gstreamer altogether when they are missing.
>
> SPICE_CHECK_GSTREAMER_ELEMENTS() only issues warnings.
>
>
> [...]
> > > +/* A helper for spice_gst_encoder_encode_frame() */
> > > +static SpiceFormatForGStreamer *map_format(SpiceBitmapFmt format)
> > > +{
> > > + /* See GStreamer's part-mediatype-video-raw.txt and
> > > + * section-types-definitions.html documents.
> > > + */
> > > + static SpiceFormatForGStreamer format_map[] = {
> >
> > Some constness could probably be added here and to the return
> > value.
>
> Makes sense. Done.
>
>
> [...]
> > > +/* A helper for push_raw_frame() */
> > > +static inline int chunk_copy(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap,
> > > + uint32_t chunk_offset, uint32_t len, uint8_t *dst)
> > > +{
> > > + SpiceChunks *chunks = bitmap->data;
> > > + uint32_t chunk_index = 0;
> > > + /* We can copy the bitmap chunk by chunk */
> > > + while (len && chunk_index < chunks->num_chunks) {
> > > + if (is_chunk_padded(bitmap, chunk_index)) {
> > > + return FALSE;
> > > + }
> > > + if (chunk_offset >= chunks->chunk[chunk_index].len) {
> > > + chunk_offset -= chunks->chunk[chunk_index].len;
> > > + chunk_index++;
> > > + continue;
> > > + }
> >
> > I would split the loop here to make it more obvious that we have a first
> > loop skipping chunks we need to ignore, and then a loop doing the copy.
>
> Done.
>
>
>
> --
> Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
> _______________________________________________
> 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