[Spice-devel] [PATCH 4/7] gstreamer: Use a dummy format to make sure format is always defined.

[Pavel Grunt]

On Thu, 2016-08-11 at 13:38 +0200, Pavel Grunt wrote:
> Hi Frediano,
> 
> ack and one suggestion below
> 
> On Thu, 2016-08-11 at 09:50 +0100, Frediano Ziglio wrote:
> > 
> > This avoid a check for NULL.

There is one more check in spice_gst_encoder_encode_frame()

> > Also will be used to catch invalid values when table will be extended.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/gstreamer-encoder.c | 34 ++++++++++++++++++++--------------
> >  1 file changed, 20 insertions(+), 14 deletions(-)
> > 
> > diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> > index 7376d2b..78d4f74 100644
> > --- a/server/gstreamer-encoder.c
> > +++ b/server/gstreamer-encoder.c
> > @@ -747,31 +747,36 @@ static inline void
> > server_increase_bit_rate(SpiceGstEncoder *encoder,
> >  
> >  /* ---------- GStreamer pipeline ---------- */
> >  
> > +/* See GStreamer's part-mediatype-video-raw.txt and
> > + * section-types-definitions.html documents.
> > + */
> > +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},
> > +    /* TODO: Test the other formats */
> > +    {SPICE_BITMAP_FMT_32BIT, "BGRx", 32, 24, 4321, 0xff000000, 0xff0000,
> > 0xff00},
> > +    {SPICE_BITMAP_FMT_24BIT, "BGR", 24, 24, 4321, 0xff0000, 0xff00, 0xff},
> > +};
> > +#define GSTREAMER_FORMAT_INVALID (&format_map[0])
> we can have
>    #define GSTREAMER_FORMAT_TESTED 2
> 
> > 
> > +
> >  /* A helper for spice_gst_encoder_encode_frame() */
> >  static const SpiceFormatForGStreamer *map_format(SpiceBitmapFmt format)
> >  {
> > -    /* See GStreamer's part-mediatype-video-raw.txt and
> > -     * section-types-definitions.html documents.
> > -     */
> > -    static const SpiceFormatForGStreamer format_map[] =  {
> > -        {SPICE_BITMAP_FMT_RGBA, "BGRA", 32, 24, 4321, 0xff000000, 0xff0000,
> > 0xff00},
> > -        {SPICE_BITMAP_FMT_16BIT, "RGB15", 16, 15, 4321, 0x001f, 0x03E0,
> > 0x7C00},
> > -        /* TODO: Test the other formats */
> > -        {SPICE_BITMAP_FMT_32BIT, "BGRx", 32, 24, 4321, 0xff000000,
> > 0xff0000,
> > 0xff00},
> > -        {SPICE_BITMAP_FMT_24BIT, "BGR", 24, 24, 4321, 0xff0000, 0xff00,
> > 0xff},
> > -    };
> > -
> >      int i;
> >      for (i = 0; i < G_N_ELEMENTS(format_map); i++) {
> >          if (format_map[i].spice_format == format) {
> > -            if (i > 1) {
> > +            if (i > 2) {
> and
>                if (i > GSTREAMER_FORMAT_TESTED)
> > 
> >                  spice_warning("The %d format has not been tested yet",
> > format);
> >              }
> >              return &format_map[i];
> >          }
> >      }
> >  
> > -    return NULL;
> > +    return GSTREAMER_FORMAT_INVALID;
> >  }
> >  
> >  static void set_appsrc_caps(SpiceGstEncoder *encoder)
> > @@ -1669,7 +1674,7 @@ static void spice_gst_encoder_get_stats(VideoEncoder
> > *video_encoder,
> >                                          VideoEncoderStats *stats)
> >  {
> >      SpiceGstEncoder *encoder = (SpiceGstEncoder*)video_encoder;
> > -    uint64_t raw_bit_rate = encoder->width * encoder->height * (encoder-
> > > 
> > > format ? encoder->format->bpp : 0) * get_source_fps(encoder);
> > +    uint64_t raw_bit_rate = encoder->width * encoder->height * encoder-
> > > 
> > > format->bpp * get_source_fps(encoder);
> >  
> >      spice_return_if_fail(stats != NULL);
> >      stats->starting_bit_rate = encoder->starting_bit_rate;
> > @@ -1718,6 +1723,7 @@ VideoEncoder
> > *gstreamer_encoder_new(SpiceVideoCodecType
> > codec_type,
> >      encoder->starting_bit_rate = starting_bit_rate;
> >      encoder->bitmap_ref = bitmap_ref;
> >      encoder->bitmap_unref = bitmap_unref;
> > +    encoder->format = GSTREAMER_FORMAT_INVALID;
> >      g_mutex_init(&encoder->outbuf_mutex);
> >      g_cond_init(&encoder->outbuf_cond);
> >  
> Pavel
>