> On 2 Jan 2019, at 16:03, Oliver Collyer <ovcoll...@mac.com> wrote: > > > >> On 2 Jan 2019, at 12:58, Oliver Collyer <ovcoll...@mac.com> wrote: >> >> Hello >> >> So this time I'm reporting some potential memory leaks in the x265 encoder. >> There are a few hundred following a short encode session, but all seem to >> have the same call stack as below but varying allocation sizes. >> >> I am calling avcodec_open2() to open the encoder and then >> avcodec_free_context() to clean it up afterwards, as per the documentation. >> >> My code is built against the latest from >> https://github.com/ShiftMediaProject/FFmpeg >> <https://github.com/ShiftMediaProject/FFmpeg> >> <https://github.com/ShiftMediaProject/FFmpeg >> <https://github.com/ShiftMediaProject/FFmpeg>> and I'm using Visual Leak >> Detector to detect the leaks. >> >> Or would this be better posted in whatever mailing list x265 development >> uses? >> >> Regards >> >> Oliver >> >> ---------- Block 63379 at 0x000000008CC3A370: 262225 bytes ---------- >> Leak Hash: 0x95CBF73D, Count: 15, Total 3933375 bytes >> Call Stack: >> ucrtbased.dll!aligned_malloc() >> emu-server.exe!x265::x265_malloc() + 0x48 bytes >> emu-server.exe!x265::BitCost::setQP() + 0x9A bytes >> emu-server.exe!x265::Search::setLambdaFromQP() + 0xF8 bytes >> emu-server.exe!x265::Analysis::compressIntraCU() + 0x87E bytes >> emu-server.exe!x265::Analysis::compressCTU() + 0xC8B bytes >> emu-server.exe!x265::FrameEncoder::processRowEncoder() + 0xFA7 bytes >> emu-server.exe!x265::FrameEncoder::processRow() + 0x128 bytes >> emu-server.exe!x265::WaveFront::findJob() + 0x125 bytes >> emu-server.exe!x265::WorkerThread::threadMain() + 0x18A bytes >> emu-server.exe!x265::Thread::`scalar deleting destructor'() + 0x14A bytes >> emu-server.exe!x265::Thread::`scalar deleting destructor'() + 0xD2 bytes >> KERNEL32.DLL!BaseThreadInitThunk() + 0x14 bytes >> ntdll.dll!RtlUserThreadStart() + 0x21 bytes >> Data: >> 70 A3 C3 8C 4B 02 00 00 ED ED ED ED ED ED ED ED p...K... >> ........ >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 7D 01 }.}.}.}. >> }.}.}.}. >> >> >> Visual Leak Detector detected 236 memory leaks (25197214 bytes). > > So looking into this some more, according to the x265 docs: > > "When the application has completed all encodes, it should call > x265_cleanup() to free process global, particularly if a memory-leak > detection tool is being used. x265_cleanup() also resets the saved CTU size > so it will be possible to create a new encoder with a different CTU size: > /* x265_cleanup: > * release library static allocations, reset configured CTU size */ > void x265_cleanup(void);" > > This function explicitly frees the allocations being reported by VLD above. > > I cannot see that this done anywhere by the ffmpeg libraries though. Maybe > this is not considered significant as the OS deals with this on exit? > > I did notice in libx265.c... > > AVCodec ff_libx265_encoder = { > .name = "libx265", > .long_name = NULL_IF_CONFIG_SMALL("libx265 H.265 / HEVC"), > .type = AVMEDIA_TYPE_VIDEO, > .id = AV_CODEC_ID_HEVC, > .init = libx265_encode_init, > .init_static_data = libx265_encode_init_csp, > .encode2 = libx265_encode_frame, > .close = libx265_encode_close, > .priv_data_size = sizeof(libx265Context), > .priv_class = &class, > .defaults = x265_defaults, > .capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_AUTO_THREADS, > .wrapper_name = "libx265", > }; > > ...there is the concept of initialisation of static data, but looking at > AVCodec there is no similar cleaning-up function (confirmed by looking at > avcodec/allcodecs.c). > > Is there a precedent for how to handle something like this - I don't see how > I can access the x265_api pointer in order to call the x265_cleanup function > myself, because it is hidden inside the private libx265context in libx265.c. > > Perhaps libx265.c needs to keep track of the number of encoders open, and > when the last is closed, call clean-up? In any case, the documentation above > states that calling z265_clean_up() is necessary in order to create a new > encoder with a different CTU size. >
So here is a patch that cleans up x265 when the last encoder is closed and gets rid of my VLD errors. I don't know whether you folks want this as it serialises x265 encoder init/close, but I'm throwing it out there anyway.
libx265.patch
Description: Binary data
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
