[Spice-devel] [spice-server 3/3] mjpeg: Use RedChunkIterator

[Frediano Ziglio]

> ---
>  server/mjpeg-encoder.c | 54
>  +++++++++++++++-----------------------------------
>  1 file changed, 16 insertions(+), 38 deletions(-)
> 
> diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c
> index 1649516..eff64f8 100644
> --- a/server/mjpeg-encoder.c
> +++ b/server/mjpeg-encoder.c
> @@ -19,13 +19,17 @@
>  #include <config.h>
>  #endif
>  
> -#include "red-common.h"
> -#include "video-encoder.h"
> -#include "utils.h"
> +#include <stdio.h>
> +
>  #include <jerror.h>
>  #include <jpeglib.h>
>  #include <inttypes.h>
>  
> +#include "red-common.h"
> +#include "red-chunk-iterator.h"
> +#include "utils.h"
> +#include "video-encoder.h"
> +
>  #define MJPEG_MAX_FPS 25
>  #define MJPEG_MIN_FPS 1
>  
> @@ -881,57 +885,31 @@ static 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_frame(MJpegEncoder *encoder, const SpiceRect *src,
>                          const SpiceBitmap *image, int top_down)
>  {
> -    SpiceChunks *chunks;
> +    RedChunkIterator it;
>      uint32_t image_stride;
> -    size_t offset;
> -    int i, chunk;
> +    unsigned int i;
>  
> -    chunks = image->data;
> -    offset = 0;
> -    chunk = 0;
>      image_stride = image->stride;
> +    red_chunk_iterator_init(&it, image->data);
>  
>      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);
> +        red_chunk_iterator_skip_bytes(&it, 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 = get_image_line(chunks, &offset, &chunk,
> image_stride);
> +        uint8_t line_data[image_stride];

Potentially 4gb on the stack.
Beside this I think previously there was no allocation of a line
and no memory copy.
By definition a line have to be contained in a single chunk.

> +        uint8_t *src_line = line_data;
> +        size_t src_length;
>  
> -        if (!src_line) {
> +        src_length = red_chunk_iterator_get_data(&it, src_line,
> image_stride);
> +        if (src_length != image_stride) {
>              return FALSE;
>          }
>  

Frediano