Xpra: Ticket #350: dec_avcodec needs to let us manage the buffers as we see fit

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).



Tue, 04 Jun 2013 15:38:53 GMT - Antoine Martin: description, summary changed


Fri, 14 Jun 2013 06:08:40 GMT - Antoine Martin: priority changed


Mon, 22 Jul 2013 10:10:51 GMT - ahuillet:

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".


Mon, 22 Jul 2013 10:14:37 GMT - ahuillet:

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);
        }

Wed, 24 Jul 2013 05:21:27 GMT - Antoine Martin:

Reverted in r3970 - too many problems with it, see changeset for details.


Thu, 25 Jul 2013 16:02:06 GMT - Antoine Martin: attachment set

implements frame tracking in python


Fri, 26 Jul 2013 07:54:54 GMT - Antoine Martin: status changed; resolution set

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:

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()

Sun, 28 Jul 2013 06:25:40 GMT - Antoine Martin:

Important fix for the context free code in r3983


Sat, 23 Jan 2021 04:52:26 GMT - migration script:

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/350