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

[android] Handle mediacodec callback with a handler thread #2044

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

mingchou
Copy link
Contributor

@mingchou mingchou commented Dec 6, 2023

Now the MediaCodec callbacks and the activity lifecycle callbacks are called on the main thread. When pausing YouTube, the MediaCodec callbacks may be blocked until the lifecycle callbacks are done. This may cause frame drops on a resource-limited device when the MediaCodec callback to notify a newly decoded frame is blocked and we run out of decoded frames. Running the MediaCodec callbacks on a handler thread could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a

b/316008643

@Topi-Chen
Copy link

Hi guys, this change effectively prevents the decoded frame expired to improve the frame drop. please help to review it. thanks.

Copy link
Contributor

@sideb0ard sideb0ard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks nice and clean to me.

Copy link

codecov bot commented Dec 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6dd5f99) 58.45% compared to head (13de765) 58.55%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2044      +/-   ##
==========================================
+ Coverage   58.45%   58.55%   +0.10%     
==========================================
  Files        1904     1904              
  Lines       94327    94522     +195     
==========================================
+ Hits        55138    55352     +214     
+ Misses      39189    39170      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xiaomings
Copy link
Contributor

@borongc It would be great to understand the performance impact.

@mingchou mingchou force-pushed the mediacodec_cb_with_thread branch from fb9e04c to 38710b1 Compare December 8, 2023 10:58
@Topi-Chen
Copy link

Hi Bo-Rong and Xiaoming, may I have your comments about this commit? It is really helpful for a tight CPU resource. If you need any information, please feel free to let us know. Thanks.

@xiaomings
Copy link
Contributor

Hi Bo-Rong and Xiaoming, may I have your comments about this commit? It is really helpful for a tight CPU resource. If you need any information, please feel free to let us know. Thanks.

Hi Topi, thank you for contributing the PR. @borongc will double check that the PR doesn't cause any performance regression during normal playbacks. Please feel free to let us know if it's high priority to get this PR merged, so we can shuffle tasks around accordingly.

@kaidokert kaidokert force-pushed the mediacodec_cb_with_thread branch from 38710b1 to 8547d12 Compare December 16, 2023 03:46
@mingchou mingchou force-pushed the mediacodec_cb_with_thread branch from 8547d12 to 7c1d952 Compare December 19, 2023 10:09
@jasonzhangxx
Copy link
Contributor

The pr looks good to me. The only question is if we want to add more attribute in mime string for features. @xiaomings, what do you think?

@xiaomings
Copy link
Contributor

The pr looks good to me. The only question is if we want to add more attribute in mime string for features. @xiaomings, what do you think?

It should be ok here.

We no longer use mime attribute to configure media features due to that it increases the complexity of the web app when a set of interdependent mime attributes are being maintained. We may still use mime attribute to turn off an feature under emergencies, i.e. the mime attribute shouldn't be used by the webapp under normal situation. This allows us to pass information down to SbPlayer without introducing an extension.

When we expose a feature to be used by the web app under normal situations (i.e. to configure the append buffer size), we should avoid using mime attribute, and use dom interface instead.

@borongc borongc force-pushed the mediacodec_cb_with_thread branch 2 times, most recently from db0048a to 2268a44 Compare January 3, 2024 00:12
@borongc
Copy link
Contributor

borongc commented Jan 17, 2024

As there are 3 on_device tests keep failing, we are investigating the possible causes. @Topi-Chen and @mingchou would you be able to help as well? Thank you!

@borongc borongc force-pushed the mediacodec_cb_with_thread branch from 58686f7 to 264be75 Compare January 22, 2024 19:46
@borongc borongc self-requested a review January 22, 2024 20:54
@mingchou mingchou force-pushed the mediacodec_cb_with_thread branch from 264be75 to 3743985 Compare January 23, 2024 01:37
@borongc borongc added the cp-24.lts.1+ Cherry Pick to the 24.lts.1+ branch label Jan 24, 2024
Now the MediaCodec callbacks and the activity lifecycle callbacks
are called on the main thread. When pausing YouTube, the MediaCodec
callbacks may be blocked until the lifecycle callbacks are done.
This may cause frame drops on a resource-limited device when the
MediaCodec callback to notify a newly decoded frame is blocked and
we run out of decoded frames. Running the MediaCodec callbacks on a
handler thread could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a
@borongc borongc force-pushed the mediacodec_cb_with_thread branch from 3743985 to 13de765 Compare January 24, 2024 01:02
@borongc borongc merged commit ecde722 into youtube:main Jan 24, 2024
375 of 376 checks passed
cobalt-github-releaser-bot pushed a commit that referenced this pull request Jan 24, 2024
Now the MediaCodec callbacks and the activity lifecycle callbacks are
called on the main thread. When pausing YouTube, the MediaCodec
callbacks may be blocked until the lifecycle callbacks are done. This
may cause frame drops on a resource-limited device when the MediaCodec
callback to notify a newly decoded frame is blocked and we run out of
decoded frames. Running the MediaCodec callbacks on a handler thread
could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a

b/316008643

(cherry picked from commit ecde722)
@cobalt-github-releaser-bot
Copy link
Collaborator

Important

Creating the cherry pick PR failed! Check the log at https://github.com/youtube/cobalt/actions/runs/7644837688 for details.

borongc pushed a commit to borongc/cobalt that referenced this pull request Jan 24, 2024
)

Now the MediaCodec callbacks and the activity lifecycle callbacks are
called on the main thread. When pausing YouTube, the MediaCodec
callbacks may be blocked until the lifecycle callbacks are done. This
may cause frame drops on a resource-limited device when the MediaCodec
callback to notify a newly decoded frame is blocked and we run out of
decoded frames. Running the MediaCodec callbacks on a handler thread
could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a

b/316008643

(cherry picked from commit ecde722)
borongc pushed a commit that referenced this pull request Jan 24, 2024
Now the MediaCodec callbacks and the activity lifecycle callbacks are
called on the main thread. When pausing YouTube, the MediaCodec
callbacks may be blocked until the lifecycle callbacks are done. This
may cause frame drops on a resource-limited device when the MediaCodec
callback to notify a newly decoded frame is blocked and we run out of
decoded frames. Running the MediaCodec callbacks on a handler thread
could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a

b/316008643

(cherry picked from commit ecde722)
borongc pushed a commit that referenced this pull request Jan 24, 2024
Now the MediaCodec callbacks and the activity lifecycle callbacks are
called on the main thread. When pausing YouTube, the MediaCodec
callbacks may be blocked until the lifecycle callbacks are done. This
may cause frame drops on a resource-limited device when the MediaCodec
callback to notify a newly decoded frame is blocked and we run out of
decoded frames. Running the MediaCodec callbacks on a handler thread
could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a

b/316008643

(cherry picked from commit ecde722)
borongc pushed a commit that referenced this pull request Jan 25, 2024
Now the MediaCodec callbacks and the activity lifecycle callbacks are
called on the main thread. When pausing YouTube, the MediaCodec
callbacks may be blocked until the lifecycle callbacks are done. This
may cause frame drops on a resource-limited device when the MediaCodec
callback to notify a newly decoded frame is blocked and we run out of
decoded frames. Running the MediaCodec callbacks on a handler thread
could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a

b/316008643

(cherry picked from commit ecde722)
borongc pushed a commit that referenced this pull request Jan 26, 2024
…dler thread (#2277)

Refer to the original PR: #2044

Now the MediaCodec callbacks and the activity lifecycle callbacks are
called on the main thread. When pausing YouTube, the MediaCodec
callbacks may be blocked until the lifecycle callbacks are done. This
may cause frame drops on a resource-limited device when the MediaCodec
callback to notify a newly decoded frame is blocked and we run out of
decoded frames. Running the MediaCodec callbacks on a handler thread
could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a

b/316008643

Co-authored-by: Stone <[email protected]>
borongc added a commit to borongc/cobalt that referenced this pull request Feb 20, 2024
borongc added a commit to borongc/cobalt that referenced this pull request Feb 21, 2024
borongc added a commit to borongc/cobalt that referenced this pull request Feb 21, 2024
borongc added a commit that referenced this pull request Feb 22, 2024
cobalt-github-releaser-bot pushed a commit that referenced this pull request Feb 22, 2024
borongc added a commit that referenced this pull request Feb 22, 2024
kaidokert pushed a commit that referenced this pull request Feb 22, 2024
…th a handler thread (#2044)" (#2465)

Refer to the original PR: #2458

Reverts #2044

b/326091431

Co-authored-by: Bo-Rong Chen <[email protected]>
borongc pushed a commit to borongc/cobalt that referenced this pull request Apr 9, 2024
)

Now the MediaCodec callbacks and the activity lifecycle callbacks are
called on the main thread. When pausing YouTube, the MediaCodec
callbacks may be blocked until the lifecycle callbacks are done. This
may cause frame drops on a resource-limited device when the MediaCodec
callback to notify a newly decoded frame is blocked and we run out of
decoded frames. Running the MediaCodec callbacks on a handler thread
could avoid this problem.

Change-Id: I5ffdb1f5a582c3d01964b3f98c99d7aed211674a

b/316008643
borongc added a commit to borongc/cobalt that referenced this pull request Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cp-24.lts.1+ Cherry Pick to the 24.lts.1+ branch on_device outside collaborator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants