[Bug 730995] Memory leak with interlaced h264 stream

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=730995
  GStreamer | gst-omx | 1.3.1

Nicolas Dufresne <nicolas.dufresne> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nicolas.dufresne at collabora.
                   |                            |co.uk

--- Comment #5 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2014-05-30 16:00:19 UTC ---
(In reply to comment #4)
> (In reply to comment #3)
> > Review of attachment 277545 [details] [details]:
> > 
> > I agree it's mostly a hack, but we didn't find anything better yet, hence this
> > is what we do in libav to fix the same issue. Please, have a look at livav
> > implementation to double check (iirc it was a bit more complex).
> 
> I look at gstlibav and you're right. there is the same behavior of cleaning
> frames. The method is a little different, first it marks all incoming frames as
> decode only. Then, when a frame is decoded, it is unmarked and all frames
> marked DECODE_ONLY between the beginning of the list and the current frame are
> discarded.
> 
> If you prefer this way, I can do it.

I see, so the list order is the order in which frames came in. Re-ordering
happens in the decoder. But libav will allocate output buffers for them, even
though they have been re-ordered. In fact, it only marks one of the field. This
is pretty clever, so we can drop unneeded frames much sooner. I don't think we
can do the same here, as the API is different.

My only concern is that we rely on correct PTS, hopefully it's always right,
though even if it was wrong, I think the result will be ok, and better than
leaking.

In the long term, it would be much nicer if the parsers could give a little
more information, so we could group the fields in the base class. Though it
would mean a frame with multiple input buffers, which would be an API break.

> 
> > ::: omx/gstomxvideodec.c
> > @@ +1179,3 @@
> > +    GstVideoCodecFrame *tmp = l->data;
> > +
> > +    if (tmp->pts < timestamp)
> > 
> > I'm not sure but can we cleanup <= here ?
> 
> I call drop_earlier_frame() before the current frame is finished, so cleanup
> with '<=' will also release the frame we want to output.

Thanks, it's clear now.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.