[Ffmpeg-devel] [PATCH] do not probe AVFMT_NOFILE formats if file is already opened

Reimar Döffinger Reimar.Doeffinger
Sat Jan 20 00:02:08 CET 2007


Hello,
On Fri, Jan 19, 2007 at 12:20:22AM +0100, Michael Niedermayer wrote:
> On Thu, Jan 18, 2007 at 12:39:06PM +0100, Reimar D?ffinger wrote:
> > Hello,
> > I am not sure about the supposed behaviour, but currently when calling
> > av_probe_input_format with is_opened == 1 the img2 demuxer will try to
> > substitute %d and open the resulting file names.
> > IMO it would be more desirable (and fixes a crash with mplayer) not to
> > try any demuxers that will open a file when is_opened == 1, as attached
> > patch does.
> 
> if this patch doesnt break anything then iam fine with it

Couldn't detect any breakage, and the way it is currently used in
utils.c seems to fit well with the change, so applied.
ffplay seems to handle "numbered" jpgs with changing resolution very
badly though...
(In case someone cares, e.g. valgrind gives stuff like
==9531== Thread 4:
==9531== Invalid write of size 1
==9531==    at 0x490744B: memcpy (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==9531==    by 0x553B6D: ff_img_copy_plane (imgconvert.c:737)
==9531==    by 0x556FC4: img_convert (imgconvert.c:2471)
==9531==    by 0x5A0C20: sws_scale (imgresample.c:795)
==9531==    by 0x413C90: video_thread (ffplay.c:1254)
==9531==    by 0x4A39EE6: (within /usr/lib64/libSDL-1.2.so.0.11.0)
==9531==    by 0x4A70D58: (within /usr/lib64/libSDL-1.2.so.0.11.0)
==9531==    by 0x3D5A906464: start_thread (in /lib64/libpthread-2.5.so)
==9531==    by 0x3D5A0CF92D: clone (in /lib64/libc-2.5.so)
==9531==  Address 0x961A10F is not stack'd, malloc'd or (recently)
free'd

or

==9697== Invalid write of size 8
==9697==    at 0x5A2CF4: put_pixels_clamped_mmx (dsputil_mmx.c:300)
==9697==    by 0x677261: ff_simple_idct_put_mmx (simple_idct_mmx.c:1288)
==9697==    by 0x490A78: mjpeg_decode_sos (mjpeg.c:1610)
==9697==    by 0x49320E: mjpeg_decode_frame (mjpeg.c:2187)
==9697==    by 0x468202: avcodec_decode_video (utils.c:904)
==9697==    by 0x413A7A: video_thread (ffplay.c:1337)
==9697==    by 0x4A39EE6: (within /usr/lib64/libSDL-1.2.so.0.11.0)
==9697==    by 0x4A70D58: (within /usr/lib64/libSDL-1.2.so.0.11.0)
==9697==    by 0x3D5A906464: start_thread (in /lib64/libpthread-2.5.so)
==9697==    by 0x3D5A0CF92D: clone (in /lib64/libc-2.5.so)
==9697==  Address 0x73783F0 is 0 bytes after a block of size 246,672
alloc'd
==9697==    at 0x4904B0B: memalign (in
/usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==9697==    by 0x467F9D: avcodec_default_get_buffer (utils.c:304)
==9697==    by 0x48FCFD: mjpeg_decode_sof (mjpeg.c:1240)
==9697==    by 0x493097: mjpeg_decode_frame (mjpeg.c:2151)
==9697==    by 0x468202: avcodec_decode_video (utils.c:904)
==9697==    by 0x413A7A: video_thread (ffplay.c:1337)
==9697==    by 0x4A39EE6: (within /usr/lib64/libSDL-1.2.so.0.11.0)
==9697==    by 0x4A70D58: (within /usr/lib64/libSDL-1.2.so.0.11.0)
==9697==    by 0x3D5A906464: start_thread (in /lib64/libpthread-2.5.so)
==9697==    by 0x3D5A0CF92D: clone (in /lib64/libc-2.5.so)
)

> > Also, after deciding about the supposed semantics, I would propose renaming
> > AVFMT_NOFILE to something that gives at least some hint about what it is
> > supposed to mean, the comment for it is just as stupid: "/* no file
> > should be opened */" well, obviously somewhen a "file" must be opened, so
> > is the comment supposed to mean no file should be opened by the caller,
> > or by the demuxer or...
> 
> id rather change the comment then the identifer which breaks compatibility

Now if that isn't a good example why it's bad to confuse "then" and "than" ;-)
Anyway, I added two comments, hopefully they are good enough.

Greetings,
Reimar D?ffinger




More information about the ffmpeg-devel mailing list