[FFmpeg-devel] [PATCH] lavd/sdl: add event handler thread

[Stefano Sabatini]

On date Monday 2013-11-25 01:09:24 +0100, Lukasz M encoded:
> On 25 November 2013 00:30, Stefano Sabatini <stefasab at gmail.com> wrote:
> 
> > On date Sunday 2013-11-24 20:36:28 +0100, Lukasz M encoded:
> > > On 24 November 2013 20:12, Stefano Sabatini <stefasab at gmail.com> wrote:
> > >
> > > > Experimental and partially untested, meant to address ticket #1743 and
> > > > #1744.
> > > > ---
> > > >  libavdevice/sdl.c | 115
> > > > +++++++++++++++++++++++++++++++++++++++++++++---------
> > > >  1 file changed, 96 insertions(+), 19 deletions(-)
> > > >
> > > > +static int event_thread(void *arg)
> > > > +{
> > > > +    AVFormatContext *s = arg;
> > > > +    SDLContext *sdl = s->priv_data;
> > > > +    int flags = SDL_SWSURFACE | sdl->window_fullscreen ?
> > SDL_FULLSCREEN :
> > > > 0;
> > > > +    AVStream *st = s->streams[0];
> > > > +    AVCodecContext *encctx = st->codec;
> > > > +
> > > > +    /* initialization */
> > > > +    SDL_WM_SetCaption(sdl->window_title, sdl->icon_title);
> > > > +    sdl->surface = SDL_SetVideoMode(sdl->window_width,
> > sdl->window_height,
> > > > +                                    24, flags);
> > > > +    if (!sdl->surface) {
> > > > +        av_log(sdl, AV_LOG_ERROR, "Unable to set video mode: %s\n",
> > > > SDL_GetError());
> > > > +        return AVERROR(EINVAL);
> > > >
> > >
> > > You will have a deadlock, sdl->inited is never set to 1 and
> > > sdl_write_header will wait forever. The same below.
> > > Returned values are send to nowhere, but no clue if they can be handled.
> > > Setting sdl->quit to 1 and signal sdl->init_event_cond may help.
> > > In sdl_write_header you may check if quit is set and return with error.
> >
> > Should be fixed.
> >
> 
> Not tested, but looks OK.
> 
> 
> >
> > >
> > > > +    }
> > > > +
> > > > +    sdl->overlay = SDL_CreateYUVOverlay(encctx->width, encctx->height,
> > > > +                                        sdl->overlay_fmt,
> > sdl->surface);
> > > > +    if (!sdl->overlay || sdl->overlay->pitches[0] < encctx->width) {
> > > > +        av_log(s, AV_LOG_ERROR,
> > > > +               "SDL does not support an overlay with size of %dx%d
> > > > pixels\n",
> > > > +               encctx->width, encctx->height);
> > > > +        return AVERROR(EINVAL);
> > > > +    }
> > > > +
> > > > +    av_log(s, AV_LOG_VERBOSE, "w:%d h:%d fmt:%s -> w:%d h:%d\n",
> > > > +           encctx->width, encctx->height,
> > > > av_get_pix_fmt_name(encctx->pix_fmt),
> > > > +           sdl->overlay_width, sdl->overlay_height);
> > > > +
> >
> > > > +    SDL_LockMutex(sdl->mutex);
> > > > +    sdl->inited = 1;
> > > > +    SDL_CondSignal(sdl->init_event_cond);
> > > > +    SDL_UnlockMutex(sdl->mutex);
> > > >
> > >
> > > You can unlock mutex before signaling.
> >
> > Does it make any difference? I based the code on the SDL wiki example.
> >
> 
> Its not an error for sure so feel free to ignore, but SDL_CondWait will
> have to lock this mutex at quiting and it is still locked here for no
> reason, so there may be not significant delay.

Changed. 

> > One thing I don't like very much about the patch, is that it is not
> > clear how to signal that the "output ended". I'm doing it by sending
> > an EOF when quit is created, but this will cause an error and a
> > failure from ffmpeg (we should probably handle the case, and don't
> > treat EOF as an error).
> >
> 
> I saw, but I am not sure this is not what should be expected.
> It can be compared oo usb flash taken out of port while ffmpeg is writing
> to it.
> Not checked though what is happening in this case.
> I was just surprised you picked EOF, and not AVERROR(EIO) for example.

Changed to EIO.

Updated. If someone can test on Windows I'll appreciate that,
otherwise I'll test it myself later.
-- 
FFmpeg = Frenzy and Fascinating Mastering Peaceless Eager Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavd-sdl-add-event-handler-thread.patch
Type: text/x-diff
Size: 7799 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131125/b79cb9e5/attachment.bin>