PlayerEventSink::update_current_images is unsound about gecko-media HOT 2 CLOSED

Note that the PlanarYCbCrImage struct's Clone impl increments a C++ reference count on the pixel data, and its Drop impl decrements the reference count on the pixel data, which is supposed to ensure the pointer doesn't become dangling.

However, I have since realised that we override the ref count when we shutdown. So if we have a pointer to the planar data held while the Player object shuts down, we could end up deallocating the pixel data Rust code could end up derefing a dangling pointer. Since the compositor and the Player objects can live on different threads, this could potentially be an issue.

We don't see this in the example player, because the shutdown is initiated after the compositor's render loop has exited.

Fixing the lifetimes is complicated by the fact that C++ code will pass over the frame queue to the client multiple times; so we will call update_current_images() several times with the different PlanarYCbCrImage instances which referencing the same FrameIDs. So we have to be careful to allow the pixel data to be shared by multiple frame objects.

We may be able to do this by having the pixel data stored in Vec's allocated by Rust, and have some sort of indirection layer that allows them to be shared between frames and allows the C++ code to refer to them by some sort of handle (i.e. the opposite of what we currently do, which is have the frame's memory allocated by C++ code).

from gecko-media.

This should be resolved by PR 131.

from gecko-media.