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

[Stefano Sabatini]

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.

> 
> > +    }
> > +
> > +    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.

[...]

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).
-- 
FFmpeg = Fundamentalist and Friendly Miracolous Prodigious Educated God
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavd-sdl-add-event-handler-thread.patch
Type: text/x-diff
Size: 7120 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131125/cb2bc618/attachment.bin>