mltframework
melt uses freed memory and crashes
Putting together a slide show in Kdenlive I hit a rendering crash. It's perfectly reproducible and Valgrind provides a sensible analysis of the memory error. I'm attaching the MLT XML input and the full Valgrind output. I guess the starting "uninitialized value" warnings are unrelated (and possibly even false positives), though I wonder if the loop in serialize_branch (at procuder_xml.c:119) isn't one item too short. Anyway, we eventually reach
==24954== Invalid read of size 8
==24954== at 0x129F3F60: QImage::hasAlphaChannel() const (qimage.cpp:4588)
==24954== by 0x121547FF: refresh_image (qimage_wrapper.cpp:237)
==24954== by 0x1214E82E: producer_get_image (producer_qimage.c:244)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x138EB385: filter_get_image (filter_deinterlace.c:251)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D6EEAA: get_image (filter_fieldorder.c:33)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D6DA63: filter_get_image (filter_crop.c:76)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D7376B: filter_get_image (filter_rescale.c:221)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== Address 0x68e9888 is 24 bytes inside a block of size 32 free'd
==24954== at 0x483808B: operator delete(void*, unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==24954== by 0x121545EA: reorient_with_exif (qimage_wrapper.cpp:134)
==24954== by 0x121545EA: refresh_qimage (qimage_wrapper.cpp:186)
==24954== by 0x12154738: refresh_image (qimage_wrapper.cpp:225)
==24954== by 0x1214E82E: producer_get_image (producer_qimage.c:244)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x138EB385: filter_get_image (filter_deinterlace.c:251)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D6EEAA: get_image (filter_fieldorder.c:33)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D6DA63: filter_get_image (filter_crop.c:76)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D7376B: filter_get_image (filter_rescale.c:221)
==24954== Block was alloc'd at
==24954== at 0x4836DEF: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==24954== by 0x1215431D: refresh_qimage (qimage_wrapper.cpp:179)
==24954== by 0x12154738: refresh_image (qimage_wrapper.cpp:225)
==24954== by 0x1214E82E: producer_get_image (producer_qimage.c:244)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x138EB385: filter_get_image (filter_deinterlace.c:251)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D6EEAA: get_image (filter_fieldorder.c:33)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D6DA63: filter_get_image (filter_crop.c:76)
==24954== by 0x486116E: mlt_frame_get_image (mlt_frame.c:620)
==24954== by 0x11D7376B: filter_get_image (filter_rescale.c:221)
Some tinkering with gdb also shows that the image acquired by
QImage *qimage = static_cast<QImage*>( self->qimage );
at qimage_wrapper.cpp:236 isn't valid at the 387th occasion, leading to
(gdb) p qimage->size()
$13 = {wd = 32767, ht = -1073739568}
(gdb) p scaled.size()
$16 = {wd = 0, ht = 0}
(gdb) p scaled.scanLine(0)
$20 = (uchar *) 0x0
which is the final NULL pointer access (*src) leading to the segmentation fault.
I'm happy to provide further info if needed. I'm working with the current Debian unstable package (mlt 6.20.0-2) on the x86_64 architecture.
Waldorf.mlt.txt
valgrind.log
Looks like the problem was the image being deleted in the exif rotation code while being used in a filter. I have now updated the QImage producer to use Qt's internal orientation detection for image rotation, which means the exif code will not be used anymore. Do you have a way to test against current git master?
Yes, unless there are extensive changes beyond 6.20.0. I expect to be able to cherry-pick 3bfff158 at least, would that be enough? Thanks for looking into this!
Indeed, adding 3bfff15 to 6.20.0 avoids the crash (I decided to try that instead of master for better isolation). Thank you very much for the quick fix! Out of interest, how do you plan to fix this for Qt < 5.5?
As I understand it, reorient_with_exif() leaves self->qimage invalid when qimage is replaced in refresh_qimage(). By the way, doesn't using pointers to implicitly shared Qt classes (like QImage) add an extra redirection and mislead reference counting without much performance benefit over direct assignment? (I've got no Qt programming experience, just read up on these things a little chasing this crash.)