libsdl-org / sdl12-compat

An SDL-1.2 compatibility layer that uses SDL 2.0 behind the scenes.
Other
193 stars 40 forks source link

MLT fix #288

Closed cg9999 closed 1 year ago

cg9999 commented 1 year ago

Please consider and comment on this change to fix MLT SDL1 consumer. Without this change Flowblade video editor crashes, when using the sdl12-compat layer. See issue https://github.com/jliljebl/flowblade/issues/1061

edit: the changes included a fix to infinite loop in SDL_FreeYUVOverlay, but another fix is already in main branch, so it's not included anymore. (see: https://github.com/libsdl-org/sdl12-compat/commit/683234e8708a081224942181710ca5dd3cee409e ) That change is also required to fix Flowblade hanging.

Do you prefer the fix here, or should I submit patches to MLT instead, to avoid using pixels pointer without lock?

icculus commented 1 year ago

Weird, I thought I already made this change for SMPEG. I'm on a phone right now, but let me check into this on a real computer to see what happened there, before I merge this.

cg9999 commented 1 year ago

Maybe you are referring to this in SDL_CreateYUVOverlay:

  /* Some programs (e.g. mplayer) access pixels without locking. */
    retval->pixels = hwdata->pixels;
    hwdata->dirty = SDL_TRUE;

That is also needed, but MLT also uses SDL_LockYUVOverlay and SDL_UnlockYUVOverlay, which makes the pixels pointer null again.

code in MLT's consumer_sdl.c

        if ( self->running && SDL_GetVideoSurface() && self->sdl_overlay != NULL )
        {
            self->buffer = self->sdl_overlay->pixels[ 0 ];
            if ( SDL_LockYUVOverlay( self->sdl_overlay ) >= 0 )
            {
                int size = mlt_image_format_size( vfmt, width, height, NULL );
                if ( image != NULL )
                    memcpy( self->buffer, image, size );
                SDL_UnlockYUVOverlay( self->sdl_overlay );
                SDL_DisplayYUVOverlay( self->sdl_overlay, &SDL_GetVideoSurface()->clip_rect );
            }
        }
noahrama commented 1 year ago

i have the latest sdl12-compat: 1.2.60-1 And i wonder if this issue is actually resolved? Flowblade is crashing in the same manner described above

cg9999 commented 1 year ago

i have the latest sdl12-compat: 1.2.60-1 And i wonder if this issue is actually resolved? Flowblade is crashing in the same manner described above

The version with the fix is not yet released.

icculus commented 1 year ago

We're trying to get a new release out shortly, stay tuned!