Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2332253004:
Don't require free PicturBuffers when draining AVDA for destroy. (Closed)
|
Created:
4 years, 3 months ago by liberato (no reviews please) Modified:
4 years, 3 months ago Reviewers:
watk CC:
chromium-reviews, posciak+watch_chromium.org, piman+watch_chromium.org, feature-media-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDon't require free PicturBuffers when draining AVDA for destroy.
When draining AVDA for destroy or reset, DequeueOutput will discard
any decoded frames from the codec. Previously, it would still
refuse to check for available output unless a free picture buffer
was available to hold it.
This could prevent a flush for destroy or reset from completing.
In the case of destruction, this would also prevent the codec from
being released. This affected only VP8 codecs, which require a
drain on some platforms.
BUG=642948
Committed: https://crrev.com/78ab3ba497f659cbec188ed50f6cb70dc5c10b1c
Cr-Commit-Position: refs/heads/master@{#418651}
Patch Set 1 #
Messages
Total messages: 17 (9 generated)
The CQ bit was checked by liberato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Don't require free PicturBuffers when draining AVDA for destroy. When draining AVDA for destroy or reset, DequeueOutput will discard any decoded frames from the codec. Previously, it would still refuse to check for available output unless a free picture buffer was available to hold it. This could prevent a flush for destroy or reset from completing. In the case of destruction, this would also prevent the codec from being released. This affected only VP8 codecs, which require a drain on some platforms. BUG=642948 ========== to ========== Don't require free PicturBuffers when draining AVDA for destroy. When draining AVDA for destroy or reset, DequeueOutput will discard any decoded frames from the codec. Previously, it would still refuse to check for available output unless a free picture buffer was available to hold it. This could prevent a flush for destroy or reset from completing. In the case of destruction, this would also prevent the codec from being released. This affected only VP8 codecs, which require a drain on some platforms. BUG=642948 ==========
liberato@chromium.org changed reviewers: + watk@chromium.org
i've not been able to figure out why i can't repro this in 52, due to compilation issues. however, it's 100% at ToT. without this patch, 'dumpsys media.resource_manager' shows a vp8 codec pretty much forever once a vp8 codec is created by AVDA. With this patch, it goes away when AVDA does after the flush completes. thanks -fl
Nice, lgtm. So are we going to cause the same issue with the construction thread any time it hangs? alive mediacodecs -> mediaserver drain.
On 2016/09/14 18:17:34, watk wrote: > Nice, lgtm. So are we going to cause the same issue with the construction thread > any time it hangs? alive mediacodecs -> mediaserver drain. probably. in those cases, it's not clear that the codec can be freed anyway, even if we spawned a thread to do so. it definitely hangs when mediaserver is hung up, in at least some cases. so, not sure we can do better. :( -fl
The CQ bit was checked by liberato@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/09/14 19:59:23, liberato wrote: > On 2016/09/14 18:17:34, watk wrote: > > Nice, lgtm. So are we going to cause the same issue with the construction > thread > > any time it hangs? alive mediacodecs -> mediaserver drain. > > probably. in those cases, it's not clear that the codec can be freed anyway, > even if we spawned a thread to do so. it definitely hangs when mediaserver is > hung up, in at least some cases. > > so, not sure we can do better. :( > > -fl I wonder if we should prefer to crash the gpu process? Because then mediaserver will get the binder death notification. Maybe that works even if it would hang otherwise. I don't know.
On 2016/09/14 20:01:59, watk wrote: > On 2016/09/14 19:59:23, liberato wrote: > > On 2016/09/14 18:17:34, watk wrote: > > > Nice, lgtm. So are we going to cause the same issue with the construction > > thread > > > any time it hangs? alive mediacodecs -> mediaserver drain. > > > > probably. in those cases, it's not clear that the codec can be freed anyway, > > even if we spawned a thread to do so. it definitely hangs when mediaserver is > > hung up, in at least some cases. > > > > so, not sure we can do better. :( > > > > -fl > > I wonder if we should prefer to crash the gpu process? Because then mediaserver > will get the binder death notification. Maybe that works even if it would hang > otherwise. I don't know. so far, that has worked. it's a possibility. maybe we could wait until chrome is backgrounded to minimize impact. i'm not sure if there's a way to close the binder on the chrome side, without going through the normal client-side MediaCodecBridge shutdown code. that might also do it, if such a way exists.
Message was sent while issue was closed.
Description was changed from ========== Don't require free PicturBuffers when draining AVDA for destroy. When draining AVDA for destroy or reset, DequeueOutput will discard any decoded frames from the codec. Previously, it would still refuse to check for available output unless a free picture buffer was available to hold it. This could prevent a flush for destroy or reset from completing. In the case of destruction, this would also prevent the codec from being released. This affected only VP8 codecs, which require a drain on some platforms. BUG=642948 ========== to ========== Don't require free PicturBuffers when draining AVDA for destroy. When draining AVDA for destroy or reset, DequeueOutput will discard any decoded frames from the codec. Previously, it would still refuse to check for available output unless a free picture buffer was available to hold it. This could prevent a flush for destroy or reset from completing. In the case of destruction, this would also prevent the codec from being released. This affected only VP8 codecs, which require a drain on some platforms. BUG=642948 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Don't require free PicturBuffers when draining AVDA for destroy. When draining AVDA for destroy or reset, DequeueOutput will discard any decoded frames from the codec. Previously, it would still refuse to check for available output unless a free picture buffer was available to hold it. This could prevent a flush for destroy or reset from completing. In the case of destruction, this would also prevent the codec from being released. This affected only VP8 codecs, which require a drain on some platforms. BUG=642948 ========== to ========== Don't require free PicturBuffers when draining AVDA for destroy. When draining AVDA for destroy or reset, DequeueOutput will discard any decoded frames from the codec. Previously, it would still refuse to check for available output unless a free picture buffer was available to hold it. This could prevent a flush for destroy or reset from completing. In the case of destruction, this would also prevent the codec from being released. This affected only VP8 codecs, which require a drain on some platforms. BUG=642948 Committed: https://crrev.com/78ab3ba497f659cbec188ed50f6cb70dc5c10b1c Cr-Commit-Position: refs/heads/master@{#418651} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/78ab3ba497f659cbec188ed50f6cb70dc5c10b1c Cr-Commit-Position: refs/heads/master@{#418651} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
