GNOME Bugzilla – Bug 767934
vaapidecoder_h265: critical memory leak
Last modified: 2016-06-22 12:32:50 UTC
Once I play h265 media like below, GST_DEBUG="GST_TRACER:7" GST_TRACERS="leaks" gst-play-1.0 video-h265.mkv During quit to play, it results in below. 0:00:01.573694986 12176 0x19bbe60 TRACE GST_TRACER :0:: object-alive, type-name=(string)GstCaps, address=(gpointer)0x7f65bc056c50, description=(string)image/jpeg; video/mpeg, mpegversion=(int)2, profile=(string){ simple, main }; video/x-h264, profile=(string){ main, high, constrained-baseline, multiview-high, stereo-high }; video/x-h265, profile=(string)main; video/x-vp8; video/x-wmv, wmvversion=(int)3, format=(string)WVC1, profile=(string)advanced; video/x-wmv, wmvversion=(int)3, profile=(string){ simple, main }, ref-count=(uint)1; 0:00:01.573747091 12176 0x19bbe60 TRACE GST_TRACER :0:: object-alive, type-name=(string)GstPad, address=(gpointer)0x7f65c4025b50, description=(string)<vaapidecode:src>, ref-count=(uint)2; 0:00:01.573771883 12176 0x19bbe60 TRACE GST_TRACER :0:: object-alive, type-name=(string)GstPad, address=(gpointer)0x7f65c4025910, description=(string)<vaapidecode:sink>, ref-count=(uint)2; 0:00:01.573792458 12176 0x19bbe60 TRACE GST_TRACER :0:: object-alive, type-name=(string)GstVaapiDecode, address=(gpointer)0x7f65bc0431a0, description=(string)<vaapidecode>, ref-count=(uint)1; It shows GstVaapiDecode object is not released, it leads to that other caps and pads are not released either. I think this leak causes a problem during seek, too. As far as I investigate, gst_vaapidecode_release is not being called enough, which is doing unref GstVaapiDecode object. Because dpb picture, which has a ref to surface proxy, is not released properly during reset or close. IMHO, we should re-implement the logic to remove all dpb pictures during reset in h265 decoder, I guess current logic is wrong.
Created attachment 330174 [details] [review] decoder: h265: fix to release all dpb pictures Once this patch is applied, there's no leak which I explained above. Also confirmed if it's working during seek.
Review of attachment 330174 [details] [review]: H264 and H265 decoders are very similar, and in H264 is not failing, if I understand correctly. Why is happening only in H265? ::: gst-libs/gst/vaapi/gstvaapidecoder_h265.c @@ -747,3 @@ if (hard_flush) { - for (i = 0; i < priv->dpb_count; i++) - dpb_remove_index (decoder, i); The question is, why dpb_remove_index() is not working as expected? why do we need another function?
(In reply to Víctor Manuel Jáquez Leal from comment #2) > Review of attachment 330174 [details] [review] [review]: > > H264 and H265 decoders are very similar, and in H264 is not failing, if I > understand correctly. Why is happening only in H265? > dpb manipulation of h264 and hevc are different. H264 dpb management is way more complicated than hevc. There could be a bug... I will have a look...
(In reply to Víctor Manuel Jáquez Leal from comment #2) > Review of attachment 330174 [details] [review] [review]: > > H264 and H265 decoders are very similar, and in H264 is not failing, if I > understand correctly. Why is happening only in H265? > > ::: gst-libs/gst/vaapi/gstvaapidecoder_h265.c > @@ -747,3 @@ > if (hard_flush) { > - for (i = 0; i < priv->dpb_count; i++) > - dpb_remove_index (decoder, i); > > The question is, why dpb_remove_index() is not working as expected? why do > we need another function? The one thing that I could say is the logic below is not doing to remove all pictures > - for (i = 0; i < priv->dpb_count; i++) > - dpb_remove_index (decoder, i);
(In reply to Hyunjun Ko from comment #0) > Once I play h265 media like below, > > GST_DEBUG="GST_TRACER:7" GST_TRACERS="leaks" gst-play-1.0 video-h265.mkv > > During quit to play, it results in below. > > 0:00:01.573694986 12176 0x19bbe60 TRACE GST_TRACER :0:: > object-alive, type-name=(string)GstCaps, address=(gpointer)0x7f65bc056c50, > description=(string)image/jpeg; video/mpeg, mpegversion=(int)2, > profile=(string){ simple, main }; video/x-h264, profile=(string){ main, > high, constrained-baseline, multiview-high, stereo-high }; video/x-h265, > profile=(string)main; video/x-vp8; video/x-wmv, wmvversion=(int)3, > format=(string)WVC1, profile=(string)advanced; video/x-wmv, > wmvversion=(int)3, profile=(string){ simple, main }, ref-count=(uint)1; > 0:00:01.573747091 12176 0x19bbe60 TRACE GST_TRACER :0:: > object-alive, type-name=(string)GstPad, address=(gpointer)0x7f65c4025b50, > description=(string)<vaapidecode:src>, ref-count=(uint)2; > 0:00:01.573771883 12176 0x19bbe60 TRACE GST_TRACER :0:: > object-alive, type-name=(string)GstPad, address=(gpointer)0x7f65c4025910, > description=(string)<vaapidecode:sink>, ref-count=(uint)2; > 0:00:01.573792458 12176 0x19bbe60 TRACE GST_TRACER :0:: > object-alive, type-name=(string)GstVaapiDecode, > address=(gpointer)0x7f65bc0431a0, description=(string)<vaapidecode>, > ref-count=(uint)1; > > It shows GstVaapiDecode object is not released, it leads to that other caps > and pads are not released either. I think this leak causes a problem during > seek, too. Could you please share the sample video? I can't find any memleak with my test samples...
(In reply to sreerenj from comment #5) > > Could you please share the sample video? > I can't find any memleak with my test samples... TearsOfSteel_720p_h265.mkv on http://www.h265files.com/ If you test again and again, you'll see leaks logs. It doesn't always happen. but likely 6-70%
(In reply to Hyunjun Ko from comment #4) > (In reply to Víctor Manuel Jáquez Leal from comment #2). > > Review of attachment 330174 [details] [review] [review] [review]: > > > > H264 and H265 decoders are very similar, and in H264 is not failing, if I > > understand correctly. Why is happening only in H265? > > > > ::: gst-libs/gst/vaapi/gstvaapidecoder_h265.c > > @@ -747,3 @@ > > if (hard_flush) { > > - for (i = 0; i < priv->dpb_count; i++) > > - dpb_remove_index (decoder, i); > > > > The question is, why dpb_remove_index() is not working as expected? why do > > we need another function? > > The one thing that I could say is the logic below is not doing to remove all > pictures > > - for (i = 0; i < priv->dpb_count; i++) > > - dpb_remove_index (decoder, i); You are right :), wonder why did't hit issues till now !! I will push the patch after few other verification, thanks!
Review of attachment 330174 [details] [review]: Pushed, thanks for the patch.
commit 05cf583c97737ac531faef6db0588fcbdb949d68 Author: Hyunjun Ko <zzoon@igalia.com> Date: Wed Jun 22 15:11:56 2016 +0300 decoder: h265: fix to release all dpb pictures Without this, all dpb pictures are not released during flush, because we used the global dpb_count variable for checking the dpb fullness which get decremented in dpb_remove_index() routine during each loop iteration. https://bugzilla.gnome.org/show_bug.cgi?id=767934