[FFmpeg-devel] VP6 issues in Swfdec
Thu Sep 6 01:15:53 CEST 2007
On Wed, 5 Sep 2007 13:27:31 +0200
"Benjamin Otte" <otte at gnome.org> wrote:
> I'm one of the Swfdec Flash player developers. As you may be aware,
> Flash uses VP6 as a possible video decoder.
> Someone recently grabbed all of the videos at http://pown.alluc.org
> and threw them at ffmpeg. Surprisingly, lots of them killed the vp6
> decoder and in turn my browser.
> I'll attached a series of patches for issues that I could fix myself,
> and point out issues I could not fix myself.
> If you want to reproduce the issues, you'll have to get swfdec git,
> wget the files I'll point to and try to play them with player/swfplay
> from the swfdec sources.
I want to reproduce the issue, but I want to do it with ffmpeg/ffplay
(instead of swfdec). As the swf demuxer don't support compressed swf,
I tried to convert the samples using swf2flv (from FLV::Info). It
failed to convert some of the samples, so I wasn't able to test
(BTW: any information about compressed swf format would be appreciated,
is it simply a zlib compressed swf or something like that ?)
> -- SEGV
> patch: check_coeff_offset.diff
> SInce this was the first file where I hit the issue, I'll just list
> this one. FFmpeg reads an offset and doesn't validate it, this patch
> does that.
This sample failed to convert so I wasn't able to test, but the fix
seems OK. I will probably apply something similar soon.
(BTW, already said in previous review, but please don't use tabs in
> -- sanity checks
> patch: sanity.diff
> While trying to find the issue, I added sanity checks to the read
> functions used by vp56, as there doesn't seem to be any protection
> against overreads.
I don't like it very much. Moreover it probably has a significant
impact on performances. So at least a benchmark would be needed
(+ taking care of all remarks in other reviews).
> -- small memleak
> patch: small_memleak.diff
> This is a small memleak fix I noticed when looking at the code.
This looks OK (except the unneeded if(ctx)).
But I'm not the maintainer of imgresample.c so I will let approval
to our beloved maintainer ;-)
> -- memleak
> There's a huge memleak reported by valgrind (this is from a run of 210.swf):
> ==31127== 224,784,144 bytes in 177 blocks are still reachable in loss
> record 5,098 of 5,098
> ==31127== at 0x4021990: memalign (vg_replace_malloc.c:332)
> ==31127== by 0x510ECE6: av_malloc (mem.c:61)
> ==31127== by 0x4D71B01: avcodec_default_get_buffer (utils.c:308)
> ==31127== by 0x4F4F789: vp56_decode_frame (vp56.c:509)
> ==31127== by 0x4D70B7D: avcodec_decode_video (utils.c:937)
> ==31127== by 0x406712D: swfdec_video_decoder_ffmpeg_decode
> ==31127== by 0x406A2A8: swfdec_video_decoder_decode
> ==31127== by 0x40A32F0: swfdec_video_input_iterate (swfdec_video.c:82)
> ==31127== by 0x40A40F8: swfdec_video_movie_iterate_end
> ==31127== by 0x4089287: swfdec_player_iterate (swfdec_player.c:1114)
> ==31127== by 0x4089533: swfdec_player_do_advance (swfdec_player.c:1156)
> ==31127== by 0x407A53A: swfdec_marshal_VOID__ULONG_UINT
Unable to reproduce. No ffmpeg related memleak here when playing 991.flv
with ffplay (I wasn't able to convert 210.swf to flv).
So I assume this memleak is due to bad usage of lavc by swfdec.
> -- "alternative entropy decoding not supported"
> The VP6 decoder often claims the above error message and then produces
> weird images. So feel free to use the files listed here if you want to
> implement it. I should also notice that these files do a lot of other
> weird stuff (like using invalid image sizes for the decoded image). I
> guess that's just a side effect?
I was able to convert more than half of them to flv, and none of them
shows the "alternative entropy decoding not supported" message...
> -- SEGV in mmx code
> There's a SEGV here. I have no clue where it comes from. I'll leave
> that for you to figure out:
> #0 0xb6f69cfa in put_pixels8_mmx (block=0xb6a827a8 "????????ds\t",
> pixels=0xb611d6a8 '?' <repeats 200 times>..., line_size=1464, h=8)
> at i386/dsputil_mmx.c:416
> #1 0xb71431d6 in vp56_decode_frame (avctx=0x80d6c30, data=0x80d24a0,
> buf_size=2840) at vp56.c:432
> #2 0xb6f63b7e in avcodec_decode_video (avctx=0x80d6c30,
> picture=0x80d24a0, got_picture_ptr=0xbfa4b008,
> buf_size=2840) at utils.c:937
No crash here, but I guess the mmx version is not used on my computer.
I will try to force it's usage later, to try reproducing it.
More information about the ffmpeg-devel