[Ffmpeg-devel] segfault in ff_mpeg1_find_frame_end()

[Michael Niedermayer]

Hi

On Saturday 14 May 2005 01:57, Stefan Lucke wrote:
> On Samstag, 14. Mai 2005 01:37, M?ns Rullg?rd wrote:
> > Stefan Lucke <stefan at lucke.in-berlin.de> writes:
> > > On Samstag, 14. Mai 2005 00:17, M?ns Rullg?rd wrote:
> > >> Stefan Lucke <stefan at lucke.in-berlin.de> writes:
> > >> > I just made some progress. At the beginning of function
> > >> > MPV_common_end() there are av_freep() for the parser buffer and
> > >> > parser buffer_size is set to 0. But I think pointer into the parses
> > >> > buffer are still present. So I commented out these two lines and
> > >> > even with av_read_packet() I get __no__ crash.
> > >> >
> > >> > I run valgrind with the modified code and there are no new leaks
> > >> > compared to run without my modification.
> > >>
> > >> What does a valgrind say when it does crash?  That's what's
> > >> interesting.
> > >
> > > I certenly cannot run valgrind with a realtime video decoding software
> > > on my box. For that it is to slow and as softdevice tries up to sync
> > > the video to real time, it is not not guaranteed the the same code gets
> > > executed and/or the frames are decoded (they are skipped).
> > >
> > > But I tried it. I used a statically linked ffmpeg libs. It's a script
> > > in which I inserted the following line:
> > > --- about that time crashing stream started playback ---
> > >
> > > This time there was no segfault but there are some messages
> > > when there is the 4:3 16:9 switch. the plain 4:3 part was without.
> > > Hopefully we can get some hints for the segault.
> >
> > ==3487== Invalid read of size 4
> > ==3487==    at 0x3468108C: mpeg1_decode_picture (bswap.h:29)
> > ==3487==    by 0x34683412: mpeg_decode_frame (mpeg12.c:3117)
> > ==3487==    by 0x34592683: avcodec_decode_video (utils.c:612)
> > ==3487==    by 0x3458C152: cVideoStreamDecoder::DecodePacket(AVPacket*)
> > (mpeg2decoder.c:446) ==3487==    by 0x3458B398: cStreamDecoder::Action()
> > (mpeg2decoder.c:148) ==3487==    by 0x80DD08F:
> > cThread::StartThread(cThread*) (thread.c:227) ==3487==    by 0x34193F5F:
> > pthread_start_thread (in /lib/i686/libpthread.so.0) ==3487==    by
> > 0x343AE326: clone (in /lib/i686/libc.so.6)
> > ==3487==  Address 0x38AEA628 is not stack'd, malloc'd or (recently)
> > free'd ==3487==
> > ==3487== Invalid read of size 4
> > ==3487==    at 0x3468109D: mpeg1_decode_picture (bswap.h:29)
> > ==3487==    by 0x34683412: mpeg_decode_frame (mpeg12.c:3117)
> > ==3487==    by 0x34592683: avcodec_decode_video (utils.c:612)
> > ==3487==    by 0x3458C152: cVideoStreamDecoder::DecodePacket(AVPacket*)
> > (mpeg2decoder.c:446) ==3487==    by 0x3458B398: cStreamDecoder::Action()
> > (mpeg2decoder.c:148) ==3487==    by 0x80DD08F:
> > cThread::StartThread(cThread*) (thread.c:227) ==3487==    by 0x34193F5F:
> > pthread_start_thread (in /lib/i686/libpthread.so.0) ==3487==    by
> > 0x343AE326: clone (in /lib/i686/libc.so.6)
> > ==3487==  Address 0x38AEA629 is not stack'd, malloc'd or (recently)
> > free'd
> >
> > This looks like the decoder is being fed garbage addresses to decode.
> >
> > Are you by any chance buffering AVPackets before decoding them?
>
> Yes. There are several threads. At least one for audio and one for
> video and some others. My guess from previous post is that
> parse_context.buffer is freed while another thread allocs this memory.
> So existing pointers into that buffer area refer to someone else.
> If ffmpeg is not threaded it may get the same memory locations again.
> So no segfault. I won't say that my commented out av_freep() is
> the solution. I want to say that these two lines are the causer.
>
> > That's generally not a safe thing to do.
>
> Not save ? or forbidden ?

buffering AVPackets blindly is forbidden, but theres av_dup_packet() which 
makes them bufferable

-- 
Michael