After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 693772 - avdec: decoder frame list getting long
avdec: decoder frame list getting long
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other All
: Normal normal
: 1.2.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2013-02-14 09:40 UTC by Matej Knopp
Modified: 2013-12-13 13:39 UTC
See Also:
GNOME target: ---
GNOME version: ---

videodecoder: make _release_frame external API (2.63 KB, patch)
2013-11-26 20:12 UTC, Mark Nauwelaerts
committed Details | Review
avviddec: really release frame at proper time (1.59 KB, patch)
2013-11-26 20:13 UTC, Mark Nauwelaerts
committed Details | Review
avvidec: discard unused input frames (2.75 KB, patch)
2013-11-26 20:18 UTC, Mark Nauwelaerts
committed Details | Review

Description Matej Knopp 2013-02-14 09:40:08 UTC
There is a possible decoder leak in gst-libav. 
With first test file, the number of "leaked" frames seems to be capped, it could be just decoder not giving any output with empty VOPs at the beginning of file.
With second file, the numbers keeps increasing.
Comment 1 Sebastian Dröge (slomo) 2013-02-14 10:08:45 UTC
I can confirm this. This should be a) fixed somehow in libav/gst-libav and b) this warning in the video decoder base class should be a bit more clever in general :)
Comment 2 David Schleef 2013-02-14 20:02:14 UTC
I was going to run through all the decoders today and force "parsed=true" on the sink caps.  That should fix nearly everything.

I'll also write a better WARNING message.
Comment 3 Matej Knopp 2013-02-14 23:32:59 UTC
Well, with these files you get the exact same result with mpeg 4 parser before decoder.
Comment 4 Matej Knopp 2013-04-07 01:36:53 UTC
FIY, I tried h.264 decoder on windows and it needs 16 frames in before it starts pushing anything out. I really think this message should be configurable at least.
Comment 5 Sebastian Dröge (slomo) 2013-04-07 14:21:25 UTC
On RPi there's an internal queue in the codecs that allows up to 100 compressed frames... so the list would grow up to that size on RPi without any problems.
Comment 6 Sebastian Dröge (slomo) 2013-07-15 06:07:02 UTC
Degraded this warning to a debug output for now until someone has a better idea.

commit bd62595a758d63544f09e45d9f8d59804ffc20e0
Author: Sebastian Dröge <>
Date:   Tue Jun 4 17:49:55 2013 +0200

    videodecoder: Change GST_WARNING to a GST_DEBUG
    It's completely normal for some decoders to queue 50-60 frames without
    it causing any problems, e.g. RPi.

Nonetheless with the files above there's a bug in gst-libav
Comment 7 Mark Nauwelaerts 2013-11-26 20:07:42 UTC
Looks like we can be piling up pending frames in 2 cases:

(1) A frame has had a _get_buffer call happen for it, but it has never popped up as a "decoded one", i.e. never ended up submitted to _finish_frame.
This could be due to decode-only data/frames or maybe also in some 'hurry up decoding' cases (?).  It would eventually have _release_buffer called for it, but that one would only drop a ref and not arrange removing it altogether from the list.

(2) A frame never has a _get_buffer call happen for it (and so also no _release_buffer, and no _finish_frame).  This could happen due to buggy decoder (?), but also due to misparsed input (interlaced frame stuff or otherwise rare/exotic cases) or maybe keyframe-only (skip) decoding cases.

Patches follow that try to fix this.
Comment 8 Mark Nauwelaerts 2013-11-26 20:10:12 UTC
Btw, at least one sample here is an example of (2), and the file in bug #712219 is an example of (1) (when used with avdec_vp8).
Comment 9 Mark Nauwelaerts 2013-11-26 20:12:09 UTC
Created attachment 262897 [details] [review]
videodecoder: make _release_frame external API

Patches makes an internal function external API to allow subclass to perform some more cleanup.  The effect is slightly similar to _finish_frame with DECODE_ONLY flag, but performs even less accounting.
Comment 10 Mark Nauwelaerts 2013-11-26 20:13:29 UTC
Created attachment 262898 [details] [review]
avviddec: really release frame at proper time

Using API from previous patch, this should solve case (1) outlined above.
Comment 11 Mark Nauwelaerts 2013-11-26 20:18:09 UTC
Created attachment 262899 [details] [review]
avvidec: discard unused input frames

This one performs a bit of trickery to determine frames that never had some _get_buffer happen, and so are not worth keeping around anymore at some point.

Will commit in some time pending comments and a bit of further testing.
Comment 12 Nicolas Dufresne (ndufresne) 2013-11-27 22:05:45 UTC
Review of attachment 262897 [details] [review]:

::: gst-libs/gst/video/gstvideodecoder.c
@@ +2339,3 @@
+ * after which it is considered finished and released.
+ *
+ * Since: 1.3

should be Since 1.4
Comment 13 Nicolas Dufresne (ndufresne) 2013-11-27 22:08:11 UTC
Review of attachment 262898 [details] [review]:

::: ext/libav/gstavviddec.c
@@ +545,3 @@
   if (frame->mapped)
     gst_video_frame_unmap (&frame->vframe);
+  gst_video_decoder_release_frame (GST_VIDEO_DECODER (ffmpegdec), frame->frame);

How do you prevent calling both finish_frame() and release_frame() ?
Comment 14 Mark Nauwelaerts 2013-11-27 22:32:11 UTC
(In reply to comment #13)
> Review of attachment 262898 [details] [review]:
> ::: ext/libav/gstavviddec.c
> @@ +545,3 @@
>    if (frame->mapped)
>      gst_video_frame_unmap (&frame->vframe);
> +  gst_video_decoder_release_frame (GST_VIDEO_DECODER (ffmpegdec),
> frame->frame);
> How do you prevent calling both finish_frame() and release_frame() ?

I don't have to.  It is perfectly safe when that happens.  Then release_frame will only unref the codec frame, which is what happened on that line before patch.  If it is still in the list (as when _finish_frame not called), then it will also remove it there.  That is the behaviour which is needed there, as we need to release the frame "all the way".
Comment 15 Nicolas Dufresne (ndufresne) 2013-11-28 00:09:33 UTC
Btw, this should also work I think for H264 streams where the interlaced field (top or bottom) are encoded separatly, what do you think ?
Comment 16 Nicolas Dufresne (ndufresne) 2013-11-28 00:20:01 UTC
Review of attachment 262899 [details] [review]:

I think it's a rather good solution, it covers all the use cases and cannot really be handled from the baseclass.
Comment 17 Nicolas Dufresne (ndufresne) 2013-11-28 00:20:37 UTC
Review of attachment 262898 [details] [review]:

Agreed, so asside the Since line seems good.
Comment 18 Sebastian Dröge (slomo) 2013-11-28 07:34:13 UTC
Comment on attachment 262897 [details] [review]
videodecoder: make _release_frame external API

ACK, please push all this after fixing the Since :) gst_video_decoder_release_frame() should also improve things a bit in gst-omx and elsewhere
Comment 19 Mark Nauwelaerts 2013-11-28 21:25:05 UTC
Thanks for the review and comments.

I think/hope it should also work for the interlaced H264 case.  The intention at least is to resolve any leaking (other than possibly due to libav), while also compensating for some other parsing glitches etc.

I am also thinking of introducing a GST_VIDEO_CODEC_FRAME_FLAG_NO_FRAME flag (akin to the baseparse _NO_FRAME flag) that signals a "frame" to be even more discardable than _DECODE_ONLY.  That is, it allows making this frame cleaning re-usable by having it done by the baseclass for frames marked as _NO_FRAME (which also avoids the frame list copying in addition to being reusable).

Also, coming to think of it, it is probably safer/required to have _release_frame take _STREAM_LOCK when manipulating the frame list.
Comment 20 Mark Nauwelaerts 2013-12-01 11:12:45 UTC
Did some more testing and seems to work out fine.

Fwiw, looks like the avi clips mentioned here contain not-codec (null vop frames), which are being discarded. Afaik, those are typically inserted by mencoder/libav when muxing an avi if needed to preserve a/v sync.  So libav is likely doing the right thing in discarding these, and we should now be handling this as well.

Will stick to these patches for now, and move to the GstVideoDecoder _NO_FRAME approach following more field/master testing.

[in -base]
commit 40fc3060174bc402fdd9584b09b917d696a40724
Author: Mark Nauwelaerts <>
Date:   Tue Nov 26 20:50:33 2013 +0100

    videodecoder: make _release_frame external API
    ... so subclasses can release a frame all the way (also from frame list)
    without having to pass through _finish_frame or _drop_frame.
    The latter may not be applicable, or may or may not have already
    been called for the frame in question.

[in -libav]
commit fa5865c0cdab48c6701fdf6f437ccaea3ab3286a
Author: Mark Nauwelaerts <>
Date:   Tue Nov 26 20:57:37 2013 +0100

    avviddec: discard unused input frames
    ... to avoid these piling up in list of pending frames.

commit 2ec4008ea5f3296f50d56b11422ed5d56de6692b
Author: Mark Nauwelaerts <>
Date:   Tue Nov 26 20:55:43 2013 +0100

    avviddec: really release frame at proper time
    ... by also removing it from the pending list of frames,
    where it may still be in if it has never been submitted to _finish.
    This could happen if is a decode-only frame, or in skipped decoding
    situation, ...
Comment 21 Mark Nauwelaerts 2013-12-01 11:13:35 UTC
Comment on attachment 262897 [details] [review]
videodecoder: make _release_frame external API

(with minor adjustments as mentioned)
Comment 22 Sebastian Dröge (slomo) 2013-12-13 13:39:22 UTC
All backported to 1.2, will be in 1.2.2 release