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

[Lukasz M]

On 14 November 2013 15:53, Lukasz M <lukasz.m.luki at gmail.com> wrote:

> 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.
>>
>
Removed that hunk. I remember I got valgrind complaining about that
allocation, but cannot reproduce it now.

>
>> > -    if (XvQueryAdaptors(xv->display, DefaultRootWindow(xv->display),
>> &num_adaptors, &ai) != Success)
>> > +    if (XvQueryAdaptors(xv->display, DefaultRootWindow(xv->display),
>> &num_adaptors, &ai) != Success) {
>> > +        XCloseDisplay(xv->display);
>> >          return AVERROR_EXTERNAL;
>> > +    }
>> >      xv->xv_port = ai[0].base_id;
>> >      XvFreeAdaptorInfo(ai);
>> >
>> > @@ -146,6 +144,7 @@ static int xv_write_header(AVFormatContext *s)
>> >          av_log(s, AV_LOG_ERROR,
>> >                 "Device does not support pixel format %s, aborting\n",
>> >                 av_get_pix_fmt_name(encctx->pix_fmt));
>> > +        XCloseDisplay(xv->display);
>> >          return AVERROR(EINVAL);
>> >      }
>>
>> LGTM.
>>
>> Probably cleaner: you do all the deinit stuff in write_trailer, and
>> call it from write_header() in case of failure (e.g. how I did it in
>> sdl.c).
>
>
Updated this way.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-lavd-xv-free-resources-on-errors.patch
Type: text/x-patch
Size: 3872 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131115/44c54085/attachment.bin>