Memory leak somewhat identified

Archive of historical development discussions
Discussions / Development has moved to GitHub
Forum rules
*******************************
Please be aware we are now using GitHub for issue tracking and feature requests.
- This section of the forum is now closed to new topics.

*******************************
Post Reply
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Memory leak somewhat identified

Post by rhester »

Having done full memory regression testing from all releases 33 (0.7.1 release) forward, I've found the following:

- Release 40 increased memory usage by almost 50%. This is likely a result of the threading change, but the memory leak was NOT introduced here...just a lot more memory usage.
- Release 60 changed muxing to the new avformat method, necessary for later enhanced atom changes. This *did* introduce the memory leak I'm tracking. Likely candidates are:

encx264.c:

+ /* Sequence Parameter Set */
+ w->config->h264.sps_length = 1 + nal[1].i_payload;
+ w->config->h264.sps[0] = 0x67;
+ memcpy( &w->config->h264.sps[1], nal[1].p_payload, nal[1].i_payload );

+ /* Picture Parameter Set */
+ w->config->h264.pps_length = 1 + nal[2].i_payload;
+ w->config->h264.pps[0] = 0x68;
+ memcpy( &w->config->h264.pps[1], nal[2].p_payload, nal[2].i_payload );

(memcpy is still used in revision 69/70 and is what is being actively tracked)

muxmp4.c:

+ /* Video track */
+ mux_data = malloc( sizeof( hb_mux_data_t ) );
+ job->mux_data = mux_data;

Obviously, further significant changes were made between revision 60 and 69, but the changes from 59->60 definitely introduced the serious virtual memory bug I'm currently tracking that is preventing a full release. This impacts all platforms.

memcpy is notorious for introducing memory leaks, so that's where I'm paying the most attention at this time...anyone have a lot of experience with this that would like to jump in?

Rodney
rhester
Veteran User
Posts: 2888
Joined: Tue Apr 18, 2006 10:24 pm

Post by rhester »

I never got around to clarifying this, but it would appear the "memory leak" is in fact an artifact of the change to a multi-threaded model in HandBrake just after 0.7.0 was released. On slower machines, it is possible to have the audio thread "outrun" the video thread by a very significant amount, causing buffering to increase virtual memory utilization to high (read: >1GB at higher resolutions/bitrates) levels and ultimately potentially exhausting available memory resources.

This is not a bug. It is, if anything, a design flaw, and in the future HandBrake developers would likely do well to add some sort of thread synchronization to HandBrake to lessen the impact.

Rodney
Post Reply