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

[Frediano Ziglio]

> 
> On Fri, Sep 09, 2016 at 09:54:17AM -0400, Frediano Ziglio wrote:
> > >  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.
> 
> Avoiding that means exporting something similar to
> red_chunk_iterator_get_data_one_chunk(). With RedChunkIterator, we
> do not really care whether one line fits in one chunk or not. You say
> "by definition", is there anything guaranteeing that? I agree the stack
> usage, and then the memcpy should be fixed.
> 
> Christophe
> 

You are right "by definition" is a bit misleading and wrong.
It's a kind of not written rule.
Lot of code (like image compression for instance but obviously even
driver code) rely on this assumption.
Image handling and memory copies are the most expensive tasks in
spice-server so memcpy is quite a worsening.

Frediano