[FFmpeg-devel] [RFC] v4l2_m2m: Fix races around freeing data on close

[Mark Thompson]

On 19/10/17 08:28, Jorge Ramirez wrote:
> On 10/19/2017 02:10 AM, Mark Thompson wrote:
>> Refcount all of the context information.
>> ---
>> As discussed in the other thread, something like this.  We move most of the context into a refcounted buffer and AVCodecContext.priv_data is left as a stub holding a reference to it.
> 
> um, ok ... please could you send a patch so I can apply it? I see several problems in v4l2_free_buffer.

What goes wrong?  It applies fine for me on current head (f4090940bd3024e69d236257d327f11d1e496229).

> To sum up the RFC, instead of using private_data to place the codec's context, it uses private data to place a _pointer_ to an allocated codec context "just" so it wont be deallocated after the codec is closed (codec close frees the private_data)
> 
> Personally I think adding AV_CODEC_FLAG2_PRESERVE_PRIVDATA_ONCLOSE and use private_data to hold the codec context is a cleaner/simpler approach.
> 
> - the codec requests private_data with a size (sizeof(type))
> - the code explicitly informs the API whether all memory will be released on close or it will preserve it.

- All APIs in ffmpeg with this sort of private data field use them in the same way - they are allocated at create/alloc time (with the given size, for AVOptions), and freed at close/destroy time.
- Using the well-tested reference-counted buffer implementation is IMO strongly preferable to making ad-hoc synchronisation with atomics and semaphores.
- All other decoders use the reference-counted buffer approach (e.g. look at rkmpp for a direct implementation, the hwaccels all do it via hwcontext).