[FFmpeg-devel] lavd/opengl: use SDL2

On 18.09.2016 10:04, Josh de Kock wrote:
> On 18/09/2016 05:51, Lukasz Marek wrote:
>> W dniu niedziela, 18 września 2016 Lou Logan <lou@lrcd.com
>> <javascript:_e(%7B%7D,'cvml','lou@lrcd.com');>> napisał(a):
>>
>>> On Sat, Sep 17, 2016, at 05:21 PM, Lukasz Marek wrote:
>>>> On 18 September 2016 at 00:30, Josh de Kock <josh@itanimul.li> wrote:
>>> [...]
>>>>>  Would like some feedback where it doesnt work etc
>>> [...]
>>>> I said I will patch opengl, your implemantation donsn't look good at
>>>> this
>>>> point, declined,
>>>
>>> It would be helpful to provide some details describing what exactly does
>>> not look good.
>>> _______________________________________________
>>
>>
>> opengl device depends on opengl context. this patch works on sdl renderer
>> which works on X. in many cases X is opengł so patch is working for many
>> cases, but is shity in general.
>>
>
> Sorry, I don't understand. Could you rephrase this?

I updated my version. I wanted to submit after vote is finished, but to 
save our time you can review it and include in your sdl patchset.

>
> From c8988099c8535c77382b6f05d23326a0270bb2f4 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki2@gmail.com>
> Date: Sun, 18 Sep 2016 19:13:12 +0200
> Subject: [PATCH] lavd/opengl: use SDL2
>
> Signed-off-by: Lukasz Marek <lukasz.m.luki2@gmail.com>
> ---
>  libavdevice/opengl_enc.c | 109 +++++++++++++++++++++--------------------------
>  1 file changed, 49 insertions(+), 60 deletions(-)
>
> diff --git a/libavdevice/opengl_enc.c b/libavdevice/opengl_enc.c
> index 1dbbb80..94259a2 100644
> --- a/libavdevice/opengl_enc.c
> +++ b/libavdevice/opengl_enc.c
> @@ -46,7 +46,7 @@
>  #include <GL/glx.h>
>  #endif
>
> -#if HAVE_SDL
> +#if HAVE_SDL2
>  #include <SDL.h>
>  #endif
>
> @@ -174,8 +174,9 @@ static const GLushort g_index[6] =
>  typedef struct OpenGLContext {
>      AVClass *class;                    ///< class for private options
>
> -#if HAVE_SDL
> -    SDL_Surface *surface;
> +#if HAVE_SDL2
> +    SDL_Window *window;
> +    SDL_GLContext glcontext;
>  #endif
>      FFOpenGLFunctions glprocs;
>
> @@ -341,30 +342,14 @@ static int opengl_control_message(AVFormatContext *h, int type, void *data, size
>      return AVERROR(ENOSYS);
>  }
>
> -#if HAVE_SDL
> -static int opengl_sdl_recreate_window(OpenGLContext *opengl, int width, int height)
> -{
> -    opengl->surface = SDL_SetVideoMode(width, height,
> -                                       32, SDL_OPENGL | SDL_RESIZABLE);
> -    if (!opengl->surface) {
> -        av_log(opengl, AV_LOG_ERROR, "Unable to set video mode: %s\n", SDL_GetError());
> -        return AVERROR_EXTERNAL;
> -    }
> -    SDL_GL_SetAttribute(SDL_GL_RED_SIZE, 8);
> -    SDL_GL_SetAttribute(SDL_GL_GREEN_SIZE, 8);
> -    SDL_GL_SetAttribute(SDL_GL_BLUE_SIZE, 8);
> -    SDL_GL_SetAttribute(SDL_GL_ALPHA_SIZE, 8);
> -    SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);
> -    return 0;
> -}
> -
> +#if HAVE_SDL2
>  static int opengl_sdl_process_events(AVFormatContext *h)
>  {
> -    int ret;
>      OpenGLContext *opengl = h->priv_data;
> +    AVDeviceRect message;
>      SDL_Event event;
>      SDL_PumpEvents();
> -    while (SDL_PeepEvents(&event, 1, SDL_GETEVENT, SDL_ALLEVENTS) > 0) {
> +    while (SDL_PeepEvents(&event, 1, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT) > 0) {
Please use SDL_PollEvent() instead.

>          switch (event.type) {
>          case SDL_QUIT:
>              return AVERROR(EIO);
> @@ -375,23 +360,14 @@ static int opengl_sdl_process_events(AVFormatContext *h)
>                  return AVERROR(EIO);
>              }
>              return 0;
> -        case SDL_VIDEORESIZE: {
> -            char buffer[100];
> -            int reinit;
> -            AVDeviceRect message;
> -            /* clean up old context because SDL_SetVideoMode may lose its state. */
> -            SDL_VideoDriverName(buffer, sizeof(buffer));
> -            reinit = !av_strncasecmp(buffer, "quartz", sizeof(buffer));
> -            if (reinit) {
> -                opengl_deinit_context(opengl);
> -            }
> -            if ((ret = opengl_sdl_recreate_window(opengl, event.resize.w, event.resize.h)) < 0)
> -                return ret;
> -            if (reinit && (ret = opengl_init_context(opengl)) < 0)
> -                return ret;
> -            message.width = opengl->surface->w;
> -            message.height = opengl->surface->h;
> -            return opengl_control_message(h, AV_APP_TO_DEV_WINDOW_SIZE, &message, sizeof(AVDeviceRect));
> +        case SDL_WINDOWEVENT:
> +            switch(event.window.event) {
> +            case SDL_WINDOWEVENT_RESIZED:
> +            case SDL_WINDOWEVENT_SIZE_CHANGED:
> +                SDL_GL_GetDrawableSize(opengl->window, &message.width, &message.height);
> +                return opengl_control_message(h, AV_APP_TO_DEV_WINDOW_SIZE, &message, sizeof(AVDeviceRect));
> +            default:
> +                break;
>              }
>          }
>      }
> @@ -400,23 +376,34 @@ static int opengl_sdl_process_events(AVFormatContext *h)
>
>  static int av_cold opengl_sdl_create_window(AVFormatContext *h)
>  {
> -    int ret;
> -    char buffer[100];
>      OpenGLContext *opengl = h->priv_data;
>      AVDeviceRect message;
>      if (SDL_Init(SDL_INIT_VIDEO)) {
>          av_log(opengl, AV_LOG_ERROR, "Unable to initialize SDL: %s\n", SDL_GetError());
>          return AVERROR_EXTERNAL;
>      }
> -    if ((ret = opengl_sdl_recreate_window(opengl, opengl->window_width,
> -                                          opengl->window_height)) < 0)
> -        return ret;
> -    av_log(opengl, AV_LOG_INFO, "SDL driver: '%s'.\n", SDL_VideoDriverName(buffer, sizeof(buffer)));
> -    message.width = opengl->surface->w;
> -    message.height = opengl->surface->h;
> -    SDL_WM_SetCaption(opengl->window_title, NULL);
> -    opengl_control_message(h, AV_APP_TO_DEV_WINDOW_SIZE, &message, sizeof(AVDeviceRect));
> -    return 0;
> +    opengl->window = SDL_CreateWindow(opengl->window_title,
> +                                      SDL_WINDOWPOS_UNDEFINED,
> +                                      SDL_WINDOWPOS_UNDEFINED,
> +                                      opengl->window_width, opengl->window_height,
> +                                      SDL_WINDOW_RESIZABLE | SDL_WINDOW_OPENGL);
> +    if (!opengl->window) {
> +        av_log(opengl, AV_LOG_ERROR, "Unable to create default window: %s\n", SDL_GetError());
> +        return AVERROR_EXTERNAL;
> +    }
> +    opengl->glcontext = SDL_GL_CreateContext(opengl->window);
> +    if (!opengl->glcontext) {
> +        av_log(opengl, AV_LOG_ERROR, "Unable to create OpenGL context on default window: %s\n", SDL_GetError());
> +        return AVERROR_EXTERNAL;
> +    }
> +    SDL_GL_SetAttribute(SDL_GL_RED_SIZE, 8);
> +    SDL_GL_SetAttribute(SDL_GL_GREEN_SIZE, 8);
> +    SDL_GL_SetAttribute(SDL_GL_BLUE_SIZE, 8);
> +    SDL_GL_SetAttribute(SDL_GL_ALPHA_SIZE, 8);
> +    SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);
> +    av_log(opengl, AV_LOG_INFO, "SDL driver: '%s'.\n", SDL_GetCurrentVideoDriver());
> +    SDL_GL_GetDrawableSize(opengl->window, &message.width, &message.height);
> +    return opengl_control_message(h, AV_APP_TO_DEV_WINDOW_SIZE, &message, sizeof(AVDeviceRect));
>  }
>
>  static int av_cold opengl_sdl_load_procedures(OpenGLContext *opengl)
> @@ -460,14 +447,14 @@ static int av_cold opengl_sdl_load_procedures(OpenGLContext *opengl)
>
>  #undef LOAD_OPENGL_FUN
>  }
> -#endif /* HAVE_SDL */
> +#endif /* HAVE_SDL2 */
>
>  #if defined(__APPLE__)
>  static int av_cold opengl_load_procedures(OpenGLContext *opengl)
>  {
>      FFOpenGLFunctions *procs = &opengl->glprocs;
>
> -#if HAVE_SDL
> +#if HAVE_SDL2
>      if (!opengl->no_window)
>          return opengl_sdl_load_procedures(opengl);
>  #endif
> @@ -517,7 +504,7 @@ static int av_cold opengl_load_procedures(OpenGLContext *opengl)
>          return AVERROR(ENOSYS); \
>      }
>
> -#if HAVE_SDL
> +#if HAVE_SDL2
>      if (!opengl->no_window)
>          return opengl_sdl_load_procedures(opengl);
>  #endif
> @@ -943,7 +930,7 @@ static int opengl_create_window(AVFormatContext *h)
>      int ret;
>
>      if (!opengl->no_window) {
> -#if HAVE_SDL
> +#if HAVE_SDL2
>          if ((ret = opengl_sdl_create_window(h)) < 0) {
>              av_log(opengl, AV_LOG_ERROR, "Cannot create default SDL window.\n");
>              return ret;
> @@ -975,7 +962,9 @@ static int opengl_release_window(AVFormatContext *h)
>      int ret;
>      OpenGLContext *opengl = h->priv_data;
>      if (!opengl->no_window) {
> -#if HAVE_SDL
> +#if HAVE_SDL2
> +        SDL_GL_DeleteContext(opengl->glcontext);
> +        SDL_DestroyWindow(opengl->window);
>          SDL_Quit();
>  #endif
>      } else if ((ret = avdevice_dev_to_app_control_message(h, AV_DEV_TO_APP_DESTROY_WINDOW_BUFFER, NULL , 0)) < 0) {
> @@ -1109,9 +1098,9 @@ static av_cold int opengl_write_header(AVFormatContext *h)
>
>      glClear(GL_COLOR_BUFFER_BIT);
>
> -#if HAVE_SDL
> +#if HAVE_SDL2
>      if (!opengl->no_window)
> -        SDL_GL_SwapBuffers();
> +        SDL_GL_SwapWindow(opengl->window);
>  #endif
>      if (opengl->no_window &&
>          (ret = avdevice_dev_to_app_control_message(h, AV_DEV_TO_APP_DISPLAY_WINDOW_BUFFER, NULL , 0)) < 0) {
> @@ -1204,7 +1193,7 @@ static int opengl_draw(AVFormatContext *h, void *input, int repaint, int is_pkt)
>      const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pix_fmt);
>      int ret;
>
> -#if HAVE_SDL
> +#if HAVE_SDL2
>      if (!opengl->no_window && (ret = opengl_sdl_process_events(h)) < 0)
>          goto fail;
>  #endif
> @@ -1245,9 +1234,9 @@ static int opengl_draw(AVFormatContext *h, void *input, int repaint, int is_pkt)
>      ret = AVERROR_EXTERNAL;
>      OPENGL_ERROR_CHECK(opengl);
>
> -#if HAVE_SDL
> +#if HAVE_SDL2
>      if (!opengl->no_window)
> -        SDL_GL_SwapBuffers();
> +        SDL_GL_SwapWindow(opengl->window);
>  #endif
>      if (opengl->no_window &&
>          (ret = avdevice_dev_to_app_control_message(h, AV_DEV_TO_APP_DISPLAY_WINDOW_BUFFER, NULL , 0)) < 0) {
> --
> 1.9.1
>

Another than SDL_PollEvent(), this looks fine; if SDL1 is dropped then I 
don't see any reason why it shouldn't be applied.

--
Josh