Mark Thompson wrote:
Hmph, looks like the parameter buffer confusion strikes again. The original VAAPI specification contained the following functions: """ /** * After this call, the buffer is deleted and this buffer_id is no longer valid * Only call this if the buffer is not going to be passed to vaRenderBuffer */ VAStatus vaDestroyBuffer ( ... /** * Send decode buffers to the server. * Buffers are automatically destroyed afterwards */ VAStatus vaRenderPicture ( """ The newer lavc implementation follows this, and does not call vaDestroyBuffer() on buffers passed to vaRenderPicture(). Not doing this would cause random crashes in multithreaded programs, because another buffer with the same ID could be created between the two calls. However, the Intel implementation never actually followed the specification - it leaked the buffers passed to vaRenderPicture(). So, a special driver quirk was added to detect that driver and additionally destroy the buffers in that case: <http://git.videolan.org/?p=ffmpeg.git;a=commit;h=4926fa9a4aa03f3b751f52e900b9efb87fea0591>. This restored same behaviour as old lavc with the Intel driver without breaking other implementations. That worked for a bit. Unfortunately, Intel got tired of people complaining that their driver was broken. Rather than fixing it, they decided to change the specification: <https://github.com/01org/libva/commit/3eb038aa13bdd785808286c0a4995bd7a1ef07e9> (this is an ABI and API break, and the SONAME was not changed). Thus we have the state now: VAAPI < 0.40: A) Destroy buffers: wrong, will crash in multithreaded programs. B) Don't destroy buffers: correct, driver quirk restores behaviour for Intel. VAAPI >= 0.40: A) Destroy buffers: correct, but may crash old drivers which haven't been updated (not detectable, because the library allows loading old drivers without telling the user). B) Don't destroy buffers: wrong, driver quirk restores correct behaviour for Intel, others may leak memory. lavc currently chooses (B) in both cases on the grounds that leaking memory is preferable to crashing randomly. There is some thought of switching to (A) for VAAPI >= 0.40, but given that we can't detect the version at runtime and the libraries consider everything to be compatible this seems dangerous. For Mesa, I feel like I remember it having a correct < 0.40 implementation, but looking again it doesn't seem to and there isn't any obvious change (maybe I am just wrong). Do you know if this changed? Adding the driver quirk like the Intel driver is probably the right thing to do if it has always been like that, if it hasn't then we need more magic to be able to distinguish versions to prevent older ones from crashing.
Thanks for the explanation. I haven't seen anything change in mesa and st/va search on cgit doesn't come up with anything obvious. In your github link there is a link to a bug that states gstreamer always released, and testing it doesn't leak. Since AMD devs develop with that, they wouldn't have seen this, or, I guess, needed to implement in driver. _______________________________________________ ffmpeg-user mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-user To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
