mlt_service_get_frame using movit filters may result in opengl operations w/o valid context

[coreyoconnor]

From investigating kdenlive, MLT and movit. Using this ticket for tracking the MLT implementation aspects of the customer goal of GPU accelerated filters. The only GPU accelerated filters (afaik anyways) are those using movit.

• the current MLT contract is: client applications are responsible for establishing the appropriate GL context.
• movit has a similar contract: client applications are responsible for establishing the appropriate GL context.
• MLT can delegate thread creation to client applications for uses of mlt_thread_create but not for consumer_worker_thread.
• the rendering of a movit filter chain cannot be invoked in parallel.
• delegating thread creation to client applications is necessary to establish a valid GL context for the thread. (At least for Qt applications)

All combined, client applications must: manage GL contexts ; use real_time -1, 0, or 1 when using movit for proper behavior.

Considering only movit as currently implemented and OpenGL only: The current implementation works with the restriction on real_time. Considering beyond these to situations like: OpenCL / Vulkan acceleration ; parallel GL rendering. This is not sufficient.

The current MLT and movit contract effectively delegates to the client application management of per-thread acceleration resources. Which I agree with. I think there is the need for two things:

1. MLT delegation of consumer_worker_thread thread creation to client application.

2. Addition of a per-render resource structure. (maybe already exists?) This would not be sharable between threads and is necessary for the client application to provide.

The GL context is a per-thread global which is (thankfully) not a design continued by opencl/vulkan. These have explicit device or command queue values. A per-render resource structure would enable providing client application managed resources to accelerated filters. As well as making explicit the GL context.

Still digging through the current kdenlive integration. Will try to keep these notes updates.

Original ticket:

Digging into the kdenlive issues using GPU effects I determined, experimentally, that MLT was performing OpenGL operations on a thread that was not a render thread. This meant the OpenGL context was either invalid (resulting in a variety of errors) or missing (resulting in a quick exit).

Specifically, this function in kdenlive:

• https://github.com/KDE/kdenlive/blob/refactoring_timeline/src/mltcontroller/clipcontroller.cpp#L662

calls get_frame on an Mlt::Producer. If this producer is a movit/glsl effect then this can result in opengl commands being issued on whatever thread that called get_frame. Which may or may not have a GL context.

When looking into the MLT docs I noticed the warning about multithreaded and opengl. Tho nothing explicit about all application code having to invoke MLT operations from within a rendering thread. Filing here so the caveat is known.

Question tho: should get_frame be thread safe for application code? IMO yes, this is necessary.

Related to this might be relaxing the restrictions on real_time values and the create thread events. Looked fairly straight forward, but not sure if there is some subtlety I'm missing:

• https://github.com/coreyoconnor/mlt/commit/1347f75d5506096dbf176b4f5c6840cf2ea7bbbb

[ddennedy]

calls get_frame on an Mlt::Producer

Actually, Mlt::Producer::get_frame() is not doing anything on an OpenGL context. However, later in that method, it calls Mlt::Frame::get_image(), which will when using the opengl/Movit module.

Question tho: should get_frame be thread safe?

Yes, but it does not need coordination with the rendering threads. However, calling get_image() on the proper thread with OpenGL is required, and MLT does not provide anything to ensure that. I ran into this same issue with Shotcut's Properties dock. When GPU Effect (OpenGL) is turned on, then I do not directly get a frame and its image on a non-MLT thread. Instead, I receive the next consumer-frame-show event and read the properties from the frame it supplies. However, that is prone to a race condition, and the frame is not always from the selected producer. It is a problem I never solved.

Related to this might be relaxing the restrictions on real_time and the create thread events.

OpenGL contexts are bound to a thread. If you use a single context, you need to make it current on each thread prior to making OpenGL calls in a mutually exclusive manner thereby defeating the usage of threads. If you give each thread an OpenGL context, then you may need to setup resource GL sharing, which MLT does not do for you. I am not an OpenGL expert, but I know things get hairy quickly with multiple threads and contexts. It is not yet even stable on a single thread within an interactive app that is changing the object model and rendering images.

[coreyoconnor]

Thanks for confirming the issue Dan. All code that uses mlt (at least mlt_consumer_get_frame?) would then have to be executed from a render thread. Which I'll consider it a bug in itself haha.

Fortunately, for the case of movit/OpenGL and get_frame, this is resolvable: Always delegate to the worker thread pool to render. Which, as that is always delegated to consumer-thread-create (with below change) might be easy enough...

Which.. gets me to the first proposed change. The specific restriction I was referring to was in this comment (emph added):

• starting a thread by listening and responding to this (real_time 1 or -1 only).

This change:

• https://github.com/coreyoconnor/mlt/commit/1347f75d5506096dbf176b4f5c6840cf2ea7bbbb

Removes that specific restriction. Results in the construction of worker threads used by MLT to delegate to consumer-thread-create regardless of real_time setting. (Ignoring thread attributes for now)

Which, to me, looks like a few changes short of ensuring mlt_consumer_get_frame is always as safe as mlt_consumer_rt_frame. Maybe?

[ddennedy]

All code that uses mlt (at least mlt_consumer_get_frame?) would then have to be executed from a render thread.

No, that is not what I said.

This change:

Your change has too many unrelated changes and is not clean.

Removes that specific restriction.

Why? You cannot use multiple render threads with the OpenGL module.

[coreyoconnor]

Updated the top with some ideas after digging into the kdenlive/MLT/movit integration.