We need to have control over the lifecycle of the buffer as it may get used from another thread. For an overview of the problems this causes, see r3570 - the code is very brittle because of this misfeature.
This is also true of enc_x264 (to a lesser extent).
r3955 implements a buffer tracking logic in dec_avcodec.
avcodec notifies dec_avcodec when it is done with a buffer. Instead of freeing it immediately (which is avcodec's default behavior), we now wait for *both* avcodec and xpra to be done with the buffer.
Xpra needs to call xpra_release_buffer(data[0])
to notify dec_avcodec that it is done with a given buffer. dec_avcodec will then work from there to find the AVFrame * pointer, and mark it as "xpra released".
Once the xpra_release_buffer() calls have been added, dec_avcodec's tracking logic will need to be fully enabled, with the following patch:
diff --git a/src/xpra/codecs/dec_avcodec/dec_avcodec.c b/src/xpra/codecs/dec_avcodec/dec_avcodec.c index b36238d..4697ed7 100644 --- a/src/xpra/codecs/dec_avcodec/dec_avcodec.c +++ b/src/xpra/codecs/dec_avcodec/dec_avcodec.c @@ -148,7 +148,7 @@ static int my_avcodec_get_buffer(AVCodecContext *avctx, AVFrame *frame) static void release_buffer_if_ready(struct frame_pointer *f, AVCodecContext *avctx, AVFrame *frame) { // If buffer is released both by avcodec and xpra, free it... - if (f->avcodec_released) { // disable xpra tracking until hooked up && f->xpra_released) { + if (f->avcodec_released && f->xpra_released) { avcodec_default_release_buffer(avctx, frame); }
Reverted in r3970 - too many problems with it, see changeset for details.
implements frame tracking in python
Done in r3976, lots of details both in changelog and in the code itself since the "pointer as dictionary key" isn't obvious.
with XPRA_AVCODEC_DEBUG=1
we can see:
avcodec_get_buffer
makes us create and register a new framewrapper, then we also create an AVImageWrapper
for the pixels and when that is freed, we call xpra_free
:
add_framewrapper(0x7f79980ca430L, AVFrameWrapper(0x7f79980ca430)) known frame keys: [] avcodec: 257856 bytes in <class 'xpra.codecs.dec_avcodec.decoder.AVImageWrapper'>(YUV420P:(0, 0, 498, 316, 24)) AVImageWrapper.free() av_frame=AVFrameWrapper(0x7f79980ca430) xpra_free(0x7f79980ca430) framewrapper=AVFrameWrapper(0x7f79980ca430) AVFrameWrapper(0x7f79980ca430).xpra_free() AVImageWrapper.free() av_frame=None
add_framewrapper(0x7f7998174f50L, AVFrameWrapper(0x7f7998174f50)) known frame keys: ['0x7f79980ca430'] avcodec: 257856 bytes in <class 'xpra.codecs.dec_avcodec.decoder.AVImageWrapper'>(YUV420P:(0, 0, 498, 316, 24)) AVImageWrapper.free() av_frame=AVFrameWrapper(0x7f7998174f50) xpra_free(0x7f7998174f50) framewrapper=AVFrameWrapper(0x7f7998174f50) AVFrameWrapper(0x7f7998174f50).xpra_free() AVImageWrapper.free() av_frame=None
0x7f79980ca430L
), so we call av_free()
which calls free()
since we had already called xpra_free()
on this frame at step 1:
avcodec_release_buffer(0x7f7998002160L, 0x7f79980ca430L) \ decoder=<xpra.codecs.dec_avcodec.decoder.Decoder object at 0x1f7b590>, frame_key=140160218735664 av_free(0x7f79980ca430L) framewrapper=AVFrameWrapper(0x7f79980ca430) AVFrameWrapper(0x7f79980ca430).av_free() AVFrameWrapper(0x7f79980ca430).free()
Then we continue processing the frame as before:
add_framewrapper(0x7f79980ca430L, AVFrameWrapper(0x7f79980ca430)) known frame keys: ['0x7f7998174f50'] avcodec: 257856 bytes in <class 'xpra.codecs.dec_avcodec.decoder.AVImageWrapper'>(YUV420P:(0, 0, 498, 316, 24)) AVImageWrapper.free() av_frame=AVFrameWrapper(0x7f79980ca430) xpra_free(0x7f79980ca430) framewrapper=AVFrameWrapper(0x7f79980ca430) AVFrameWrapper(0x7f79980ca430).xpra_free() AVImageWrapper.free() av_frame=None
etc
On exit, we cleanup the decoder, which ends up firing avcodec_release_buffer
on the 2 remaining frames it was still using and we can actually free them:
avcodec_release_buffer(0x7f7998002160L, 0x7f79980ca430L) \ decoder=<xpra.codecs.dec_avcodec.decoder.Decoder object at 0x1f7b590>, frame_key=140160218735664 av_free(0x7f79980ca430L) framewrapper=AVFrameWrapper(0x7f79980ca430) AVFrameWrapper(0x7f79980ca430).av_free() AVFrameWrapper(0x7f79980ca430).free() avcodec_release_buffer(0x7f7998002160L, 0x7f7998174f50L) \ decoder=<xpra.codecs.dec_avcodec.decoder.Decoder object at 0x1f7b590>, frame_key=140160219434832 av_free(0x7f7998174f50L) framewrapper=AVFrameWrapper(0x7f7998174f50) AVFrameWrapper(0x7f7998174f50).av_free() AVFrameWrapper(0x7f7998174f50).free()
Important fix for the context free code in r3983
this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/350