dec_avcodec needs to let us manage the buffers as we see fit

[Antoine Martin]

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

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

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

[Antoine Martin]

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

[Antoine Martin]

implements frame tracking in python

[Antoine Martin]

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
  

• next frame comes in and we do the same again

  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
  

• when the next frame comes in, avcodec frees the first frame (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()

[Antoine Martin]

Important fix for the context free code in r3983

[migration script]

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