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

Merge main into Ozone #4659

Merged
merged 38 commits into from
Jan 8, 2025
Merged

Merge main into Ozone #4659

merged 38 commits into from
Jan 8, 2025

Conversation

madhurajayaraman
Copy link
Contributor

Keep Ozone up to date

Tested on Cloudtop,

haozheng-cobalt and others added 30 commits December 19, 2024 12:06
b/372559388

This PR configures jni_generator for cobalt_apk_java. 
It creates the corresponding native class StarboardBridge for
cobalt/android/apk/app/src/main/java/dev/cobalt/coat/StarboardBridge.java,
adhering to Chromium's JNI standards.
The StarboardBridge native class is implemented as a singleton and is
initialized with the same JNIEnv and jobject used when calling the
Starboard custom JniEnvExt::Initialize function.
Additionally, it updates the JNI function calls in
starboard/android/shared/application_android.cc to align with Chromium's
modern JNI standards.


This PR amends the reverted PR
#4545

There was an issue in the reverted PR that there are old annotations
@UsedByNative embeded in the inner function calls in getResourceOverlay,
somehow making the app not able to launch..
Culprit ->

https://github.com/youtube/cobalt/pull/4545/files#diff-22285847addbd15025f71dadd357129f86573e042655067048f94fae301fb1d3R480.

We can not blindly replace @UsedByNative with @CalledByNative, when
switching to @CalledByNative, we should make sure to replace all
occurrence of @UserByNative inside, and implement the new JNI template
functions. I don't get why compiler didn't complain about the
getResourceOverlay issue tho.
Also fixes how the COBALT_IS_RELEASE_BUILD buildflag is set.

b/377410179
… as in crrev.com/cfd2020d.

Issue: 377049113
Reviewed-on: #4616
This includes a new Blink IDL and a Mojo interface with a stub
implementation in the browser. A real implementation, that actually adds
annotations to crash reports, will be added soon.

b/383301493
Calling DecoderBuffer::timestamp() on end of stream buffers triggers
DCHECK(). Now EOS buffers are explicitly checked for before calling
timestamp() in StarboardRenderer::OnDemuxerStreamRead(), to avoid a
DCHECK() triggered in Devel build when playback finishes.

b/326652276
A collection of small updates that fixes failing unit tests:
* Include runtime_deps files in test archive
* Disable annotations for junit parser (increases parsing speed)
* Continue running tests after suite fails, print failing suites at the
end

b/372303096
b/371590965
Issue: 365150653
Reviewed-on: #4621
CobaltActivity adopts ContentShellActivity, but ContentShell.apk didn't
set WebContents correctly when the activity onStop(), resulting in it
continued to play the video after pushing to the background.

Set WebContents to onHide() when the activity onStop(). This allows
Cobalt.apk to have the same behavior as Chrome when suspend/resume the
app on android.

Chrome upstream code is:
https://chromium-review.googlesource.com/c/chromium/src/+/6092099

b/384086780
b/383153069
b/326497953
The target does not build on other platforms.

b/375243230
Updates the separator used to parse codecs listed under the "codecs"
parameter. Previously, `IsProgressiveFormat()` would reject content
types that had no whitespace between codecs in the "codecs" paramter,
but were otherwise valid.

b/382791540
Removed
"//starboard/shared/starboard/decode_target/decode_target_internal.cc".
It only contains the default ctor and dtor for `SbDecodeTargetPrivate`,
and they are now moved into decode_target_internal.h to be inline.

b/327287075
…summary/action because it is faster. (#4573)

b/372303096

Change to use a new test publisher because the existing publisher can
take over an hour to run when there are many test failures:
https://github.com/youtube/cobalt/actions/runs/12302321578/job/34336630966?pr=4569

The new publisher reports >2000 failures with annotations in 3 seconds:

https://github.com/youtube/cobalt/actions/runs/12472477285/job/34812061568?pr=4573

Also remove debug from linux build targets because it takes >1 hour to
complete.
1. Ensure to initialize RecursiveMutex for SbLog before
ApplicationAndroid is initialized.
2. Enable qa build with official build.

b/384807408
On Android, compositor memory limits are derived from system memory and
dalvik memory limits. This code was noted as outdated in 2017 (see
linked bug), and as a result didn't work the way it should have.

This is because:
- Reported system RAM on Android is lower than installed RAM because of
  carveouts
- Dalvik memory limits are not really correlated with system RAM

Based on field data, the large majority of devices is running with a
computed limit of 256MiB, with some devices using 96MiB. This CL
simplifies the code to:
- Low-end or <2GiB: 96MiB
- Otherwise, 256MiB

Which mostly matches the current in-the-wild reality. These limits are
likely not optimal, but at least this simplifies the code. Also, lower
the priority cutoff to "NICE_TO_HAVE", which matches desktop, and
reality, since ALLOW_EVERYTHING is lowered to NICE_TO_HAVE elsewhere in
the code (see PriorityCutoffToTileMemoryLimitPolicy() for instance).

This is gated behind a feature flag, to make sure that this is not
breaking things.

(cherry picked from commit 42b97eaa9deace1c188434ccac37ee22223ade4f)

Bug: 380310632
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4860245
On Android, compositor memory limits are derived from system memory and
dalvik memory limits. This code was noted as outdated in 2017 (see
linked bug), and as a result didn't work the way it should have.

This is because:
- Reported system RAM on Android is lower than installed RAM because of
  carveouts
- Dalvik memory limits are not really correlated with system RAM

Based on field data, the large majority of devices is running with a
computed limit of 256MiB, with some devices using 96MiB. This CL
simplifies the code to:
- Low-end or <2GiB: 96MiB
- Otherwise, 256MiB

Which mostly matches the current in-the-wild reality. These limits are
likely not optimal, but at least this simplifies the code. Also, lower
the priority cutoff to "NICE_TO_HAVE", which matches desktop, and
reality, since ALLOW_EVERYTHING is lowered to NICE_TO_HAVE elsewhere in
the code (see PriorityCutoffToTileMemoryLimitPolicy() for instance).

This is gated behind a feature flag, to make sure that this is not
breaking things.

This change is backported from m120+.

(cherry picked from commit 42b97eaa9deace1c188434ccac37ee22223ade4f)

Bug: 380310632
Reviewed-on:
https://chromium-review.googlesource.com/c/chromium/src/+/4860245
This removes the feature SimpleCompositorMemoryLimits, making its
behavior the default one. Based on Chrome Stable data, this reduces the
number of frames where we run out of space for required tiles by ~37%,
at no memory cost (note though that even prior to this patch, this was a
rare occurence). It also fixes code that has been known to be incorrect
for years (to detect system memory).

This change is backported from m126+.

(cherry picked from commit 1e631bbc9599b97b02626d36115cab30a675812e)

Bug: b/380310632
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5062876
SbAudioSink is a typedef to SbAudioSinkPrivate*, and the concrete
implementation of SbAudioSinkPrivate was in the global namespace. Now
it's moved into a new class named SbAudioSinkImpl in the namespace
starboard::shared::starboard::audio_sink, and it's derived from
SbAudioSinkPrivate, which is now an interface.

This allows to have multiple SbAudioSink implementations in different
namespaces.

b/327287075
This removes the feature SimpleCompositorMemoryLimits, making its
behavior the default one. Based on Chrome Stable data, this reduces the
number of frames where we run out of space for required tiles by ~37%,
at no memory cost (note though that even prior to this patch, this was a
rare occurence). It also fixes code that has been known to be incorrect
for years (to detect system memory).

This change is backported from m126+.

(cherry picked from commit 1e631bbc9599b97b02626d36115cab30a675812e)

Bug: 380310632
Reviewed-on:
https://chromium-review.googlesource.com/c/chromium/src/+/5062876
1. Enable low end device mode to reduce memory usage.
2. The rendering issue was due to the memory limits in Blink, which is
resolved in m126+, where two upstream commits
(#4636,
#4637) were cherry picked.

b/380310632
These sources were removed from nplb for android platforms during the
migration of nplb from older branches. I couldn't find a reason for
their removal, and adding them back resolves a gn include check.

b/377295011
… in main Kokoro build scripts.

Issue: 365150653
Reviewed-on: #4644
It's already exported to the environment by dind{,_builder}_runner.sh.

Issue: 365150653
Reviewed-on: #4647
We made a mistake during Starboard removal and called exit() during
suspend.

b/384086780
RGBA_4444 doesn't work with overlay video mode.
- Disable it as it is enabled with low-end-device-mode,
#4625.

b/380310632
Issue: 365150653
Reviewed-on: #4653
@madhurajayaraman madhurajayaraman requested review from a team as code owners January 7, 2025 22:20
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@madhurajayaraman madhurajayaraman requested review from dahlstrom-g and removed request for a team January 7, 2025 22:20
@datadog-cobalt-youtube
Copy link

datadog-cobalt-youtube bot commented Jan 7, 2025

Datadog Report

Branch report: main
Commit report: baffdd5
Test service: cobalt

✅ 0 Failed, 11665 Passed, 13 Skipped, 2m 18.4s Total Time

@TyHolc TyHolc self-requested a review January 7, 2025 23:00
Copy link
Contributor

@TyHolc TyHolc left a comment

Choose a reason for hiding this comment

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

These changes work for me as well. Let's keep this branch up to date if possible! LGTM

osagie98 and others added 4 commits January 8, 2025 00:50
This change implements a partial interface for SourceBuffer, adding the
writeHead attribute. This attribute returns the highest buffer
presentation timestamp written to the Renderer. The writeHead attribute
is only included in Cobalt Starboard media builds.

Link to the original PR with most of the functional changes:
#121

b/326652276
Removed initializing CommandLine from the file
`content-shell-command-line`, as Cobalt doesn't need it.

b/387917915
Disable low-end-device-mode as it cannot play 4k video smoothly.

b/388291268
Remove @UsedByNative annotations for beforeStartOrResume and
beforeSuspend, for they are no longer needed in ApplicationAndroid.
Reference b/377042903.

Remove getLocalInterfaceAddressAndNetmask and isCurrentNetworkWireless,
for it's not used in Chrobalt.

Remove @UsedByNative annotations for getDisplaySize, for it's never used
in native code.

Change UA related fields getIsAmatiDevice, getBuildFingerprint,
getPlayServicesVersion to @CalledByNative, we are currently not using
them in UserAgentPlatformInfo::ToString(), but might need them for
cobalt/browser/client_hint_headers.cc in the future.

Migrate the rest of JNI calls in
starboard/android/shared/android_main.cc to Chromium standard JNI,
When testing locally:
startDeepLink is set with empty string.
GetArgs retrieves url.
01-02 22:06:07.303 13993 13993 I starboard:
[dev.cobalt.coat/13993:0103/060607.300538(UTC):INFO:android_main.cc(76)]
Test GetArgs: android_main--url=https://www.youtube.com/tv

b/372559388
Copy link
Contributor

@andrewsavage1 andrewsavage1 left a comment

Choose a reason for hiding this comment

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

We can look into removing codeowner rules for this branch if that would be helpful

haozheng-cobalt and others added 3 commits January 8, 2025 10:21
This PR serves as an example of how we should migration @UsedByNative
annotated API to Chromium JNI standard.

Local test: https://paste.googleplex.com/5475365477416960

b/372559388
`SbMediaIsAudioSupported()`, `SbMediaIsSupported()`, and
`SbMediaIsVideoSupported()` were never part of the Starboard interface.

Their Sb prefixes were removed and renamed to `MediaIsAudioSupported()`,
`MediaIsSupported()`, and `MediaIsVideoSupported()`. They are also moved
from the global namespace into ::starboard::shared::starboard::media.

b/327287075
@madhurajayaraman madhurajayaraman merged commit bc843a1 into experimental/ozone Jan 8, 2025
113 checks passed
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.