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>
diff --git a/media/libstagefright/wifi-display/source/MediaPuller.cpp b/media/libstagefright/wifi-display/source/MediaPuller.cpp
index 7bdef1d..5bf893d 100644
--- a/media/libstagefright/wifi-display/source/MediaPuller.cpp
+++ b/media/libstagefright/wifi-display/source/MediaPuller.cpp
@@ -181,6 +181,8 @@
// video encoder will release MediaBufferBase when done
// with underlying data.
accessUnit->meta()->setObject("mediaBufferHolder", new MediaBufferHolder(mbuf));
+ mbuf->release();
+ mbuf = NULL;
}
sp<AMessage> notify = mNotify->dup();