GNOME Bugzilla – Bug 701257
Huge memory leak when processing mpeg2 field pictures.
Last modified: 2013-07-15 12:05:00 UTC
Created attachment 245622 [details] [review] Fix the memory leak with mpeg2 field encoded stream decoding. There is a huge memory leak when decoding mpeg2 field encoded streams. How to reproduce the issue: sample: http://samples.mplayerhq.hu/MPEG-VOB/interlaced/uruseiyatsura.vob Download and play this sample with gst-launch. Check the top command o/p. Attaching a quick-fix patch. Every GstVideoCodecFrame that the handle_frame() receiving from basedecoder should be disposed with either _finish_frame() or drop_frame() apis. Currenlty we are utilizing same vasurface for decoding both top and bottom field. Which means the gstvaapidecoder is NOT pushing all the video_codec_frames to async queue (decoder->frames). Instead we push only the complete frames to decoder->frames. This causing huge memory leak since are not releasing all the GstVideoCodecFrames which are holding by the basedecoder.
Yes, the first field should be dropped from GstVideoDecoder view, i.e. marked as decode-only and submitted to finish.
(In reply to comment #1) > Yes, the first field should be dropped from GstVideoDecoder view, i.e. marked > as decode-only and submitted to finish. Hm,, It seems that I am not receiving the e-mail notification for your comments :)
(In reply to comment #1) > Yes, the first field should be dropped from GstVideoDecoder view, i.e. marked > as decode-only and submitted to finish. So do you want to handle it from gstvideodecoder instead of doing it separately from each decoders?
(In reply to comment #3) > (In reply to comment #1) > > Yes, the first field should be dropped from GstVideoDecoder view, i.e. marked > > as decode-only and submitted to finish. > > So do you want to handle it from gstvideodecoder instead of doing it > separately from each decoders? Yes, that would be the preferred method. e.g. as in the attached patch. There is still an issue though: cases where the second field fails to decode, but we still have to emit the first field. This could easily fixed in gst_vaapi_picture_destroy(), very early, by calling do_output() with the skipped flag, if the picture was not output yet.
Created attachment 246510 [details] [review] decoder: fix memory leak when processing interlaced pictures Alternative patch that handles this issue in gstvaapidecoder_objects.c (gst_vaapi_picture_output).
Hi, did you have a chance to try Patch 246510? Thanks. :)
Hhm,,not yet. You might have already tested with your patch ,right? I guess it doesn't need to test again if you can't reproduce the issue. ;)
(In reply to comment #7) > Hhm,,not yet. You might have already tested with your patch ,right? I guess it > doesn't need to test again if you can't reproduce the issue. ;) Yes, I had tested it a long time ago too, it was hanging in my local "staging" branch. :)
Just push it then ;)
commit 1fff3a44dde33a3d817a47a72cf7bb1ecb9c3bf7 Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com> Date: Tue Jun 11 15:11:34 2013 +0200 decoder: fix memory leak when processing interlaced pictures. Fix memory leak when processing interlaced pictures and that occurs because the first field, represented as a GstVideoCodecFrame, never gets released. i.e. when the picture is completed, this is generally the case when the second field is successfully decoded, we need to propagate the GstVideoCodecFrame of the first field to the original GstVideoDecoder so that it could reclaim memory. Otherwise, we keep accumulating the first fields into GstVideoDecoder private frames list until the end-of-stream is reached. The frames are eventually released there, but too late, i.e. too much memory may have been consumed. https://bugzilla.gnome.org/show_bug.cgi?id=701257