kleopatra999 / webm

Automatically exported from code.google.com/p/webm
0 stars 0 forks source link

Segmentation fault for certain image size changes #837

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The change on July 19th:

https://gerrit.chromium.org/gerrit/#/c/70814

changes the allocation of frame buffers to only reallocate if 
aligned_width*aligned_height is greater than the width*height of the allocated 
buffer.  (See resize_context_buffers in vp9/decoder/vp9_decodeframe.c)

However, this causes segmentation faults on certain streams.

The issue seems to be that the amount of memory cleared (in setup_mi of 
vp9/common/vp9_alloccommon.c) is proportional to mi_stride*(mi_rows+1), where 
mi_stride is mi_cols + MI_BLOCK_SIZE.

The amount of memory allocated (in vp9_alloc_context_buffers) is proportional 
to mi_stride*(mi_rows+MI_BLOCK_SIZE).

Now this works fine the first time a frame is allocated (although it does seem 
a bit wasteful that it is allocating more memory than is required), but when 
the frame size changes, the memory is not always sufficient.

For example, for a 80*80 image changing to a 8*800 image the area is the same, 
so no reallocation occurs.
The amount of memory allocated is (80/8+8)*(80/8+8)=324 units,
but the amount of memory used is (8/8+8)*(800/8+1)=909 units, so a segmentation 
fault occurs.

Original issue reported on code.google.com by peter.de...@gmail.com on 11 Aug 2014 at 3:10

GoogleCodeExporter commented 9 years ago

Original comment by agra...@google.com on 12 Aug 2014 at 6:25

GoogleCodeExporter commented 9 years ago
Patch to fix this is in-process:
https://gerrit.chromium.org/gerrit/#/c/71175/

Original comment by agra...@google.com on 13 Aug 2014 at 9:24

GoogleCodeExporter commented 9 years ago
The current fix seems to correctly deal with the mode info allocation, but 
there are also buffers allocated in proportion to width such as above_context.

This means we now get segmentation errors if the area stays the same but the 
width increases.

Original comment by peter.de...@gmail.com on 20 Aug 2014 at 1:16

GoogleCodeExporter commented 9 years ago
Yes, that got lost in the previous patch. New patch in progress:
https://gerrit.chromium.org/gerrit/#/c/71292/

Original comment by agra...@google.com on 21 Aug 2014 at 3:24

GoogleCodeExporter commented 9 years ago
Fix is now merged. 

Original comment by ya...@google.com on 3 Oct 2014 at 3:37