Opened 4 years ago

Closed 4 years ago

Last modified 4 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 4 years ago.
inttypes.h (8.1 KB) - added by cyril 4 years ago.
root_banner.jpg (4.0 KB) - added by cyril 4 years ago.
typhoon-lr.jpg (677 bytes) - added by cyril 4 years ago.
another jpg file that triggers the bug
ffmpeg.h (13.7 KB) - added by cyril 4 years ago.
JpegTest?.cpp converted to C (1/2)
ffmpeg.c (5.1 KB) - added by cyril 4 years ago.
Jpeg

Download all attachments as: .zip

Change History (16)

Changed 4 years ago by cyril

Changed 4 years ago by cyril

Changed 4 years ago by cyril

Changed 4 years ago by cyril

another jpg file that triggers the bug

comment:1 Changed 4 years ago by cyril

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 4 years ago by cyril (previous) (diff)

comment:2 Changed 4 years ago by cehoyos

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

Changed 4 years ago by cyril

JpegTest?.cpp converted to C (1/2)

Changed 4 years ago by cyril

Jpeg

comment:3 Changed 4 years ago by cyril

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 Changed 4 years ago by gjdfgh

    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 Changed 4 years ago by cyril

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 Changed 4 years ago by cyril

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 Changed 4 years ago by gjdfgh

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 Changed 4 years ago by cyril

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 Changed 4 years ago by cyril

  • Resolution set to needs_more_info
  • Status changed from new to closed

comment:10 Changed 4 years ago by michael

  • Resolution changed from needs_more_info to invalid

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.