Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AOSP wfd fix #3

Closed
wants to merge 10 commits into from
Closed

AOSP wfd fix #3

wants to merge 10 commits into from

Conversation

Jprimero15
Copy link

No description provided.

vladimiroltean and others added 10 commits August 7, 2019 00:09
This adds back the SurfaceMediaSource class, needed for WFD.

This reverts commit e885915.

Change-Id: I3f67d01f18441e49205e2e263d20f0fb6fc91fe6
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
This reverts commit d0a98fa.

Change-Id: I0554b92c290c1ebbd1a40fc2edb43573a97d4f6a
* Among others, adapt to the ABuffer API changes in
  "f03606d9 Move MediaBufferXXX from foundation to libmediaextractor"

Change-Id: Ie92fc035c6430f1458d45995a5b2627d0bc75122
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
…n issue

In function passMetadataBuffer_l, the bufferHandle(ANativeWindowBuffer) is
saved to data (VideoNativeMetadata) but in function getMediaBufferHandle it
gets the bufferHandle from (MediaBuffer*)buffer->data() + 4, which is a wrong
position. To solve this problem, we should get handle from ANativeWindowBuffer,
not from buffer->data() + 4. (If get bufferHandle from buffer->data() + 4, the
function signalBufferReturned will print "returned buffer was not found in the
current list" error.

Test: Running wifi display, we can see the handle could be found in buffer list.

Change-Id: I71ecf9e2bca1db67d8d6e862ac16b07e939bf521
Signed-off-by: zhangbo_a <zhangbo_a@pinecone.net>
In commit 3e32878 the stagefright code was restructured to fix
the logic for native handle source, but the change in the
function SurfaceMediaSource::signalBufferReturned was probably
missed.

Try to compare the media buffer handle also to the current native
buffer handle in this function when searching for correspondance.

Change-Id: I352293e525f75dde500ac8e71ee49209710030c3
…erBase 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>
AudioFlinger is not able to determine the correct
pid/tid for WifiDisplay and thus we do not pass checks
for CAPTURE_AUDIO_OUTPUT and RECORD_AUDIO permissions.

To fix audio for WifiDisplay, it should be safe to
always allow a trusted calling uid (AID_MEDIA which
has the same perms as AID_AUDIOSERVER).

Change-Id: Ifa46d8e77a43027645cad02a04263b58e134c3ad
When realloc fails, free *param instead.

Change-Id: Ib9d4632f9bc79698550bc74b5a3a0fa2e3c3fdda
mAttributes in AudioTrack can not be set in AudioTrack::set()
when pAttributes is NULL.
AudioTrack::getPosition() sometimes returns wrong value
since mAttributes is undefined value.
This change sets default values to mAttributes.

Bug: 123607740
Test: manual
Change-Id: Ie590c17efc466cc89cbd7e9f89fc3ebeed98eace
    In AudioPolicyManager::getOutputForDevice it will close an old direct output
if currently open and configured with different parameters, but when the old direct
output closed it will cause AudioTrack to restore and select output again.When this
goto AudioPolicyManager::getOutputForDevice, it will close the current direct output
and cause the other AudioTrack to restore.This process will loop infinitely until one
is stopped.
    Also when two voice_call tracks with flag AUDIO_OUTPUT_FLAG_VOIP_RX | AUDIO_OUTPUT_FLAG_DIRECT
are create, in AudioPolicyManager::getOutputForDevice one track will goto non-direct
and select a exist output with flag AUDIO_OUTPUT_FLAG_VOIP_RX. And this flag will match the
exist output with flag AUDIO_OUTPUT_FLAG_VOIP_RX | AUDIO_OUTPUT_FLAG_DIRECT opened by previous
voice_call track which will cause it selecting a mismatched output.

Test:CtsMediaTestCases
Test: manual
1.install LINE 8.15.3 and call somebody
2.play two voice_call streams in direct mode
3.Play media files normally, no abnormalities

Change-Id: I8d96ccc66c64e3e22f44d5adecbf2b01a8ff31c6
@SKULSHADY SKULSHADY closed this Sep 2, 2019
SKULSHADY pushed a commit that referenced this pull request Mar 21, 2023
Do not hold lock when IPC call is expected from HAL.

C2SurfaceSyncObj is shared lock between framework and HAL. HAL process
can have only one thread to handle IPC from HAL to framework.
Therefore Holding C2SurfaceSyncObj from HAL during IPC call could
trigger deadlock. The exact scenario is as follows.

Thread #1:(HAL -> framework IPC) HIDL call onInputBuffersReleased()
            calls to feedInputBufferIfAvailable(). Since this is using
            HAL IPC thread, this will block Thread #3. This is waiting
            for mOuput mutex which is held by Thread #2.
Thread #2:(framework) discardBuffer() holds mOutput mutex which blocks
            Thread #1. But this is waiting for C2SurfaceSyncObj which is
            held by Thread #3.
Thread #3:(HAL) Dtor of C2BufferQueueBlockPoolData is holding
            C2SurfaceSyncObj, therefore this will block #2. This thread
            is waiting for HIDL IPC thread to be free in order for
            'igbp->cancel()', but HIDL IPC thread is already occupied by
            Thread #1.

Bug: 246707566
Test: atest android.media.decoder.cts.AdaptivePlaybackTest
Test: atest android.media.decoder.cts.DecoderTest
Change-Id: I6a9540d3b4d03806cd40bb4f89a234a6b77758a9
Merged-In: I6a9540d3b4d03806cd40bb4f89a234a6b77758a9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants