[vlc-devel] commit: Fixed image_Convert by properly overriding release policy. ( Laurent Aimar )

[Laurent Aimar]

On Thu, Jul 17, 2008, Pierre d'Herbemont wrote:
> 
> On Jul 17, 2008, at 9:38 PM, Laurent Aimar wrote:
> 
> > On Thu, Jul 17, 2008, Pierre d'Herbemont wrote:
> >>
> >> On Jul 17, 2008, at 9:22 PM, git version control wrote:
> >>
> >>> vlc | branch: master | Laurent Aimar <fenrir at videolan.org> | Thu Jul
> >>> 17 21:23:19 2008 +0200| [796e72022f71598de3a162998a240632e65fed60]
> >>>
> >>> Fixed image_Convert by properly overriding release policy.
> >>>
> >>>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=796e72022f71598de3a162998a240632e65fed60
> >>> ---
> >>> +    p_pic->pf_release = video_release_buffer_dummy;
> >>>    p_pif = p_image->p_filter->pf_video_filter( p_image->p_filter,
> >>> p_pic );
> >>> +    p_pic->pf_release = pf_sav_release;
> >>
> >> Does that means that pf_video_filter is trying to release p_pic? Why
> >> don't we correct that in video filter plugin instead?
> > The video filter does destroy the picture they are given when they do
> > not need them anymore.
> > It is by design and it is not a bug. (You cannot suppose when a video
> > filter does not need the input picture anymore).
> 
> Well, isn't the refcounting for picture working? If someone wants to  
> access a p_picture outside current scope it needs to retain it. When  
> it is done, it releases it...
 Sorry maybe a mix up between release/destroy.

> > The bug is that image_Convert wasn't handling it correclty as it does
> > not want this (default) behavior.
> > Well now that I think of it, it is not completely correct as the  
> > picture
> > may be destroyed by image_Convert caller while the filter still need  
> > it.
> > In the current state (vout does not use pf_release but a special  
> > scheme),
> > there is no proper solution but to do a complete copy :(
> Ok, I trust you, but I fear that we may leak something here... I am  
> not familiar with the code though.

 The vout_thread_t does not set and use pf_release unlike the filter
architecture.
 It use picture_t i_refcount and i_status.

 The vout creates a bunch of pictures at start and handles the
'reference' state through vout_LinkPicture/vout_DisplayPicture/...
 You may want to look at src/input/decoder.c to see how it is currently
handled from a user side.

 I am starting to fill uneased about the way video filter2 and vout interract
right now.

-- 
fenrir