tree bd3a785d895d4610ceb7854e412183831812452c
parent 46a304103e9dbc203bfa9fac987da96d7aabcdb3
author Vladimir Oltean <olteanv@gmail.com> 1548015041 +0200
committer Tim Zimmermann <tim@linux4.de> 1709665266 +0100
gpgsig -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCgAdFiEEuwYxAyE4TRnuhLgPbcIaY/gZxe8FAmXna/IACgkQbcIaY/gZ
 xe82fg/9GI7rEihU49Znq7G0oa62CWabDh9nYAPgDixynrVC1q0GzumaCmnSLEJF
 nnhan8q5O48S6YbBHZ+4i6a37yw4bctLtiIqW76GfcBl8mMVsvJyQgfKv8lekp7t
 LflK2VVYR9ccy8nOUYbeaYKdYaUCejlr+A4aUoqgnpQoPnlmm+Rk6tg8YpK5I5Tw
 +q3Mw2ON1/NJ3TETKLygUfzcDHqIyb0tTNjjvzUbIQHTHx7P2jtnscxggAYLtq2b
 EXXf6YZ10R3h0CcYNHxgdH7cY9SWYF/18GYNHRApJKe5dCSDi4qv/TXIvIyfb+dC
 CCXivYqFnfvv5dFfAeMPQ5i8p6rZOdw1zBXfQk4zlXPgyCVbX4/IzBrNbB9XOZfz
 cKM0MIXrDE2c38fU/DPbWdpR9GS9gvrsiz7Vmx934EvV//lh8Gg68CR6QW62SxTr
 6vA+iNgRAn+I4+WCU+MD4rWx3XSjSci9ar61klQK0mUCEPkmJU/0Jk/aeUTapUQo
 JXXkRQlc31tsukJSHDP4LVq+Bm2bK8dSz+yhH+ikqTYfsdau0VN4y5YexB4e+QeL
 X0EqElbv8eyUGojMpdywtjjgstt4Xor6FV194yGof3oBsGKOuDIf6nMA6GCu/maC
 MvsPjayyZkzwoMyuXHIT0PKg/kqaO/Sd1K5+BKojDj9QXiSobiM=
 =DOg7
 -----END PGP SIGNATURE-----

libstagefright_wfd: video encoder does not actually release MediaBufferBase when done

* This fixes buffer flow SurfaceMediaSource -> MediaPuller -> Converted
  freezing at mMediaBuffersAvailableCondition.wait(), due to this
  condition never being broadcast. This was supposed to happen from within
  SurfaceMediaSource::signalBufferReturned(), but this was never called.
  The Converter class does feedEncoderInputBuffers(), and after the
  encoder does its job, it should return the video buffer to the
  SurfaceMediaSource in ACodec::BaseState::onOMXEmptyBufferDone().
* There (in ACodec class), the code for doing that used to be:

    // We're in "store-metadata-in-buffers" mode, the underlying
    // OMX component had access to data that's implicitly refcounted
    // by this "MediaBuffer" object. Now that the OMX component has
    // told us that it's done with the input buffer, we can decrement
    // the mediaBuffer's reference count.
    info->mData->setMediaBufferBase(NULL);

  This means that if there was already a MediaBufferBase assigned to
  this mediaBuffer, then it got released when explicitly setting it to NULL:

  void MediaCodecBuffer::setMediaBufferBase(MediaBufferBase *mediaBuffer) {
      if (mMediaBufferBase != NULL) {
          mMediaBufferBase->release();
      }
      mMediaBufferBase = mediaBuffer;
  }

  Then in MediaBuffer::release(), which is a subclass of
  MediaBufferBase, there is code that does

        mObserver->signalBufferReturned(this);

  This should have went on to call SurfaceMediaSource::signalBufferReturned(),
  as it was setting itself as observer on the buffers sent to the video
  encoder. Stay tuned to find out why the call path was broken.

* Now, after Mr. Dongwon Kang's commit
  "f03606d9 Move MediaBufferXXX from foundation to libmediaextractor",
  the setMediaBufferBase and getMediaBufferBase functions no longer
  exist, and reference counting on MediaBuffer's is different.
  The direct replacement of setMediaBufferBase(mbuf) is now
  meta()->setObject("mediaBufferHolder", new MediaBufferHolder(mbuf)).
  The reference counting seems to now be managed through the constructor
  and destructor of this new MediaBufferHolder class (the code for
  release() is now in the holder's destructor). Now the issue seems to
  be that the lifetime of these new MediaBufferHolder's is not quite
  what it should be, because their destructor never gets called, hence
  the buffers never get returned.

* This might be an API problem that Mr. Dongwon Kang himself acknowledged,
  since in the aforementioned patch, he forcefully called mbuf->release()
  right below a comment where it clearly said that "video encoder will
  release MediaBuffer when done with underlying data":

  https://android.googlesource.com/platform/frameworks/av/+/f03606d9034730bea1a394e6803f9ebc36f3d2eb%5E%21/#F13

* Without addressing the root cause of the issue, in this commit we are
  simply mirroring a workaround for what appears to be broken media
  buffer reference counting.

Change-Id: Ie540e6dcf5536f93091ced2af2e121b71f70bb83
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: DennySPb <dennyspb@gmail.com>