Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#3014 closed defect (invalid)

sws_scale() renders a frame buffer invalid for memory freeing in a specific situation

Reported by: cyril Owned by:
Priority: normal Component: swscale
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
After reading small jpegs and then converting them to the BGRA color space using sws_scale(), the resulting frame buffer is corrupted and freeing memory taken by the buffer lead to a debug error.

How to reproduce:
1/ Under Windows 7 and Visual Studio 2010, create a C++ Win32 console app called JpegTest (I'm using VS Pro but it should be ok with VS Express).
2/ Copy-paste the attached JpegTest.cpp within the resulting solution.
3/ Compile the latest FFmpeg to obtain .lib and .dll (or download the latest Zeranoe dev and shared zip)
4/ Configure the solution so it can compile with FFmpeg by configuring the lib and include path (Project > Properties > VC++ Directories).
5/ Don't forget to put the attached inttypes.h in the include path.
6/ Put the dlls in the generated Debug folder
7/ Change JpegTest.cpp so that it properly refers to the root_banner.jpg file.
8/ Launch in debug mode, during execution, you should get a debug breakpoint because of heap corruptions.

Attachments (6)

JpegTest.cpp (2.0 KB ) - added by cyril 11 years ago.
inttypes.h (8.1 KB ) - added by cyril 11 years ago.
root_banner.jpg (4.0 KB ) - added by cyril 11 years ago.
typhoon-lr.jpg (677 bytes ) - added by cyril 11 years ago.
another jpg file that triggers the bug
ffmpeg.h (13.7 KB ) - added by cyril 10 years ago.
JpegTest.cpp converted to C (1/2)
ffmpeg.c (5.1 KB ) - added by cyril 10 years ago.
Jpeg

Download all attachments as: .zip

Change History (16)

by cyril, 11 years ago

Attachment: JpegTest.cpp added

by cyril, 11 years ago

Attachment: inttypes.h added

by cyril, 11 years ago

Attachment: root_banner.jpg added

by cyril, 11 years ago

Attachment: typhoon-lr.jpg added

another jpg file that triggers the bug

comment:1 by cyril, 10 years ago

It seems like nobody wants to analyze the issue. Well, I did some further investigations.

The frame buffer pDest still get corrupted by sws_scale() and then cannot be freed by avpicture_free() even if I only use MinGW tools to compile and then debug this simple application. So this bug is really not related to VC++ or whatever.

Moreover, I can see that sws_scale() eventually calls the function pointed by pSwsContext.swscale, which is set to yuv420_rgb32_MMX().

The source code for yuv420_rgb32_MMX() can be found in libswscale/x86/yuv2rgb_template.c and refers to assembly codes.

I can't read assembly and don't know how to fix this. So I hope that somebody with ASM skills is willing to look at this :)

Last edited 10 years ago by cyril (previous) (diff)

comment:2 by Carl Eugen Hoyos, 10 years ago

Is the problem Windows-specific or also reproducible on Linux?

by cyril, 10 years ago

Attachment: ffmpeg.h added

JpegTest.cpp converted to C (1/2)

by cyril, 10 years ago

Attachment: ffmpeg.c added

Jpeg

comment:3 by cyril, 10 years ago

I've attached what I used to test in MinGW. I simply replaced ffmpeg.h/.c with something similar to JpegTest.cpp, compiled FFmpeg with MinGW, and reproduced the bug with the resulting ffmpeg_g.exe.
I'll try to test this on Ubuntu and see if the bug is also reproducible there.

comment:4 by gjdfgh, 10 years ago

    pDest = avcodec_alloc_frame(); 
    avpicture_alloc((AVPicture *)(pDest), PIX_FMT_BGRA, 1, 60);

This is as broken as it gets. I think the rule is: never use AVPicture.

You can allocate a frame with av_frame_get_buffer(). You have to set the image parameters separately before calling this function.

comment:5 by cyril, 10 years ago

It's very old code, and it is still working in most cases but those 1-2 jpeg files. I've replaced avpicture_alloc() by av_frame_get_buffer() and it's indeed now working fine with those jpeg.

Nevertheless, it's quite confusing as AVPicture isn't marked as deprecated.

comment:6 by cyril, 10 years ago

By the way, what should be the value for the align parameter of av_frame_get_buffer()? 32 for 32-bit OS and 64 for 64-bit OS? Or something else?

comment:7 by gjdfgh, 10 years ago

Nevertheless, it's quite confusing as AVPicture isn't marked as deprecated.

I have no idea. I think AVPicture used to be defined as "can alias AVFrame", but now this use will severely. Welcome to FFmpeg.

AVPicture standalone is still used in some other places, like subtitle decoding, though.

By the way, what should be the value for the align parameter of av_frame_get_buffer()? 32 for 32-bit OS and 64 for 64-bit OS? Or something else?

Nobody knows, not even the developers. Most code seems to use 32.

comment:8 by cyril, 10 years ago

Thanks a lot for your answers gjdfgh.
This ticket can be closed now, but it doesn't seem like I have the rights to change the status of a ticket.

comment:9 by cyril, 10 years ago

Resolution: needs_more_info
Status: newclosed

comment:10 by Michael Niedermayer, 10 years ago

Resolution: needs_more_infoinvalid

set a more correct resolution
The way i understand it, the issue was a mistake in how the API was used.
Dont hesitate to change it again, if i misunderstood

Note: See TracTickets for help on using tickets.