kleopatra999 / webm

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

bad allocation size of y4m buffer when chroma_type==444alpha #847

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
** What is the expected behavior? What do you see instead?
By trying to input a y4m in 444alpha format, the vpxenc crash before giving the 
error message about the unsupported input (for now). (see reason below).

** What version are you using? On what operating system?
last trunk version libvpx
commit 79bb2cddd38452c00b6f2f43c6665f3a25af7b28 
Sat Aug 23 08:06:13 2014

linux ubuntu 12.04, gcc 4.8.1
windows 7, msvc 2010

** Can you reproduce using the vpxdec or vpxenc tools? What command line are
you using?
vpxenc video.y4m  -o video.webm --codec=vp9  --good

** Please provide any additional information below.
Reason: 
- when reading 444alpha y4m [y4m_input_open{y4minput.c:937}]
- dst_buf_read_sz is initialized to 4*image_size (4 components Y+U+V+A)  
[y4m_input_open{y4minput.c:956}]
- later before the allocation, dst_buf_sz is initialied to 3*image size (3 
components Y+U+V) [y4m_input_open{y4minput.c:974}]
- allocation of dst_buf is performed using dst_buf_sz 
[y4m_input_open{y4minput.c:978}]
- later during the reading of the first frame, dst_buf_read_sz bytes are read 
in dst_buf [y4m_input_fetch_frame{y4minput.c:1016}] which will do a read on 
uninitialized bytes in file_read{y4minput.c:28}] and crash the program,

Solution (maybe?):
- by replacing 
_y4m->dst_buf = (unsigned char *)malloc(_y4m->dst_buf_sz);
by
_y4m->dst_buf = (unsigned char *)malloc(_y4m->dst_buf_read_sz);
at 
[y4m_input_open{y4minput.c:978}]
the program finished gently with an error message on unsupported format 
444alpha.

Original issue reported on code.google.com by galpin.f...@gmail.com on 27 Aug 2014 at 2:26

GoogleCodeExporter commented 9 years ago

Original comment by ya...@google.com on 3 Sep 2014 at 3:48

GoogleCodeExporter commented 9 years ago
Thanks, I uploaded your proposed change here: 
https://gerrit.chromium.org/gerrit/#/c/71735/

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

GoogleCodeExporter commented 9 years ago
The fix is now merged.

Original comment by ya...@google.com on 6 Oct 2014 at 4:44

GoogleCodeExporter commented 9 years ago
reverted https://gerrit.chromium.org/gerrit/#/c/71735/ for further 
investigation.

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

GoogleCodeExporter commented 9 years ago
Galpin.franck,

Do you happen to have a yuv444alpha file that we can use to test correct fix 
for this issue?

thanks,
Yaowu

Original comment by ya...@google.com on 8 Oct 2014 at 4:29

GoogleCodeExporter commented 9 years ago
Hi!
I do not a have small file to share right now, so I just created a dummy
one from a fix image (image+depth map in alpha from here
https://developers.google.com/depthmap-metadata/ , using a small script to
create the y4m C444alpha).
You can find the file here
https://drive.google.com/file/d/0B8P8Irnb5CoOaUs2VDhVVHQ2c0E/view?usp=sharing

Use the file like this:
vpxenc seq_444alpha_224x304_30p.y4m  -o video.webm --codec=vp9  --good

Hope that helps.

Original comment by galpin.f...@gmail.com on 9 Oct 2014 at 7:28

GoogleCodeExporter commented 9 years ago
Tested with HEAD, vpxenc correctly reported: 

C:\>vpxenc seq_444alpha_224x304_30p.y4m  -o video.webm --codec=vp9  --good
Pass 1/2 frame    1/0          0B       0 us 0.00 fpm [ETA  unknown] Stream 0: 
Failed to encode frame: Invalid parameter
    Invalid image format. Only YV12, I420, I422, I444 images are supported.

Original comment by ya...@google.com on 9 Oct 2014 at 4:33

GoogleCodeExporter commented 9 years ago
Thanks for providing the test sequence, looks like the issue has been fixed as 
#a007ec0f.

Original comment by ya...@google.com on 9 Oct 2014 at 4:34

GoogleCodeExporter commented 9 years ago
Hi!
I just tested on last HEAD ( #ca2745 ) and issue is still here (program
crash and valgrind report a SCP during the file read even on the small
sequence I provided). Looking at the code, it looks like the allocated
buffer still ignore a possible alpha component (only chroma size is taken
into account).
Hope that helps,

Original comment by galpin.f...@gmail.com on 10 Oct 2014 at 7:55