[FFmpeg-devel] [PATCH 3/3] lavd/xv: free resources on errors

[Stefano Sabatini]

On date Thursday 2013-11-14 15:53:18 +0100, Lukasz M encoded:
> On 14 November 2013 12:33, Stefano Sabatini <stefasab at gmail.com> wrote:
> 
> > On date Wednesday 2013-11-13 23:40:47 +0100, Lukasz Marek encoded:
> > > write_trailer callback leave not freed resources on errors.
> > >
> > > Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> > > ---
> > >  libavdevice/xv.c |   11 +++++------
> > >  1 file changed, 5 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/libavdevice/xv.c b/libavdevice/xv.c
> > > index bfa6ff5..008e818 100644
> > > --- a/libavdevice/xv.c
> > > +++ b/libavdevice/xv.c
> > > @@ -120,15 +120,13 @@ static int xv_write_header(AVFormatContext *s)
> > >                                       xv->window_x, xv->window_y,
> > >                                       xv->window_width,
> > xv->window_height,
> > >                                       0, 0, 0);
> > > -    if (!xv->window_title) {
> > > -        if (!(xv->window_title = av_strdup(s->filename)))
> > > -            return AVERROR(ENOMEM);
> > > -    }
> > > -    XStoreName(xv->display, xv->window, xv->window_title);
> > > +    XStoreName(xv->display, xv->window, xv->window_title ?
> > xv->window_title : s->filename);
> > >      XMapWindow(xv->display, xv->window);
> >
> > Partially unrelated. Also maybe it is good to explicitly set
> > window_title in the context.
> >
> 
> Valgrind complained about not loaded xv->window_title, but this may be bug
> inside ffmpeg.c
> I will check it later if leak also occurs when xv->window_title is set by
> ffmpeg's option.
> Anyway, I'm not sure want you want? You prefer to copy s->filename to
> xv->window_title or not?

I found that keeping information in the context can be useful for
debugging purposes. Also setting window_title in the context,
simulates the behavior of explicitly setting the window title, so it
*might* result in more expected behavior, but this is just a not so
important nit.
-- 
FFmpeg = Faithless & Frenzy Mysterious Perennial Elitist Geek