[FFmpeg-devel] [PATCH] Fix MPEG video lowres crash

[Michael Niedermayer]

On Thu, Dec 16, 2010 at 11:36:00AM +0300, Anatoly Nenashev wrote:
> On 16.12.2010 04:48, Michael Niedermayer wrote:
>> On Wed, Dec 15, 2010 at 02:41:22PM +0300, Anatoly Nenashev wrote:
>>    
>>> Hi!
>>> A little fix for possible crash of MPEG video as discribed in issue
>>> https://roundup.ffmpeg.org/issue2421.
>>> After that valgrind reports no error.
>>>      
>> what makes you think this fix is correct
>> The first hunk needs explanation from you what that is supposed to do and why
>> the rest looks wrong.
> The main problem is that if lowres>0 then picture sizes in input stream  
> and output stream are different.
> In test example when lowres=1 it makes ist->codec->width=704 and  
> ost->codec->width=352.
> That fact cause to problem in av_image_copy (imgutils.c:230) where  
> src_linesize=dst_linesize=704 but source picture from ost context has  
> width=352.
> Also the difference in sizes between input and output streams' pictures  
> cause ffmpeg to use rescaling filter which is really unneeded.
> As I can see from AVCodecContext API, if lowres>0 then width must be set  
> as coded_width>>lowres. So if you want to know the original size of  
> coded picture then look at the coded_width but not at the width.

There is avcodec_set_dimensions() which sets width/height correctly, the codec
should call that when being opened. The problem is av_find_stream_info() not
knowing the user specific lowres and the user is not able to set it as
av_find_stream_info() can add more streams
either way your code rounds wrong and might be exploitable



>

>> MV=0 does not need the emu code but your change looks
>> like it would call it. I guess theres rather a oversight related to the length
>> of the MC filter
>>
>>    
> This fix may by ugly but it was caused by SSSE3/MMX implementation of  
> h264_chroma_mc4. The closest look at the code shows that if mc4 applyed  
> in bottom macroblock's line of picture then overrun from source buffer  
> is available even if MV=0. That issue can be fixed by enlarging  
> picture's buffer size but I've decided that this is not a good solution  
> corresponded to flag CODEC_FLAG_EMU_EDGE.

see avcodec_align_dimensions2()

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101218/a1c09a70/attachment.pgp>