[Spice-devel] [PATCH v5 3/5] gstreamer: Add gst_format to the table of supported formats

[Frediano Ziglio]

> 
> On Tue, Jan 10, 2017 at 02:20:39PM +0000, Frediano Ziglio wrote:
> > This format is required to add metadata.
> > These metadata can be used to pass different strides or additional
> > information to pass dma buffers.
> 
> I don't really understand what you want to do with that additional info
> from the commit log :(
> "Add metadata" ? how does adding another constant help with that ?
> 

Maybe

  This format is required to add metadata using gst_buffer_add_video_meta_full.
  These metadata can be used to pass different strides or additional
  information to pass dma buffers.

> 
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/gstreamer-encoder.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> > index 4a0917e..ba81377 100644
> > --- a/server/gstreamer-encoder.c
> > +++ b/server/gstreamer-encoder.c
> > @@ -26,6 +26,7 @@
> >  #include <gst/gst.h>
> >  #include <gst/app/gstappsrc.h>
> >  #include <gst/app/gstappsink.h>
> > +#include <gst/video/video.h>
> >  
> >  #include "red-common.h"
> >  #include "video-encoder.h"
> > @@ -42,6 +43,7 @@
> >  typedef struct {
> >      SpiceBitmapFmt spice_format;
> >      char format[8];
> > +    GstVideoFormat gst_format;
> 
> If we add this here, it seems most of the following members would be
> redundant and can be filled using gst_video_format_get_info() ?
> 
> Christophe
> 

This structure handle both Gstreamer 0.10 and 1.0, in 0.10
there's no gst_video_format_get_info.
I have another patch (somewhere...) to have this structure
stripped down of unneeded fields depending on GStreamer
version.

Frediano

> >      uint32_t bpp;
> >      uint32_t depth;
> >      uint32_t endianness;
> > @@ -756,12 +758,12 @@ static const SpiceFormatForGStreamer format_map[] =
> > {
> >      /* First item is invalid.
> >       * It's located first so the loop catch invalid values.
> >       */
> > -    {SPICE_BITMAP_FMT_INVALID, "", 0, 0, 0, 0, 0, 0},
> > -    {SPICE_BITMAP_FMT_RGBA, "BGRA", 32, 24, 4321, 0xff000000, 0xff0000,
> > 0xff00},
> > -    {SPICE_BITMAP_FMT_16BIT, "RGB15", 16, 15, 4321, 0x001f, 0x03E0,
> > 0x7C00},
> > +    {SPICE_BITMAP_FMT_INVALID, "", GST_VIDEO_FORMAT_UNKNOWN, 0, 0, 0, 0,
> > 0, 0},
> > +    {SPICE_BITMAP_FMT_RGBA, "BGRA", GST_VIDEO_FORMAT_BGRA, 32, 24, 4321,
> > 0xff000000, 0xff0000, 0xff00},
> > +    {SPICE_BITMAP_FMT_16BIT, "RGB15", GST_VIDEO_FORMAT_RGB15, 16, 15,
> > 4321, 0x001f, 0x03E0, 0x7C00},
> >      /* TODO: Test the other formats under GStreamer 0.10*/
> > -    {SPICE_BITMAP_FMT_32BIT, "BGRx", 32, 24, 4321, 0xff000000, 0xff0000,
> > 0xff00},
> > -    {SPICE_BITMAP_FMT_24BIT, "BGR", 24, 24, 4321, 0xff0000, 0xff00, 0xff},
> > +    {SPICE_BITMAP_FMT_32BIT, "BGRx", GST_VIDEO_FORMAT_BGRx, 32, 24, 4321,
> > 0xff000000, 0xff0000, 0xff00},
> > +    {SPICE_BITMAP_FMT_24BIT, "BGR", GST_VIDEO_FORMAT_BGR, 24, 24, 4321,
> > 0xff0000, 0xff00, 0xff},
> >  };
> >  #define GSTREAMER_FORMAT_INVALID (&format_map[0])
> >