Skip to content

Commit

Permalink
lower memory use for crop of very large images
Browse files Browse the repository at this point in the history
The skip-ahead code in sequential could trigger unbounded allocations for
large skips on very large images.

See #3936
  • Loading branch information
jcupitt committed Apr 18, 2024
1 parent 4fa7c12 commit 12c0e62
Showing 1 changed file with 23 additions and 19 deletions.
42 changes: 23 additions & 19 deletions libvips/conversion/sequential.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,25 +142,30 @@ vips_sequential_generate(VipsRegion *out_region,
* Probably the operation is something like extract_area and
* we should skip the initial part of the image. In fact,
* we read to cache, since it may be useful.
*
* Read in chunks, since we may be skipping *many* lines of image from
* a file source.
*/
VipsRect area;

/*
printf("vips_sequential_generate %p: skipping to line %d ...\n",
sequential, r->top);
*/

area.left = 0;
area.top = sequential->y_pos;
area.width = 1;
area.height = r->top - sequential->y_pos;
if (vips_region_prepare(ir, &area)) {
sequential->error = -1;
g_mutex_unlock(sequential->lock);
return -1;
int tile_width;
int tile_height;
int n_lines;

vips_get_tile_size(ir->im, &tile_width, &tile_height, &n_lines);
for (int y = sequential->y_pos; y < r->top; y += tile_height) {

This comment has been minimized.

Copy link
@kleisauke

kleisauke Apr 19, 2024

Member

Perhaps we could use sequential->tile_height instead of the vips_get_tile_size() call?

This comment has been minimized.

Copy link
@jcupitt

jcupitt Apr 19, 2024

Author Member

Good idea!

Sorry, I should have made a PR rather than just pushing this.

VipsRect area;

area.left = 0;
area.top = y;
area.width = 1;
area.height = VIPS_MIN(tile_height, r->top - area.top);
if (vips_region_prepare(ir, &area)) {
sequential->error = -1;
g_mutex_unlock(sequential->lock);
return -1;
}

sequential->y_pos += area.height;
}

sequential->y_pos = VIPS_RECT_BOTTOM(&area);
}

/* This is a request for old pixels, or for pixels exactly at the read
Expand All @@ -174,8 +179,7 @@ vips_sequential_generate(VipsRegion *out_region,
return -1;
}

sequential->y_pos =
VIPS_MAX(sequential->y_pos, VIPS_RECT_BOTTOM(r));
sequential->y_pos = VIPS_MAX(sequential->y_pos, VIPS_RECT_BOTTOM(r));

g_mutex_unlock(sequential->lock);

Expand Down

0 comments on commit 12c0e62

Please sign in to comment.