[Spice-devel] [PATCH spice 3/11] server: Refactor the Spice server video encoding so alternative implementations can be added.

Marc-André Lureau marcandre.lureau at gmail.com
Tue May 19 06:39:33 PDT 2015


looks good

On Wed, May 13, 2015 at 10:27 PM, Francois Gouget <fgouget at codeweavers.com>
wrote:

> This patch simply replaces the mjpeg_encoder_xxx() call points with a
> more object oriented design.
> ---
>  server/Makefile.am     |   2 +-
>  server/mjpeg_encoder.c | 134 +++++++++++++++++++++++++++++++++++++---
>  server/mjpeg_encoder.h | 114 ----------------------------------
>  server/red_worker.c    | 156
> ++++++++++++++---------------------------------
>  server/video_encoder.h | 162
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 331 insertions(+), 237 deletions(-)
>  delete mode 100644 server/mjpeg_encoder.h
>  create mode 100644 server/video_encoder.h
>
> diff --git a/server/Makefile.am b/server/Makefile.am
> index 89a375c..36b6d45 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -81,7 +81,6 @@ libspice_server_la_SOURCES =                  \
>         main_channel.c                          \
>         main_channel.h                          \
>         mjpeg_encoder.c                         \
> -       mjpeg_encoder.h                         \
>         red_bitmap_utils.h                      \
>         red_channel.c                           \
>         red_channel.h                           \
> @@ -115,6 +114,7 @@ libspice_server_la_SOURCES =                        \
>         spicevmc.c                              \
>         spice_timer_queue.c                     \
>         spice_timer_queue.h                     \
> +       video_encoder.h                         \
>         zlib_encoder.c                          \
>         zlib_encoder.h                          \
>         spice_bitmap_utils.h            \
> diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
> index 9a41ef3..d734520 100644
> --- a/server/mjpeg_encoder.c
> +++ b/server/mjpeg_encoder.c
> @@ -20,7 +20,11 @@
>  #endif
>
>  #include "red_common.h"
> -#include "mjpeg_encoder.h"
> +
> +typedef struct MJpegEncoder MJpegEncoder;
> +#define VIDEO_ENCODER_T MJpegEncoder
> +#include "video_encoder.h"
> +
>  #include <jerror.h>
>  #include <jpeglib.h>
>  #include <inttypes.h>
> @@ -154,6 +158,7 @@ typedef struct MJpegEncoderRateControl {
>  } MJpegEncoderRateControl;
>
>  struct MJpegEncoder {
> +    VideoEncoder base;
>      uint8_t *row;
>      uint32_t row_size;
>      int first_frame;
> @@ -165,7 +170,7 @@ struct MJpegEncoder {
>      void (*pixel_converter)(uint8_t *src, uint8_t *dest);
>
>      MJpegEncoderRateControl rate_control;
> -    MJpegEncoderRateControlCbs cbs;
> +    VideoEncoderRateControlCbs cbs;
>      void *cbs_opaque;
>
>      /* stats */
> @@ -174,6 +179,22 @@ struct MJpegEncoder {
>      uint32_t num_frames;
>  };
>
> +static void mjpeg_encoder_destroy(MJpegEncoder *encoder);
> +static int mjpeg_encoder_encode_frame(MJpegEncoder *encoder,
> +                const SpiceBitmap *bitmap, int width, int height,
> +                const SpiceRect *src, int top_down, uint32_t
> frame_mm_time,
> +                uint8_t **outbuf, size_t *outbuf_size, int *data_size);
> +static void mjpeg_encoder_client_stream_report(MJpegEncoder *encoder,
> +                                        uint32_t num_frames,
> +                                        uint32_t num_drops,
> +                                        uint32_t start_frame_mm_time,
> +                                        uint32_t end_frame_mm_time,
> +                                        int32_t end_frame_delay,
> +                                        uint32_t audio_delay);
> +static void mjpeg_encoder_notify_server_frame_drop(MJpegEncoder *encoder);
> +static uint64_t mjpeg_encoder_get_bit_rate(MJpegEncoder *encoder);
> +static void mjpeg_encoder_get_stats(MJpegEncoder *encoder,
> VideoEncoderStats *stats);
> +
>  static inline void mjpeg_encoder_reset_quality(MJpegEncoder *encoder,
>                                                 int quality_id,
>                                                 uint32_t fps,
> @@ -189,8 +210,9 @@ static inline int rate_control_is_active(MJpegEncoder*
> encoder)
>      return encoder->cbs.get_roundtrip_ms != NULL;
>  }
>
> -MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
> -                                MJpegEncoderRateControlCbs *cbs, void
> *opaque)
> +MJpegEncoder *create_mjpeg_encoder(uint64_t starting_bit_rate,
> +                                   VideoEncoderRateControlCbs *cbs,
> +                                   void *cbs_opaque)
>  {
>      MJpegEncoder *enc;
>
> @@ -198,6 +220,12 @@ MJpegEncoder *mjpeg_encoder_new(uint64_t
> starting_bit_rate,
>
>      enc = spice_new0(MJpegEncoder, 1);
>
> +    enc->base.destroy = &mjpeg_encoder_destroy;
> +    enc->base.encode_frame = &mjpeg_encoder_encode_frame;
> +    enc->base.client_stream_report = &mjpeg_encoder_client_stream_report;
> +    enc->base.notify_server_frame_drop =
> &mjpeg_encoder_notify_server_frame_drop;
> +    enc->base.get_bit_rate = &mjpeg_encoder_get_bit_rate;
> +    enc->base.get_stats = &mjpeg_encoder_get_stats;
>      enc->first_frame = TRUE;
>      enc->rate_control.byte_rate = starting_bit_rate / 8;
>      enc->starting_bit_rate = starting_bit_rate;
> @@ -207,7 +235,7 @@ MJpegEncoder *mjpeg_encoder_new(uint64_t
> starting_bit_rate,
>
>          clock_gettime(CLOCK_MONOTONIC, &time);
>          enc->cbs = *cbs;
> -        enc->cbs_opaque = opaque;
> +        enc->cbs_opaque = cbs_opaque;
>          mjpeg_encoder_reset_quality(enc, MJPEG_QUALITY_SAMPLE_NUM / 2, 5,
> 0);
>          enc->rate_control.during_quality_eval = TRUE;
>          enc->rate_control.quality_eval_data.type =
> MJPEG_QUALITY_EVAL_TYPE_SET;
> @@ -754,7 +782,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
> SpiceBitmapFmt format,
>          interval = (now - rate_control->bit_rate_info.last_frame_time);
>
>          if (interval < (1000*1000*1000) / rate_control->adjusted_fps) {
> -            return MJPEG_ENCODER_FRAME_DROP;
> +            return VIDEO_ENCODER_FRAME_DROP;
>          }
>
>          mjpeg_encoder_adjust_params_to_bit_rate(encoder);
> @@ -802,14 +830,14 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
> SpiceBitmapFmt format,
>          break;
>      default:
>          spice_debug("unsupported format %d", format);
> -        return MJPEG_ENCODER_FRAME_UNSUPPORTED;
> +        return VIDEO_ENCODER_FRAME_UNSUPPORTED;
>      }
>
>      if (encoder->pixel_converter != NULL) {
>          unsigned int stride = width * 3;
>          /* check for integer overflow */
>          if (stride < width) {
> -            return MJPEG_ENCODER_FRAME_UNSUPPORTED;
> +            return VIDEO_ENCODER_FRAME_UNSUPPORTED;
>          }
>          if (encoder->row_size < stride) {
>              encoder->row = spice_realloc(encoder->row, stride);
> @@ -829,7 +857,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
> SpiceBitmapFmt format,
>
>      encoder->num_frames++;
>      encoder->avg_quality += quality;
> -    return MJPEG_ENCODER_FRAME_ENCODE_START;
> +    return VIDEO_ENCODER_FRAME_ENCODE_DONE;
>  }
>
>  int mjpeg_encoder_encode_scanline(MJpegEncoder *encoder, uint8_t
> *src_pixels,
> @@ -888,6 +916,92 @@ size_t mjpeg_encoder_end_frame(MJpegEncoder *encoder)
>      return encoder->rate_control.last_enc_size;
>  }
>
> +static inline uint8_t *get_image_line(SpiceChunks *chunks, size_t *offset,
> +                                          int *chunk_nr, int stride)
> +{
> +    uint8_t *ret;
> +    SpiceChunk *chunk;
> +
> +    chunk = &chunks->chunk[*chunk_nr];
> +
> +    if (*offset == chunk->len) {
> +        if (*chunk_nr == chunks->num_chunks - 1) {
> +            return NULL; /* Last chunk */
> +        }
> +        *offset = 0;
> +        (*chunk_nr)++;
> +        chunk = &chunks->chunk[*chunk_nr];
> +    }
> +
> +    if (chunk->len - *offset < stride) {
> +        spice_warning("bad chunk alignment");
> +        return NULL;
> +    }
> +    ret = chunk->data + *offset;
> +    *offset += stride;
> +    return ret;
> +}
> +
> +
> +
> +static int encode_mjpeg_frame(MJpegEncoder *encoder, const SpiceRect *src,
> +                        const SpiceBitmap *image, int top_down)
> +{
> +    SpiceChunks *chunks;
> +    uint32_t image_stride;
> +    size_t offset;
> +    int i, chunk;
> +
> +    chunks = image->data;
> +    offset = 0;
> +    chunk = 0;
> +    image_stride = image->stride;
> +
> +    const int skip_lines = top_down ? src->top : image->y - (src->bottom
> - 0);
> +    for (i = 0; i < skip_lines; i++) {
> +            get_image_line(chunks, &offset, &chunk, image_stride);
> +    }
> +
> +    const unsigned int stream_height = src->bottom - src->top;
> +    const unsigned int stream_width = src->right - src->left;
> +
> +    for (i = 0; i < stream_height; i++) {
> +        uint8_t *src_line = (uint8_t *)get_image_line(chunks, &offset,
> &chunk, image_stride);
> +
> +        if (!src_line) {
> +            return FALSE;
> +        }
> +
> +        src_line += src->left *
> mjpeg_encoder_get_bytes_per_pixel(encoder);
> +        if (mjpeg_encoder_encode_scanline(encoder, src_line,
> stream_width) == 0)
> +            return FALSE;
> +    }
> +
> +    return TRUE;
> +}
> +
> +int mjpeg_encoder_encode_frame(MJpegEncoder *encoder,
> +                const SpiceBitmap *bitmap, int width, int height,
> +                const SpiceRect *src, int top_down, uint32_t
> frame_mm_time,
> +                uint8_t **outbuf, size_t *outbuf_size, int *data_size)
> +{
> +    int ret;
> +
> +    ret = mjpeg_encoder_start_frame(encoder, bitmap->format,
> +                                    width, height, outbuf, outbuf_size,
> +                                    frame_mm_time);
> +    if (ret != VIDEO_ENCODER_FRAME_ENCODE_DONE)
> +        return ret;
> +
> +    if (!encode_mjpeg_frame(encoder, src, bitmap, top_down))
> +        return VIDEO_ENCODER_FRAME_UNSUPPORTED;
> +
> +    *data_size = mjpeg_encoder_end_frame(encoder);
> +
> +    return VIDEO_ENCODER_FRAME_ENCODE_DONE;
> +}
> +
> +
>  static void mjpeg_encoder_quality_eval_stop(MJpegEncoder *encoder)
>  {
>      MJpegEncoderRateControl *rate_control = &encoder->rate_control;
> @@ -1267,7 +1381,7 @@ uint64_t mjpeg_encoder_get_bit_rate(MJpegEncoder
> *encoder)
>      return encoder->rate_control.byte_rate * 8;
>  }
>
> -void mjpeg_encoder_get_stats(MJpegEncoder *encoder, MJpegEncoderStats
> *stats)
> +void mjpeg_encoder_get_stats(MJpegEncoder *encoder, VideoEncoderStats
> *stats)
>  {
>      spice_assert(encoder != NULL && stats != NULL);
>      stats->starting_bit_rate = encoder->starting_bit_rate;
> diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
> deleted file mode 100644
> index d584b92..0000000
> --- a/server/mjpeg_encoder.h
> +++ /dev/null
> @@ -1,114 +0,0 @@
> -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> -/*
> -   Copyright (C) 2009 Red Hat, Inc.
> -
> -   This library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   This library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with this library; if not, see <
> http://www.gnu.org/licenses/>.
> -*/
> -
> -#ifndef _H_MJPEG_ENCODER
> -#define _H_MJPEG_ENCODER
> -
> -#include "red_common.h"
> -
> -enum {
> -    MJPEG_ENCODER_FRAME_UNSUPPORTED = -1,
> -    MJPEG_ENCODER_FRAME_DROP,
> -    MJPEG_ENCODER_FRAME_ENCODE_START,
> -};
> -
> -typedef struct MJpegEncoder MJpegEncoder;
> -
> -/*
> - * Callbacks required for controling and adjusting
> - * the stream bit rate:
> - * get_roundtrip_ms: roundtrip time in milliseconds
> - * get_source_fps: the input frame rate (#frames per second), i.e.,
> - * the rate of frames arriving from the guest to spice-server,
> - * before any drops.
> - */
> -typedef struct MJpegEncoderRateControlCbs {
> -    uint32_t (*get_roundtrip_ms)(void *opaque);
> -    uint32_t (*get_source_fps)(void *opaque);
> -    void (*update_client_playback_delay)(void *opaque, uint32_t delay_ms);
> -} MJpegEncoderRateControlCbs;
> -
> -typedef struct MJpegEncoderStats {
> -    uint64_t starting_bit_rate;
> -    uint64_t cur_bit_rate;
> -    double avg_quality;
> -} MJpegEncoderStats;
> -
> -MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
> -                                MJpegEncoderRateControlCbs *cbs, void
> *opaque);
> -void mjpeg_encoder_destroy(MJpegEncoder *encoder);
> -
> -uint8_t mjpeg_encoder_get_bytes_per_pixel(MJpegEncoder *encoder);
> -
> -/*
> - * dest must be either NULL or allocated by malloc, since it might be
> freed
> - * during the encoding, if its size is too small.
> - *
> - * return:
> - *  MJPEG_ENCODER_FRAME_UNSUPPORTED : frame cannot be encoded
> - *  MJPEG_ENCODER_FRAME_DROP        : frame should be dropped. This value
> can only be returned
> - *                                    if mjpeg rate control is active.
> - *  MJPEG_ENCODER_FRAME_ENCODE_START: frame encoding started. Continue
> with
> - *                                    mjpeg_encoder_encode_scanline.
> - */
> -int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt
> format,
> -                              int width, int height,
> -                              uint8_t **dest, size_t *dest_len,
> -                              uint32_t frame_mm_time);
> -int mjpeg_encoder_encode_scanline(MJpegEncoder *encoder, uint8_t
> *src_pixels,
> -                                  size_t image_width);
> -size_t mjpeg_encoder_end_frame(MJpegEncoder *encoder);
> -
> -/*
> - * bit rate control
> - */
> -
> -/*
> - * Data that should be periodically obtained from the client. The report
> contains:
> - * num_frames         : the number of frames that reached the client
> during the time
> - *                      the report is referring to.
> - * num_drops          : the part of the above frames that was dropped by
> the client due to
> - *                      late arrival time.
> - * start_frame_mm_time: the mm_time of the first frame included in the
> report
> - * end_frame_mm_time  : the mm_time of the last_frame included in the
> report
> - * end_frame_delay    : (end_frame_mm_time - client_mm_time)
> - * audio delay        : the latency of the audio playback.
> - *                      If there is no audio playback, set it to MAX_UINT.
> - *
> - */
> -void mjpeg_encoder_client_stream_report(MJpegEncoder *encoder,
> -                                        uint32_t num_frames,
> -                                        uint32_t num_drops,
> -                                        uint32_t start_frame_mm_time,
> -                                        uint32_t end_frame_mm_time,
> -                                        int32_t end_frame_delay,
> -                                        uint32_t audio_delay);
> -
> -/*
> - * Notify the encoder each time a frame is dropped due to pipe
> - * congestion.
> - * We can deduce the client state by the frame dropping rate in the
> server.
> - * Monitoring the frame drops can help in fine tuning the playback
> parameters
> - * when the client reports are delayed.
> - */
> -void mjpeg_encoder_notify_server_frame_drop(MJpegEncoder *encoder);
> -
> -uint64_t mjpeg_encoder_get_bit_rate(MJpegEncoder *encoder);
> -void mjpeg_encoder_get_stats(MJpegEncoder *encoder, MJpegEncoderStats
> *stats);
> -
> -#endif
> diff --git a/server/red_worker.c b/server/red_worker.c
> index e0ff8e9..c6b39cb 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -70,7 +70,7 @@
>  #include "glz_encoder.h"
>  #include "stat.h"
>  #include "reds.h"
> -#include "mjpeg_encoder.h"
> +#include "video_encoder.h"
>  #include "red_memslots.h"
>  #include "red_parse_qxl.h"
>  #include "jpeg_encoder.h"
> @@ -484,7 +484,7 @@ typedef struct StreamAgent {
>      PipeItem destroy_item;
>      Stream *stream;
>      uint64_t last_send_time;
> -    MJpegEncoder *mjpeg_encoder;
> +    VideoEncoder *video_encoder;
>      DisplayChannelClient *dcc;
>
>      int frames;
> @@ -723,7 +723,7 @@ struct DisplayChannelClient {
>      QRegion surface_client_lossy_region[NUM_SURFACES];
>
>      StreamAgent stream_agents[NUM_STREAMS];
> -    int use_mjpeg_encoder_rate_control;
> +    int use_video_encoder_rate_control;
>      uint32_t streams_max_latency;
>      uint64_t streams_max_bit_rate;
>  };
> @@ -2642,10 +2642,10 @@ static void
> red_print_stream_stats(DisplayChannelClient *dcc, StreamAgent *agent
>  #ifdef STREAM_STATS
>      StreamStats *stats = &agent->stats;
>      double passed_mm_time = (stats->end - stats->start) / 1000.0;
> -    MJpegEncoderStats encoder_stats = {0};
> +    VideoEncoderStats encoder_stats = {0};
>
> -    if (agent->mjpeg_encoder) {
> -        mjpeg_encoder_get_stats(agent->mjpeg_encoder, &encoder_stats);
> +    if (agent->video_encoder) {
> +        agent->video_encoder->get_stats(agent->video_encoder,
> &encoder_stats);
>      }
>
>      spice_debug("stream=%"PRIdPTR" dim=(%dx%d) #in-frames=%"PRIu64"
> #in-avg-fps=%.2f #out-frames=%"PRIu64" "
> @@ -2688,8 +2688,8 @@ static void red_stop_stream(RedWorker *worker,
> Stream *stream)
>          region_clear(&stream_agent->vis_region);
>          region_clear(&stream_agent->clip);
>          spice_assert(!pipe_item_is_linked(&stream_agent->destroy_item));
> -        if (stream_agent->mjpeg_encoder &&
> dcc->use_mjpeg_encoder_rate_control) {
> -            uint64_t stream_bit_rate =
> mjpeg_encoder_get_bit_rate(stream_agent->mjpeg_encoder);
> +        if (stream_agent->video_encoder &&
> dcc->use_video_encoder_rate_control) {
> +            uint64_t stream_bit_rate =
> stream_agent->video_encoder->get_bit_rate(stream_agent->video_encoder);
>
>              if (stream_bit_rate > dcc->streams_max_bit_rate) {
>                  spice_debug("old max-bit-rate=%.2f new=%.2f",
> @@ -2996,7 +2996,7 @@ static uint64_t
> red_stream_get_initial_bit_rate(DisplayChannelClient *dcc,
>             stream->width * stream->height) /
> dcc->common.worker->streams_size_total;
>  }
>
> -static uint32_t red_stream_mjpeg_encoder_get_roundtrip(void *opaque)
> +static uint32_t red_stream_video_encoder_get_roundtrip(void *opaque)
>  {
>      StreamAgent *agent = opaque;
>      int roundtrip;
> @@ -3017,7 +3017,7 @@ static uint32_t
> red_stream_mjpeg_encoder_get_roundtrip(void *opaque)
>      return roundtrip;
>  }
>
> -static uint32_t red_stream_mjpeg_encoder_get_source_fps(void *opaque)
> +static uint32_t red_stream_video_encoder_get_source_fps(void *opaque)
>  {
>      StreamAgent *agent = opaque;
>
> @@ -3040,7 +3040,7 @@ static void
> red_display_update_streams_max_latency(DisplayChannelClient *dcc, St
>      }
>      for (i = 0; i < NUM_STREAMS; i++) {
>          StreamAgent *other_agent = &dcc->stream_agents[i];
> -        if (other_agent == remove_agent || !other_agent->mjpeg_encoder) {
> +        if (other_agent == remove_agent || !other_agent->video_encoder) {
>              continue;
>          }
>          if (other_agent->client_required_latency > new_max_latency) {
> @@ -3053,9 +3053,9 @@ static void
> red_display_update_streams_max_latency(DisplayChannelClient *dcc, St
>  static void red_display_stream_agent_stop(DisplayChannelClient *dcc,
> StreamAgent *agent)
>  {
>      red_display_update_streams_max_latency(dcc, agent);
> -    if (agent->mjpeg_encoder) {
> -        mjpeg_encoder_destroy(agent->mjpeg_encoder);
> -        agent->mjpeg_encoder = NULL;
> +    if (agent->video_encoder) {
> +        agent->video_encoder->destroy(agent->video_encoder);
> +        agent->video_encoder = NULL;
>      }
>  }
>
> @@ -3091,18 +3091,18 @@ static void
> red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
>      agent->fps = MAX_FPS;
>      agent->dcc = dcc;
>
> -    if (dcc->use_mjpeg_encoder_rate_control) {
> -        MJpegEncoderRateControlCbs mjpeg_cbs;
> +    if (dcc->use_video_encoder_rate_control) {
> +        VideoEncoderRateControlCbs video_cbs;
>          uint64_t initial_bit_rate;
>
> -        mjpeg_cbs.get_roundtrip_ms =
> red_stream_mjpeg_encoder_get_roundtrip;
> -        mjpeg_cbs.get_source_fps =
> red_stream_mjpeg_encoder_get_source_fps;
> -        mjpeg_cbs.update_client_playback_delay =
> red_stream_update_client_playback_latency;
> +        video_cbs.get_roundtrip_ms =
> red_stream_video_encoder_get_roundtrip;
> +        video_cbs.get_source_fps =
> red_stream_video_encoder_get_source_fps;
> +        video_cbs.update_client_playback_delay =
> red_stream_update_client_playback_latency;
>
>          initial_bit_rate = red_stream_get_initial_bit_rate(dcc, stream);
> -        agent->mjpeg_encoder = mjpeg_encoder_new(initial_bit_rate,
> &mjpeg_cbs, agent);
> +        agent->video_encoder = create_mjpeg_encoder(initial_bit_rate,
> &video_cbs, agent);
>      } else {
> -        agent->mjpeg_encoder = mjpeg_encoder_new(0, NULL, NULL);
> +        agent->video_encoder = create_mjpeg_encoder(0, NULL, NULL);
>      }
>      red_channel_client_pipe_add(&dcc->common.base, &agent->create_item);
>
> @@ -3209,7 +3209,7 @@ static void
> red_display_client_init_streams(DisplayChannelClient *dcc)
>          red_channel_pipe_item_init(channel, &agent->create_item,
> PIPE_ITEM_TYPE_STREAM_CREATE);
>          red_channel_pipe_item_init(channel, &agent->destroy_item,
> PIPE_ITEM_TYPE_STREAM_DESTROY);
>      }
> -    dcc->use_mjpeg_encoder_rate_control =
> +    dcc->use_video_encoder_rate_control =
>          red_channel_client_test_remote_cap(&dcc->common.base,
> SPICE_DISPLAY_CAP_STREAM_REPORT);
>  }
>
> @@ -3221,9 +3221,9 @@ static void
> red_display_destroy_streams_agents(DisplayChannelClient *dcc)
>          StreamAgent *agent = &dcc->stream_agents[i];
>          region_destroy(&agent->vis_region);
>          region_destroy(&agent->clip);
> -        if (agent->mjpeg_encoder) {
> -            mjpeg_encoder_destroy(agent->mjpeg_encoder);
> -            agent->mjpeg_encoder = NULL;
> +        if (agent->video_encoder) {
> +            agent->video_encoder->destroy(agent->video_encoder);
> +            agent->video_encoder = NULL;
>          }
>      }
>  }
> @@ -3350,7 +3350,7 @@ static inline void pre_stream_item_swap(RedWorker
> *worker, Stream *stream, Drawa
>          dcc = dpi->dcc;
>          agent = &dcc->stream_agents[index];
>
> -        if (!dcc->use_mjpeg_encoder_rate_control &&
> +        if (!dcc->use_video_encoder_rate_control &&
>              !dcc->common.is_low_bandwidth) {
>              continue;
>          }
> @@ -3359,8 +3359,8 @@ static inline void pre_stream_item_swap(RedWorker
> *worker, Stream *stream, Drawa
>  #ifdef STREAM_STATS
>              agent->stats.num_drops_pipe++;
>  #endif
> -            if (dcc->use_mjpeg_encoder_rate_control) {
> -
> mjpeg_encoder_notify_server_frame_drop(agent->mjpeg_encoder);
> +            if (dcc->use_video_encoder_rate_control) {
> +
> agent->video_encoder->notify_server_frame_drop(agent->video_encoder);
>              } else {
>                  ++agent->drops;
>              }
> @@ -3373,7 +3373,7 @@ static inline void pre_stream_item_swap(RedWorker
> *worker, Stream *stream, Drawa
>
>          agent = &dcc->stream_agents[index];
>
> -        if (dcc->use_mjpeg_encoder_rate_control) {
> +        if (dcc->use_video_encoder_rate_control) {
>              continue;
>          }
>          if (agent->frames / agent->fps < FPS_TEST_INTERVAL) {
> @@ -8481,70 +8481,6 @@ static inline void
> display_begin_send_message(RedChannelClient *rcc)
>      red_channel_client_begin_send_message(rcc);
>  }
>
> -static inline uint8_t *red_get_image_line(SpiceChunks *chunks, size_t
> *offset,
> -                                          int *chunk_nr, int stride)
> -{
> -    uint8_t *ret;
> -    SpiceChunk *chunk;
> -
> -    chunk = &chunks->chunk[*chunk_nr];
> -
> -    if (*offset == chunk->len) {
> -        if (*chunk_nr == chunks->num_chunks - 1) {
> -            return NULL; /* Last chunk */
> -        }
> -        *offset = 0;
> -        (*chunk_nr)++;
> -        chunk = &chunks->chunk[*chunk_nr];
> -    }
> -
> -    if (chunk->len - *offset < stride) {
> -        spice_warning("bad chunk alignment");
> -        return NULL;
> -    }
> -    ret = chunk->data + *offset;
> -    *offset += stride;
> -    return ret;
> -}
> -
> -static int encode_frame(DisplayChannelClient *dcc, const SpiceRect *src,
> -                        const SpiceBitmap *image, Stream *stream)
> -{
> -    SpiceChunks *chunks;
> -    uint32_t image_stride;
> -    size_t offset;
> -    int i, chunk;
> -    StreamAgent *agent = &dcc->stream_agents[stream -
> dcc->common.worker->streams_buf];
> -
> -    chunks = image->data;
> -    offset = 0;
> -    chunk = 0;
> -    image_stride = image->stride;
> -
> -    const int skip_lines = stream->top_down ? src->top : image->y -
> (src->bottom - 0);
> -    for (i = 0; i < skip_lines; i++) {
> -        red_get_image_line(chunks, &offset, &chunk, image_stride);
> -    }
> -
> -    const unsigned int stream_height = src->bottom - src->top;
> -    const unsigned int stream_width = src->right - src->left;
> -
> -    for (i = 0; i < stream_height; i++) {
> -        uint8_t *src_line =
> -            (uint8_t *)red_get_image_line(chunks, &offset, &chunk,
> image_stride);
> -
> -        if (!src_line) {
> -            return FALSE;
> -        }
> -
> -        src_line += src->left *
> mjpeg_encoder_get_bytes_per_pixel(agent->mjpeg_encoder);
> -        if (mjpeg_encoder_encode_scanline(agent->mjpeg_encoder, src_line,
> stream_width) == 0)
> -            return FALSE;
> -    }
> -
> -    return TRUE;
> -}
> -
>  static inline int red_marshall_stream_data(RedChannelClient *rcc,
>                    SpiceMarshaller *base_marshaller, Drawable *drawable)
>  {
> @@ -8589,7 +8525,7 @@ static inline int
> red_marshall_stream_data(RedChannelClient *rcc,
>      uint64_t time_now = red_now();
>      size_t outbuf_size;
>
> -    if (!dcc->use_mjpeg_encoder_rate_control) {
> +    if (!dcc->use_video_encoder_rate_control) {
>          if (time_now - agent->last_send_time < (1000 * 1000 * 1000) /
> agent->fps) {
>              agent->frames--;
>  #ifdef STREAM_STATS
> @@ -8603,33 +8539,29 @@ static inline int
> red_marshall_stream_data(RedChannelClient *rcc,
>      frame_mm_time =  drawable->red_drawable->mm_time ?
>                          drawable->red_drawable->mm_time :
>                          reds_get_mm_time();
> +
>      outbuf_size = dcc->send_data.stream_outbuf_size;
> -    ret = mjpeg_encoder_start_frame(agent->mjpeg_encoder,
> image->u.bitmap.format,
> -                                    width, height,
> -                                    &dcc->send_data.stream_outbuf,
> -                                    &outbuf_size,
> -                                    frame_mm_time);
> +    ret = agent->video_encoder->encode_frame(agent->video_encoder,
> +            &image->u.bitmap, width, height,
> +            &drawable->red_drawable->u.copy.src_area,
> +            stream->top_down, frame_mm_time,
> +            &dcc->send_data.stream_outbuf, &outbuf_size, &n);
> +
>      switch (ret) {
> -    case MJPEG_ENCODER_FRAME_DROP:
> -        spice_assert(dcc->use_mjpeg_encoder_rate_control);
> +    case VIDEO_ENCODER_FRAME_DROP:
> +        spice_assert(dcc->use_video_encoder_rate_control);
>  #ifdef STREAM_STATS
>          agent->stats.num_drops_fps++;
>  #endif
>          return TRUE;
> -    case MJPEG_ENCODER_FRAME_UNSUPPORTED:
> +    case VIDEO_ENCODER_FRAME_UNSUPPORTED:
>          return FALSE;
> -    case MJPEG_ENCODER_FRAME_ENCODE_START:
> +    case VIDEO_ENCODER_FRAME_ENCODE_DONE:
>          break;
>      default:
> -        spice_error("bad return value (%d) from
> mjpeg_encoder_start_frame", ret);
> -        return FALSE;
> -    }
> -
> -    if (!encode_frame(dcc, &drawable->red_drawable->u.copy.src_area,
> -                      &image->u.bitmap, stream)) {
> +        spice_error("bad return value (%d) from
> VideoEncoder::encode_frame", ret);
>          return FALSE;
>      }
> -    n = mjpeg_encoder_end_frame(agent->mjpeg_encoder);
>      dcc->send_data.stream_outbuf_size = outbuf_size;
>
>      if (!drawable->sized_stream) {
> @@ -10255,7 +10187,7 @@ static int
> display_channel_handle_stream_report(DisplayChannelClient *dcc,
>          return FALSE;
>      }
>      stream_agent = &dcc->stream_agents[stream_report->stream_id];
> -    if (!stream_agent->mjpeg_encoder) {
> +    if (!stream_agent->video_encoder) {
>          spice_info("stream_report: no encoder for stream id %u."
>                      "Probably the stream has been destroyed",
> stream_report->stream_id);
>          return TRUE;
> @@ -10266,7 +10198,7 @@ static int
> display_channel_handle_stream_report(DisplayChannelClient *dcc,
>                        stream_agent->report_id, stream_report->unique_id);
>          return TRUE;
>      }
> -    mjpeg_encoder_client_stream_report(stream_agent->mjpeg_encoder,
> +
> stream_agent->video_encoder->client_stream_report(stream_agent->video_encoder,
>                                         stream_report->num_frames,
>                                         stream_report->num_drops,
>                                         stream_report->start_frame_mm_time,
> diff --git a/server/video_encoder.h b/server/video_encoder.h
> new file mode 100644
> index 0000000..8a9f9c6
> --- /dev/null
> +++ b/server/video_encoder.h
> @@ -0,0 +1,162 @@
> +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> +/*
> +   Copyright (C) 2009 Red Hat, Inc.
> +   Copyright (C) 2015 Jeremy White
> +   Copyright (C) 2015 Francois Gouget
> +
> +   This library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   This library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with this library; if not, see <<A HREF="
> http://www.gnu.org/licenses/">http://www.gnu.org/licenses/</A>>.
> +*/
> +
> +#ifndef _H_video_encoder
> +#define _H_video_encoder
> +
> +#include "red_common.h"
> +
> +enum {
> +    VIDEO_ENCODER_FRAME_UNSUPPORTED = -1,
> +    VIDEO_ENCODER_FRAME_DROP,
> +    VIDEO_ENCODER_FRAME_ENCODE_DONE,
> +};
> +
> +typedef struct VideoEncoderStats {
> +    uint64_t starting_bit_rate;
> +    uint64_t cur_bit_rate;
> +    double avg_quality;
> +} VideoEncoderStats;
> +
> +typedef struct VideoEncoder VideoEncoder;
> +#ifndef VIDEO_ENCODER_T
> +# define VIDEO_ENCODER_T VideoEncoder
> +#endif
> +
> +struct VideoEncoder {
> +    /* Releases the video encoder's resources */
> +    void (*destroy)(VIDEO_ENCODER_T *encoder);
> +
> +    /* Compresses the specified src image area into the outbuf buffer.
> +     *
> +     * @encoder:   The video encoder.
> +     * @bitmap:    The Spice screen.
> +     * @width:     The width of the Spice screen.
> +     * @height:    The heigth of the Spice screen.
>

It's not the screen, it's the video area. It's not clear to me what would
happen if the "src" size doesn't match the width&height.

+     * @src:       A rectangle specifying the area occupied by the video.
> +     * @top_down:  If true the first video line is specified by src.top.
> +     * @outbuf:    The buffer for the compressed frame. This must either
> be
> +     *             NULL or point to a buffer allocated by malloc since it
> may be
> +     *             reallocated, if its size is too small.
> +     * @outbuf_size: The size of the outbuf buffer.
> +     * @data_size: The size of the compressed frame.
> +     * @return:
> +     *     VIDEO_ENCODER_FRAME_ENCODE_DONE if successful.
> +     *     VIDEO_ENCODER_FRAME_UNSUPPORTED if the frame cannot be encoded.
> +     *     VIDEO_ENCODER_FRAME_DROP if the frame was dropped. This value
> can
> +     *                              only hapen if rate control is active.
> +     */
> +    int (*encode_frame)(VIDEO_ENCODER_T *encoder, const SpiceBitmap
> *bitmap,
> +                        int width, int height, const SpiceRect *src,
> +                        int top_down, uint32_t frame_mm_time,uint8_t
> **outbuf,
> +                        size_t *outbuf_size, int *data_size);
> +
> +    /*
> +     * Bit rate control methods.
> +     */
> +
> +    /* When rate control is active statistics are periodically obtained
> from
> +     * the client and sent to the video encoder through this method.
> +     *
> +     * @encoder:    The video encoder.
> +     * @num_frames: The number of frames that reached the client during
> the
> +     *              time period the report is referring to.
> +     * @num_drops:  The part of the above frames that was dropped by the
> client
> +     *              due to late arrival time.
> +     * @start_frame_mm_time: The mm_time of the first frame included in
> the
> +     *              report.
> +     * @end_frame_mm_time: The mm_time of the last frame included in the
> report.
> +     * @end_frame_delay: (end_frame_mm_time - client_mm_time)
> +     * @audio delay: The latency of the audio playback. If there is no
> audio
> +     *              playback this is MAX_UINT.
> +     *
> +     */
> +    void (*client_stream_report)(VIDEO_ENCODER_T *encoder,
> +                                 uint32_t num_frames, uint32_t num_drops,
> +                                 uint32_t start_frame_mm_time,
> +                                 uint32_t end_frame_mm_time,
> +                                 int32_t end_frame_delay, uint32_t
> audio_delay);
> +
> +    /* This notifies the video encoder each time a frame is dropped due
> to pipe
> +     * congestion.
> +     *
> +     * We can deduce the client state by the frame dropping rate in the
> server.
> +     * Monitoring the frame drops can help in fine tuning the playback
> +     * parameters when the client reports are delayed.
> +     *
> +     * @encoder:    The video encoder.
> +     */
> +    void (*notify_server_frame_drop)(VIDEO_ENCODER_T *encoder);
> +
> +    /* This queries the video encoder's current bit rate.
> +     *
> +     * @encoder:    The video encoder.
> +     * @return:     The current bit rate in bits per second.
> +     */
> +    uint64_t (*get_bit_rate)(VIDEO_ENCODER_T *encoder);
> +
> +    /* Collects video statistics.
> +     *
> +     * @encoder:    The video encoder.
> +     * @stats:      A VideoEncoderStats structure to fill with the
> collected
> +     *              statistics.
> +     */
> +    void (*get_stats)(VIDEO_ENCODER_T *encoder, VideoEncoderStats *stats);
> +};
> +
> +
> +/* When rate control is active the video encoder can use these callbacks
> to
> + * figure out how to adjust the stream bit rate and adjust some stream
> + * parameters.
> + */
> +typedef struct VideoEncoderRateControlCbs {
> +    /* Returns the stream's estimated roundtrip time in milliseconds. */
> +    uint32_t (*get_roundtrip_ms)(void *opaque);
> +
> +    /* Returns the estimated input frame rate.
> +     *
> +     * This is the number of frames per second arriving from the guest to
> +     * spice-server, before any drops.
> +     */
> +    uint32_t (*get_source_fps)(void *opaque);
> +
> +    /* Informs the client of the minimum playback delay.
> +     *
> +     * @delay_ms:   The minimum number of milliseconds required for the
> frames
> +     *              to reach the client.
> +     */
> +    void (*update_client_playback_delay)(void *opaque, uint32_t delay_ms);
> +} VideoEncoderRateControlCbs;
> +
> +
> +/* Instantiates the builtin MJPEG video encoder.
> + *
> + * @starting_bit_rate: An initial estimate of the available stream bit
> rate .
> + * @bit_rate_control:  True if the client supports rate control.
> + * @cbs:               A set of callback methods to be used for rate
> control.
> + * @cbs_opaque:        A pointer to be passed to the rate control
> callbacks.
> + * @return:            A pointer to a structure implementing the
> VideoEncoder
> + *                     methods.
> + */
> +VIDEO_ENCODER_T* create_mjpeg_encoder(uint64_t starting_bit_rate,
> +                                      VideoEncoderRateControlCbs *cbs,
> +                                      void *cbs_opaque);
> +
> +#endif
> --
> 2.1.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150519/f9bca155/attachment-0001.html>


More information about the Spice-devel mailing list