From 56d05dcb75054cb64695de557b46b22d35c82a02 Mon Sep 17 00:00:00 2001 From: xiaomings Date: Mon, 30 Dec 2024 16:58:59 -0800 Subject: [PATCH 01/12] [media] Remove decode_target_internal.cc (#4631) 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 --- starboard/android/shared/BUILD.gn | 1 - .../decode_target/decode_target_internal.cc | 19 ------------------- .../decode_target/decode_target_internal.h | 4 ++-- 3 files changed, 2 insertions(+), 22 deletions(-) delete mode 100644 starboard/shared/starboard/decode_target/decode_target_internal.cc diff --git a/starboard/android/shared/BUILD.gn b/starboard/android/shared/BUILD.gn index c51409bf22c..cf4b70c4dd9 100644 --- a/starboard/android/shared/BUILD.gn +++ b/starboard/android/shared/BUILD.gn @@ -103,7 +103,6 @@ static_library("starboard_platform") { "//starboard/shared/starboard/command_line.cc", "//starboard/shared/starboard/command_line.h", "//starboard/shared/starboard/decode_target/decode_target_get_info.cc", - "//starboard/shared/starboard/decode_target/decode_target_internal.cc", "//starboard/shared/starboard/decode_target/decode_target_internal.h", "//starboard/shared/starboard/decode_target/decode_target_release.cc", "//starboard/shared/starboard/drm/drm_close_session.cc", diff --git a/starboard/shared/starboard/decode_target/decode_target_internal.cc b/starboard/shared/starboard/decode_target/decode_target_internal.cc deleted file mode 100644 index 29c40807efe..00000000000 --- a/starboard/shared/starboard/decode_target/decode_target_internal.cc +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2024 The Cobalt Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "starboard/shared/starboard/decode_target/decode_target_internal.h" - -SbDecodeTargetPrivate::SbDecodeTargetPrivate() = default; - -SbDecodeTargetPrivate::~SbDecodeTargetPrivate() = default; diff --git a/starboard/shared/starboard/decode_target/decode_target_internal.h b/starboard/shared/starboard/decode_target/decode_target_internal.h index f1db07cf338..779ad897ddc 100644 --- a/starboard/shared/starboard/decode_target/decode_target_internal.h +++ b/starboard/shared/starboard/decode_target/decode_target_internal.h @@ -20,7 +20,7 @@ #include "starboard/common/ref_counted.h" struct SbDecodeTargetPrivate : starboard::RefCounted { - SbDecodeTargetPrivate(); + SbDecodeTargetPrivate() = default; SbDecodeTargetPrivate(const SbDecodeTargetPrivate&) = delete; SbDecodeTargetPrivate& operator=(const SbDecodeTargetPrivate&) = delete; @@ -30,7 +30,7 @@ struct SbDecodeTargetPrivate : starboard::RefCounted { protected: friend class starboard::RefCounted; - virtual ~SbDecodeTargetPrivate(); + virtual ~SbDecodeTargetPrivate() = default; }; #endif // STARBOARD_SHARED_STARBOARD_DECODE_TARGET_DECODE_TARGET_INTERNAL_H_ From 8e7db49c1da7a1701b42d20f228a7cdda628ea61 Mon Sep 17 00:00:00 2001 From: Ian Mindich Date: Thu, 2 Jan 2025 11:10:34 -0800 Subject: [PATCH 02/12] Remove linux debug target, and change test results publisher to test-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. --- .github/actions/process_test_results/action.yaml | 13 +++++++++---- .github/workflows/main.yaml | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.github/actions/process_test_results/action.yaml b/.github/actions/process_test_results/action.yaml index f994480b7b2..bce228e5ad7 100644 --- a/.github/actions/process_test_results/action.yaml +++ b/.github/actions/process_test_results/action.yaml @@ -16,12 +16,17 @@ runs: name: ${{ inputs.test_results_key }} path: results/ - - name: Publish Test Report + - name: Test Summary action continue-on-error: true - uses: mikepenz/action-junit-report@992d97d6eb2e5f3de985fbf9df6a04386874114d + uses: test-summary/action@31493c76ec9e7aa675f1585d3ed6f1da69269a86 with: - report_paths: results/*.xml - skip_annotations: true + paths: "results/*.xml" + output: test-report-summary.md + show: "fail, skip" + + - name: Output test summary markdown to github + run: cat test-report-summary.md >> $GITHUB_STEP_SUMMARY + shell: bash - name: Get Datadog CLI id: download-dd-cli diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 45e7ef8e073..bfd96b5d001 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -133,7 +133,7 @@ jobs: matrix: platform: ${{ fromJson(needs.initialize.outputs.platforms) }} include: ${{ fromJson(needs.initialize.outputs.includes) }} - config: [devel, debug, qa, gold] + config: [devel, qa, gold] container: ${{ needs.docker-build-image.outputs.docker_tag }} env: TEST_ARTIFACTS_KEY: ${{ matrix.platform }}_${{ matrix.name }}_test_artifacts From 8e80e039354bf4a9a4882a80b96f5cf9303f4b29 Mon Sep 17 00:00:00 2001 From: Bo-Rong Chen Date: Thu, 2 Jan 2025 16:12:39 -0800 Subject: [PATCH 03/12] [android] Fix Cobalt crash in official build (#4617) 1. Ensure to initialize RecursiveMutex for SbLog before ApplicationAndroid is initialized. 2. Enable qa build with official build. b/384807408 --- cobalt/build/gn.py | 2 +- starboard/android/shared/android_main.cc | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cobalt/build/gn.py b/cobalt/build/gn.py index 06d86bc2a02..8e34d4e2b7c 100755 --- a/cobalt/build/gn.py +++ b/cobalt/build/gn.py @@ -56,7 +56,7 @@ def get_build_args(build_args_path): }, 'qa': { 'symbol_level': 1, - 'is_debug': 'false' + 'is_official_build': 'true' }, 'gold': { 'symbol_level': 0, diff --git a/starboard/android/shared/android_main.cc b/starboard/android/shared/android_main.cc index b16001e7a70..92b6632332e 100644 --- a/starboard/android/shared/android_main.cc +++ b/starboard/android/shared/android_main.cc @@ -30,6 +30,7 @@ #include "starboard/event.h" #include "starboard/log.h" #include "starboard/shared/starboard/command_line.h" +#include "starboard/shared/starboard/log_mutex.h" #include "starboard/thread.h" #if SB_IS(EVERGREEN_COMPATIBLE) #include "starboard/crashpad_wrapper/wrapper.h" // nogncheck @@ -310,6 +311,7 @@ Java_dev_cobalt_coat_StarboardBridge_startNativeStarboard(JniEnvExt* env) { #if SB_IS(EVERGREEN_COMPATIBLE) StarboardThreadLaunch(); #else + starboard::shared::starboard::GetLoggingMutex(); auto command_line = std::make_unique(GetArgs()); LogInit(*command_line); auto* nativeApp = new ApplicationAndroid(std::move(command_line)); From e663c95fcefac0f815fbe201f64fbeff52983392 Mon Sep 17 00:00:00 2001 From: Benoit Lize Date: Tue, 19 Sep 2023 09:37:17 +0000 Subject: [PATCH 04/12] [cc] Simplify and adjust memory limits on Android 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 --- .../widget/compositing/layer_tree_settings.cc | 129 ++++++++++-------- 1 file changed, 75 insertions(+), 54 deletions(-) diff --git a/third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc b/third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc index 3ebaee0026a..78a04665b3a 100644 --- a/third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc +++ b/third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc @@ -43,6 +43,15 @@ BASE_FEATURE(kScaleScrollbarAnimationTiming, "ScaleScrollbarAnimationTiming", base::FEATURE_DISABLED_BY_DEFAULT); +#if BUILDFLAG(IS_ANDROID) +// Whether to use a simpler way to compute compositor memory limits on +// Android. Intended to become default, but introduced temporarily to check +// it's not breaking things. +BASE_FEATURE(kSimpleCompositorMemoryLimits, + "SimpleCompositorMemoryLimits", + base::FEATURE_DISABLED_BY_DEFAULT); +#endif + constexpr base::FeatureParam kFadeDelayScalingFactor{ &kScaleScrollbarAnimationTiming, "fade_delay_scaling_factor", /*default_value=*/1.0}; @@ -105,65 +114,77 @@ cc::ManagedMemoryPolicy GetGpuMemoryPolicy( } #if BUILDFLAG(IS_ANDROID) - // We can't query available GPU memory from the system on Android. - // Physical memory is also mis-reported sometimes (eg. Nexus 10 reports - // 1262MB when it actually has 2GB, while Razr M has 1GB but only reports - // 128MB java heap size). First we estimate physical memory using both. - size_t dalvik_mb = base::SysInfo::DalvikHeapSizeMB(); - size_t physical_mb = base::SysInfo::AmountOfPhysicalMemoryMB(); - size_t physical_memory_mb = 0; - if (base::SysInfo::IsLowEndDevice()) { - // TODO(crbug.com/742534): The code below appears to no longer work. - // |dalvik_mb| no longer follows the expected heuristic pattern, causing us - // to over-estimate memory on low-end devices. This entire section probably - // needs to be re-written, but for now we can address the low-end Android - // issues by ignoring |dalvik_mb|. - physical_memory_mb = physical_mb; - } else if (dalvik_mb >= 256) { - physical_memory_mb = dalvik_mb * 4; + if (base::FeatureList::IsEnabled(kSimpleCompositorMemoryLimits)) { + if (base::SysInfo::IsLowEndDevice() || + base::SysInfo::AmountOfPhysicalMemoryMB() < 2000) { + actual.bytes_limit_when_visible = 96 * 1024 * 1024; + } else { + actual.bytes_limit_when_visible = 256 * 1024 * 1024; + } + actual.priority_cutoff_when_visible = + gpu::MemoryAllocation::CUTOFF_ALLOW_NICE_TO_HAVE; } else { - physical_memory_mb = std::max(dalvik_mb * 4, (physical_mb * 4) / 3); - } - - // Now we take a default of 1/8th of memory on high-memory devices, - // and gradually scale that back for low-memory devices (to be nicer - // to other apps so they don't get killed). Examples: - // Nexus 4/10(2GB) 256MB (normally 128MB) - // Droid Razr M(1GB) 114MB (normally 57MB) - // Galaxy Nexus(1GB) 100MB (normally 50MB) - // Xoom(1GB) 100MB (normally 50MB) - // Nexus S(low-end) 8MB (normally 8MB) - // Note that the compositor now uses only some of this memory for - // pre-painting and uses the rest only for 'emergencies'. - if (actual.bytes_limit_when_visible == 0) { - // NOTE: Non-low-end devices use only 50% of these limits, - // except during 'emergencies' where 100% can be used. - if (physical_memory_mb >= 1536) { - actual.bytes_limit_when_visible = physical_memory_mb / 8; // >192MB - } else if (physical_memory_mb >= 1152) { - actual.bytes_limit_when_visible = physical_memory_mb / 8; // >144MB - } else if (physical_memory_mb >= 768) { - actual.bytes_limit_when_visible = physical_memory_mb / 10; // >76MB - } else if (physical_memory_mb >= 513) { - actual.bytes_limit_when_visible = physical_memory_mb / 12; // <64MB + // We can't query available GPU memory from the system on Android. + // Physical memory is also mis-reported sometimes (eg. Nexus 10 reports + // 1262MB when it actually has 2GB, while Razr M has 1GB but only reports + // 128MB java heap size). First we estimate physical memory using both. + size_t dalvik_mb = base::SysInfo::DalvikHeapSizeMB(); + size_t physical_mb = base::SysInfo::AmountOfPhysicalMemoryMB(); + size_t physical_memory_mb = 0; + if (base::SysInfo::IsLowEndDevice()) { + // TODO(crbug.com/742534): The code below appears to no longer work. + // |dalvik_mb| no longer follows the expected heuristic pattern, causing + // us to over-estimate memory on low-end devices. This entire section + // probably needs to be re-written, but for now we can address the low-end + // Android issues by ignoring |dalvik_mb|. + physical_memory_mb = physical_mb; + } else if (dalvik_mb >= 256) { + physical_memory_mb = dalvik_mb * 4; } else { - // Devices with this little RAM have very little headroom so we hardcode - // the limit rather than relying on the heuristics above. (They also use - // 4444 textures so we can use a lower limit.) - actual.bytes_limit_when_visible = 8; + physical_memory_mb = std::max(dalvik_mb * 4, (physical_mb * 4) / 3); } - actual.bytes_limit_when_visible = - actual.bytes_limit_when_visible * 1024 * 1024; - // Clamp the observed value to a specific range on Android. - actual.bytes_limit_when_visible = std::max( - actual.bytes_limit_when_visible, static_cast(8 * 1024 * 1024)); - actual.bytes_limit_when_visible = - std::min(actual.bytes_limit_when_visible, - static_cast(256 * 1024 * 1024)); + // Now we take a default of 1/8th of memory on high-memory devices, + // and gradually scale that back for low-memory devices (to be nicer + // to other apps so they don't get killed). Examples: + // Nexus 4/10(2GB) 256MB (normally 128MB) + // Droid Razr M(1GB) 114MB (normally 57MB) + // Galaxy Nexus(1GB) 100MB (normally 50MB) + // Xoom(1GB) 100MB (normally 50MB) + // Nexus S(low-end) 8MB (normally 8MB) + // Note that the compositor now uses only some of this memory for + // pre-painting and uses the rest only for 'emergencies'. + if (actual.bytes_limit_when_visible == 0) { + // NOTE: Non-low-end devices use only 50% of these limits, + // except during 'emergencies' where 100% can be used. + if (physical_memory_mb >= 1536) { + actual.bytes_limit_when_visible = physical_memory_mb / 8; // >192MB + } else if (physical_memory_mb >= 1152) { + actual.bytes_limit_when_visible = physical_memory_mb / 8; // >144MB + } else if (physical_memory_mb >= 768) { + actual.bytes_limit_when_visible = physical_memory_mb / 10; // >76MB + } else if (physical_memory_mb >= 513) { + actual.bytes_limit_when_visible = physical_memory_mb / 12; // <64MB + } else { + // Devices with this little RAM have very little headroom so we hardcode + // the limit rather than relying on the heuristics above. (They also + // use 4444 textures so we can use a lower limit.) + actual.bytes_limit_when_visible = 8; + } + + actual.bytes_limit_when_visible = + actual.bytes_limit_when_visible * 1024 * 1024; + // Clamp the observed value to a specific range on Android. + actual.bytes_limit_when_visible = + std::max(actual.bytes_limit_when_visible, + static_cast(8 * 1024 * 1024)); + actual.bytes_limit_when_visible = + std::min(actual.bytes_limit_when_visible, + static_cast(256 * 1024 * 1024)); + } + actual.priority_cutoff_when_visible = + gpu::MemoryAllocation::CUTOFF_ALLOW_EVERYTHING; } - actual.priority_cutoff_when_visible = - gpu::MemoryAllocation::CUTOFF_ALLOW_EVERYTHING; #else // Ignore what the system said and give all clients the same maximum // allocation on desktop platforms. From 32040d2085eb14fab82ee30d0cd4d4aa122aac98 Mon Sep 17 00:00:00 2001 From: Benoit Lize Date: Wed, 29 Nov 2023 17:51:27 +0000 Subject: [PATCH 05/12] [cc] Use simple compositor memory limits on Android 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 --- .../widget/compositing/layer_tree_settings.cc | 87 ++----------------- 1 file changed, 7 insertions(+), 80 deletions(-) diff --git a/third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc b/third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc index 78a04665b3a..0a0888ceb3d 100644 --- a/third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc +++ b/third_party/blink/renderer/platform/widget/compositing/layer_tree_settings.cc @@ -43,15 +43,6 @@ BASE_FEATURE(kScaleScrollbarAnimationTiming, "ScaleScrollbarAnimationTiming", base::FEATURE_DISABLED_BY_DEFAULT); -#if BUILDFLAG(IS_ANDROID) -// Whether to use a simpler way to compute compositor memory limits on -// Android. Intended to become default, but introduced temporarily to check -// it's not breaking things. -BASE_FEATURE(kSimpleCompositorMemoryLimits, - "SimpleCompositorMemoryLimits", - base::FEATURE_DISABLED_BY_DEFAULT); -#endif - constexpr base::FeatureParam kFadeDelayScalingFactor{ &kScaleScrollbarAnimationTiming, "fade_delay_scaling_factor", /*default_value=*/1.0}; @@ -114,83 +105,16 @@ cc::ManagedMemoryPolicy GetGpuMemoryPolicy( } #if BUILDFLAG(IS_ANDROID) - if (base::FeatureList::IsEnabled(kSimpleCompositorMemoryLimits)) { - if (base::SysInfo::IsLowEndDevice() || - base::SysInfo::AmountOfPhysicalMemoryMB() < 2000) { - actual.bytes_limit_when_visible = 96 * 1024 * 1024; - } else { - actual.bytes_limit_when_visible = 256 * 1024 * 1024; - } - actual.priority_cutoff_when_visible = - gpu::MemoryAllocation::CUTOFF_ALLOW_NICE_TO_HAVE; + if (base::SysInfo::IsLowEndDevice() || + base::SysInfo::AmountOfPhysicalMemoryMB() < 2000) { + actual.bytes_limit_when_visible = 96 * 1024 * 1024; } else { - // We can't query available GPU memory from the system on Android. - // Physical memory is also mis-reported sometimes (eg. Nexus 10 reports - // 1262MB when it actually has 2GB, while Razr M has 1GB but only reports - // 128MB java heap size). First we estimate physical memory using both. - size_t dalvik_mb = base::SysInfo::DalvikHeapSizeMB(); - size_t physical_mb = base::SysInfo::AmountOfPhysicalMemoryMB(); - size_t physical_memory_mb = 0; - if (base::SysInfo::IsLowEndDevice()) { - // TODO(crbug.com/742534): The code below appears to no longer work. - // |dalvik_mb| no longer follows the expected heuristic pattern, causing - // us to over-estimate memory on low-end devices. This entire section - // probably needs to be re-written, but for now we can address the low-end - // Android issues by ignoring |dalvik_mb|. - physical_memory_mb = physical_mb; - } else if (dalvik_mb >= 256) { - physical_memory_mb = dalvik_mb * 4; - } else { - physical_memory_mb = std::max(dalvik_mb * 4, (physical_mb * 4) / 3); - } - - // Now we take a default of 1/8th of memory on high-memory devices, - // and gradually scale that back for low-memory devices (to be nicer - // to other apps so they don't get killed). Examples: - // Nexus 4/10(2GB) 256MB (normally 128MB) - // Droid Razr M(1GB) 114MB (normally 57MB) - // Galaxy Nexus(1GB) 100MB (normally 50MB) - // Xoom(1GB) 100MB (normally 50MB) - // Nexus S(low-end) 8MB (normally 8MB) - // Note that the compositor now uses only some of this memory for - // pre-painting and uses the rest only for 'emergencies'. - if (actual.bytes_limit_when_visible == 0) { - // NOTE: Non-low-end devices use only 50% of these limits, - // except during 'emergencies' where 100% can be used. - if (physical_memory_mb >= 1536) { - actual.bytes_limit_when_visible = physical_memory_mb / 8; // >192MB - } else if (physical_memory_mb >= 1152) { - actual.bytes_limit_when_visible = physical_memory_mb / 8; // >144MB - } else if (physical_memory_mb >= 768) { - actual.bytes_limit_when_visible = physical_memory_mb / 10; // >76MB - } else if (physical_memory_mb >= 513) { - actual.bytes_limit_when_visible = physical_memory_mb / 12; // <64MB - } else { - // Devices with this little RAM have very little headroom so we hardcode - // the limit rather than relying on the heuristics above. (They also - // use 4444 textures so we can use a lower limit.) - actual.bytes_limit_when_visible = 8; - } - - actual.bytes_limit_when_visible = - actual.bytes_limit_when_visible * 1024 * 1024; - // Clamp the observed value to a specific range on Android. - actual.bytes_limit_when_visible = - std::max(actual.bytes_limit_when_visible, - static_cast(8 * 1024 * 1024)); - actual.bytes_limit_when_visible = - std::min(actual.bytes_limit_when_visible, - static_cast(256 * 1024 * 1024)); - } - actual.priority_cutoff_when_visible = - gpu::MemoryAllocation::CUTOFF_ALLOW_EVERYTHING; + actual.bytes_limit_when_visible = 256 * 1024 * 1024; } #else // Ignore what the system said and give all clients the same maximum // allocation on desktop platforms. actual.bytes_limit_when_visible = 512 * 1024 * 1024; - actual.priority_cutoff_when_visible = - gpu::MemoryAllocation::CUTOFF_ALLOW_NICE_TO_HAVE; // For large monitors (4k), double the tile memory to avoid frequent out of // memory problems. 4k could mean a screen width of anywhere from 3840 to 4096 @@ -202,6 +126,9 @@ cc::ManagedMemoryPolicy GetGpuMemoryPolicy( if (display_width >= kLargeDisplayThreshold) actual.bytes_limit_when_visible *= 2; #endif + actual.priority_cutoff_when_visible = + gpu::MemoryAllocation::CUTOFF_ALLOW_NICE_TO_HAVE; + return actual; } From affac16210b3c2e14f6c645ba7934a8824b6c5f2 Mon Sep 17 00:00:00 2001 From: Holden Warriner Date: Fri, 3 Jan 2025 10:38:19 -0500 Subject: [PATCH 06/12] Remove unneeded #include and nogncheck annotation (#4633) b/377295011 --- starboard/nplb/player_test_util.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/starboard/nplb/player_test_util.cc b/starboard/nplb/player_test_util.cc index 01bfd49c683..ec9bf4ac601 100644 --- a/starboard/nplb/player_test_util.cc +++ b/starboard/nplb/player_test_util.cc @@ -24,8 +24,6 @@ #include "starboard/nplb/maximum_player_configuration_explorer.h" #include "starboard/nplb/player_creation_param_helpers.h" #include "starboard/shared/starboard/player/video_dmp_reader.h" -// TODO(cobalt, b/377295011): remove the nogncheck annotation. -#include "starboard/testing/fake_graphics_context_provider.h" // nogncheck #include "testing/gtest/include/gtest/gtest.h" namespace starboard { @@ -40,7 +38,6 @@ using std::placeholders::_1; using std::placeholders::_2; using std::placeholders::_3; using std::placeholders::_4; -using testing::FakeGraphicsContextProvider; const char* kAudioTestFiles[] = { "beneath_the_canopy_aac_stereo.dmp", From a6d7996af37e3df5e838e0bd3842c3d5b81a8002 Mon Sep 17 00:00:00 2001 From: xiaomings Date: Fri, 3 Jan 2025 13:35:17 -0800 Subject: [PATCH 07/12] [media] Move concrete SbAudioSink impl to namespace (#4634) 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 --- .../android/shared/application_android.cc | 2 +- .../shared/audio_track_audio_sink_type.cc | 16 +++++-- .../shared/audio_track_audio_sink_type.h | 3 +- .../shared/player_components_factory.h | 3 +- starboard/android/shared/starboard_bridge.cc | 2 +- .../shared/audio_sink_type_dispatcher.cc | 26 ++++++---- .../raspi/shared/application_dispmanx.cc | 4 +- .../shared/audio_sink_type_dispatcher.cc | 18 +++++-- starboard/shared/alsa/alsa_audio_sink_type.cc | 31 ++++++------ .../shared/pulse/pulse_audio_sink_type.cc | 11 +++-- .../starboard/audio_sink/audio_sink_create.cc | 2 +- .../audio_sink/audio_sink_destroy.cc | 6 ++- .../audio_sink/audio_sink_internal.cc | 48 +++++++++++-------- .../audio_sink/audio_sink_internal.h | 16 ++++++- .../audio_sink/audio_sink_is_valid.cc | 6 ++- .../player/filter/audio_renderer_sink_impl.cc | 2 +- starboard/shared/x11/application_x11.cc | 4 +- 17 files changed, 127 insertions(+), 73 deletions(-) diff --git a/starboard/android/shared/application_android.cc b/starboard/android/shared/application_android.cc index 1ac2878e532..8d5122bc375 100644 --- a/starboard/android/shared/application_android.cc +++ b/starboard/android/shared/application_android.cc @@ -68,7 +68,7 @@ ApplicationAndroid::ApplicationAndroid( "getResourceOverlay", "()Ldev/cobalt/coat/ResourceOverlay;"); resource_overlay_ = env->ConvertLocalRefToGlobalRef(local_ref); - SbAudioSinkPrivate::Initialize(); + ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl::Initialize(); app_start_timestamp_ = starboard_bridge_->GetAppStartTimestamp(); diff --git a/starboard/android/shared/audio_track_audio_sink_type.cc b/starboard/android/shared/audio_track_audio_sink_type.cc index 4af3a1751f1..cb7845c1fae 100644 --- a/starboard/android/shared/audio_track_audio_sink_type.cc +++ b/starboard/android/shared/audio_track_audio_sink_type.cc @@ -561,20 +561,30 @@ int AudioTrackAudioSinkType::GetMinBufferSizeInFramesInternal( } // namespace android } // namespace starboard +namespace starboard { +namespace shared { +namespace starboard { +namespace audio_sink { + // static -void SbAudioSinkPrivate::PlatformInitialize() { +void SbAudioSinkImpl::PlatformInitialize() { SB_DCHECK(!audio_track_audio_sink_type_); audio_track_audio_sink_type_ = - new starboard::android::shared::AudioTrackAudioSinkType; + new ::starboard::android::shared::AudioTrackAudioSinkType; SetPrimaryType(audio_track_audio_sink_type_); EnableFallbackToStub(); audio_track_audio_sink_type_->TestMinRequiredFrames(); } // static -void SbAudioSinkPrivate::PlatformTearDown() { +void SbAudioSinkImpl::PlatformTearDown() { SB_DCHECK(audio_track_audio_sink_type_ == GetPrimaryType()); SetPrimaryType(NULL); delete audio_track_audio_sink_type_; audio_track_audio_sink_type_ = NULL; } + +} // namespace audio_sink +} // namespace starboard +} // namespace shared +} // namespace starboard diff --git a/starboard/android/shared/audio_track_audio_sink_type.h b/starboard/android/shared/audio_track_audio_sink_type.h index 9d59c8049e9..700f421094e 100644 --- a/starboard/android/shared/audio_track_audio_sink_type.h +++ b/starboard/android/shared/audio_track_audio_sink_type.h @@ -100,7 +100,8 @@ class AudioTrackAudioSinkType : public SbAudioSinkPrivate::Type { bool has_remote_audio_output_ = false; }; -class AudioTrackAudioSink : public SbAudioSinkPrivate { +class AudioTrackAudioSink + : public ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl { public: AudioTrackAudioSink( Type* type, diff --git a/starboard/android/shared/player_components_factory.h b/starboard/android/shared/player_components_factory.h index 469fd669746..d17e7489a6e 100644 --- a/starboard/android/shared/player_components_factory.h +++ b/starboard/android/shared/player_components_factory.h @@ -99,7 +99,8 @@ class AudioRendererSinkAndroid : public ::starboard::shared::starboard::player:: SbAudioSinkPrivate::ErrorFunc error_func, void* context) { auto type = static_cast( - SbAudioSinkPrivate::GetPreferredType()); + ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl:: + GetPreferredType()); return type->Create( channels, sampling_frequency_hz, audio_sample_type, diff --git a/starboard/android/shared/starboard_bridge.cc b/starboard/android/shared/starboard_bridge.cc index d592e3b2ec7..5dcd951b471 100644 --- a/starboard/android/shared/starboard_bridge.cc +++ b/starboard/android/shared/starboard_bridge.cc @@ -27,7 +27,7 @@ namespace android { namespace shared { extern "C" SB_EXPORT_PLATFORM void JNI_StarboardBridge_OnStop(JNIEnv* env) { - SbAudioSinkPrivate::TearDown(); + ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl::TearDown(); SbFileAndroidTeardown(); } diff --git a/starboard/linux/shared/audio_sink_type_dispatcher.cc b/starboard/linux/shared/audio_sink_type_dispatcher.cc index 2a0b5817f93..4d956205125 100644 --- a/starboard/linux/shared/audio_sink_type_dispatcher.cc +++ b/starboard/linux/shared/audio_sink_type_dispatcher.cc @@ -17,28 +17,38 @@ #include "starboard/shared/pulse/pulse_audio_sink_type.h" #include "starboard/shared/starboard/audio_sink/audio_sink_internal.h" +namespace starboard { +namespace shared { +namespace starboard { +namespace audio_sink { namespace { bool is_fallback_to_alsa = false; -} +} // namespace // static -void SbAudioSinkPrivate::PlatformInitialize() { - starboard::shared::pulse::PlatformInitialize(); +void SbAudioSinkImpl::PlatformInitialize() { + ::starboard::shared::pulse::PlatformInitialize(); if (GetPrimaryType()) { SB_LOG(INFO) << "Use PulseAudio"; } else { SB_LOG(INFO) << "Use ALSA"; - starboard::shared::alsa::PlatformInitialize(); + ::starboard::shared::alsa::PlatformInitialize(); is_fallback_to_alsa = true; } - SbAudioSinkPrivate::EnableFallbackToStub(); + ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl:: + EnableFallbackToStub(); } // static -void SbAudioSinkPrivate::PlatformTearDown() { +void SbAudioSinkImpl::PlatformTearDown() { if (is_fallback_to_alsa) { - starboard::shared::alsa::PlatformTearDown(); + ::starboard::shared::alsa::PlatformTearDown(); } else { - starboard::shared::pulse::PlatformTearDown(); + ::starboard::shared::pulse::PlatformTearDown(); } } + +} // namespace audio_sink +} // namespace starboard +} // namespace shared +} // namespace starboard diff --git a/starboard/raspi/shared/application_dispmanx.cc b/starboard/raspi/shared/application_dispmanx.cc index ef924b0e92b..a5987c21664 100644 --- a/starboard/raspi/shared/application_dispmanx.cc +++ b/starboard/raspi/shared/application_dispmanx.cc @@ -68,12 +68,12 @@ bool ApplicationDispmanx::DestroyWindow(SbWindow window) { } void ApplicationDispmanx::Initialize() { - SbAudioSinkPrivate::Initialize(); + ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl::Initialize(); } void ApplicationDispmanx::Teardown() { ShutdownDispmanx(); - SbAudioSinkPrivate::TearDown(); + ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl::TearDown(); } void ApplicationDispmanx ::OnSuspend() { diff --git a/starboard/raspi/shared/audio_sink_type_dispatcher.cc b/starboard/raspi/shared/audio_sink_type_dispatcher.cc index 97b918d0f79..70a8d746154 100644 --- a/starboard/raspi/shared/audio_sink_type_dispatcher.cc +++ b/starboard/raspi/shared/audio_sink_type_dispatcher.cc @@ -15,13 +15,23 @@ #include "starboard/shared/alsa/alsa_audio_sink_type.h" #include "starboard/shared/starboard/audio_sink/audio_sink_internal.h" +namespace starboard { +namespace shared { +namespace starboard { +namespace audio_sink { + // static -void SbAudioSinkPrivate::PlatformInitialize() { - starboard::shared::alsa::PlatformInitialize(); +void SbAudioSinkImpl::PlatformInitialize() { + ::starboard::shared::alsa::PlatformInitialize(); SbAudioSinkPrivate::EnableFallbackToStub(); } // static -void SbAudioSinkPrivate::PlatformTearDown() { - starboard::shared::alsa::PlatformTearDown(); +void SbAudioSinkImpl::PlatformTearDown() { + ::starboard::shared::alsa::PlatformTearDown(); } + +} // namespace audio_sink +} // namespace starboard +} // namespace shared +} // namespace starboard diff --git a/starboard/shared/alsa/alsa_audio_sink_type.cc b/starboard/shared/alsa/alsa_audio_sink_type.cc index 08bfd20bba3..8a683d94356 100644 --- a/starboard/shared/alsa/alsa_audio_sink_type.cc +++ b/starboard/shared/alsa/alsa_audio_sink_type.cc @@ -36,10 +36,11 @@ namespace shared { namespace alsa { namespace { -using starboard::ScopedLock; -using starboard::ScopedTryLock; -using starboard::shared::alsa::AlsaGetBufferedFrames; -using starboard::shared::alsa::AlsaWriteFrames; +using ::starboard::ScopedLock; +using ::starboard::ScopedTryLock; +using ::starboard::shared::alsa::AlsaGetBufferedFrames; +using ::starboard::shared::alsa::AlsaWriteFrames; +using ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl; // The maximum number of frames that can be written to ALSA once. It must be a // power of 2. It is also used as the ALSA polling size. A small number will @@ -81,7 +82,7 @@ void* IncrementPointerByBytes(void* pointer, size_t offset) { // 2. It never stops the underlying ALSA audio sink once created. When its // source cannot provide enough data to continue playback, it simply writes // silence to ALSA. -class AlsaAudioSink : public SbAudioSinkPrivate { +class AlsaAudioSink : public SbAudioSinkImpl { public: AlsaAudioSink(Type* type, int channels, @@ -142,8 +143,8 @@ class AlsaAudioSink : public SbAudioSinkPrivate { SbMediaAudioSampleType sample_type_; pthread_t audio_out_thread_; - starboard::Mutex mutex_; - starboard::ConditionVariable creation_signal_; + ::starboard::Mutex mutex_; + ::starboard::ConditionVariable creation_signal_; int64_t time_to_wait_; @@ -215,7 +216,7 @@ AlsaAudioSink::~AlsaAudioSink() { // static void* AlsaAudioSink::ThreadEntryPoint(void* context) { pthread_setname_np(pthread_self(), "alsa_audio_out"); - starboard::shared::pthread::ThreadSetPriority(kSbThreadPriorityRealTime); + ::starboard::shared::pthread::ThreadSetPriority(kSbThreadPriorityRealTime); SB_DCHECK(context); AlsaAudioSink* sink = reinterpret_cast(context); sink->AudioThreadFunc(); @@ -228,7 +229,7 @@ void AlsaAudioSink::AudioThreadFunc() { sample_type_ == kSbMediaAudioSampleTypeFloat32 ? SND_PCM_FORMAT_FLOAT_LE : SND_PCM_FORMAT_S16; - playback_handle_ = starboard::shared::alsa::AlsaOpenPlaybackDevice( + playback_handle_ = ::starboard::shared::alsa::AlsaOpenPlaybackDevice( channels_, sampling_frequency_hz_, kFramesPerRequest, kALSABufferSizeInFrames, alsa_sample_type); { @@ -249,7 +250,7 @@ void AlsaAudioSink::AudioThreadFunc() { } } - starboard::shared::alsa::AlsaCloseDevice(playback_handle_); + ::starboard::shared::alsa::AlsaCloseDevice(playback_handle_); ScopedLock lock(mutex_); playback_handle_ = NULL; } @@ -450,25 +451,23 @@ SbAudioSink AlsaAudioSinkType::Create( return audio_sink; } -} // namespace - -namespace { AlsaAudioSinkType* alsa_audio_sink_type_; + } // namespace // static void PlatformInitialize() { SB_DCHECK(!alsa_audio_sink_type_); alsa_audio_sink_type_ = new AlsaAudioSinkType(); - SbAudioSinkPrivate::SetPrimaryType(alsa_audio_sink_type_); + SbAudioSinkImpl::SetPrimaryType(alsa_audio_sink_type_); } // static void PlatformTearDown() { SB_DCHECK(alsa_audio_sink_type_); - SB_DCHECK(alsa_audio_sink_type_ == SbAudioSinkPrivate::GetPrimaryType()); + SB_DCHECK(alsa_audio_sink_type_ == SbAudioSinkImpl::GetPrimaryType()); - SbAudioSinkPrivate::SetPrimaryType(NULL); + SbAudioSinkImpl::SetPrimaryType(NULL); delete alsa_audio_sink_type_; alsa_audio_sink_type_ = NULL; } diff --git a/starboard/shared/pulse/pulse_audio_sink_type.cc b/starboard/shared/pulse/pulse_audio_sink_type.cc index eb091fc508e..ccbcc83b076 100644 --- a/starboard/shared/pulse/pulse_audio_sink_type.cc +++ b/starboard/shared/pulse/pulse_audio_sink_type.cc @@ -53,6 +53,7 @@ namespace pulse { namespace { using starboard::media::GetBytesPerSample; +using ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl; const int64_t kAudioIdleSleepIntervalUsec = 15'000; // 15ms const int64_t kAudioRunningSleepIntervalUsec = 5'000; // 5ms @@ -65,7 +66,7 @@ const size_t kPulseBufferSizeInFrames = 8192; class PulseAudioSinkType; -class PulseAudioSink : public SbAudioSinkPrivate { +class PulseAudioSink : public SbAudioSinkImpl { public: PulseAudioSink(PulseAudioSinkType* type, int channels, @@ -554,7 +555,7 @@ void PulseAudioSinkType::StateCallback(pa_context* context, void* userdata) { void* PulseAudioSinkType::ThreadEntryPoint(void* context) { pthread_setname_np(pthread_self(), "pulse_audio"); - shared::pthread::ThreadSetPriority(kSbThreadPriorityRealTime); + ::starboard::shared::pthread::ThreadSetPriority(kSbThreadPriorityRealTime); SB_DCHECK(context); PulseAudioSinkType* type = static_cast(context); @@ -602,16 +603,16 @@ void PlatformInitialize() { std::unique_ptr(new PulseAudioSinkType()); if (audio_sink_type->Initialize()) { pulse_audio_sink_type_ = audio_sink_type.release(); - SbAudioSinkPrivate::SetPrimaryType(pulse_audio_sink_type_); + SbAudioSinkImpl::SetPrimaryType(pulse_audio_sink_type_); } } // static void PlatformTearDown() { SB_DCHECK(pulse_audio_sink_type_); - SB_DCHECK(pulse_audio_sink_type_ == SbAudioSinkPrivate::GetPrimaryType()); + SB_DCHECK(pulse_audio_sink_type_ == SbAudioSinkImpl::GetPrimaryType()); - SbAudioSinkPrivate::SetPrimaryType(NULL); + SbAudioSinkImpl::SetPrimaryType(NULL); delete pulse_audio_sink_type_; pulse_audio_sink_type_ = NULL; pulse_unload_library(); diff --git a/starboard/shared/starboard/audio_sink/audio_sink_create.cc b/starboard/shared/starboard/audio_sink/audio_sink_create.cc index 7061745ff81..14867aa7e3e 100644 --- a/starboard/shared/starboard/audio_sink/audio_sink_create.cc +++ b/starboard/shared/starboard/audio_sink/audio_sink_create.cc @@ -27,7 +27,7 @@ SbAudioSink SbAudioSinkCreate( SbAudioSinkUpdateSourceStatusFunc update_source_status_func, SbAudioSinkConsumeFramesFunc consume_frames_func, void* context) { - return SbAudioSinkPrivate::Create( + return ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl::Create( channels, sampling_frequency_hz, audio_sample_type, audio_frame_storage_type, frame_buffers, frame_buffers_size_in_frames, update_source_status_func, consume_frames_func, NULL /*error_func*/, diff --git a/starboard/shared/starboard/audio_sink/audio_sink_destroy.cc b/starboard/shared/starboard/audio_sink/audio_sink_destroy.cc index 4dea688d960..ac0b1abd978 100644 --- a/starboard/shared/starboard/audio_sink/audio_sink_destroy.cc +++ b/starboard/shared/starboard/audio_sink/audio_sink_destroy.cc @@ -18,16 +18,18 @@ #include "starboard/shared/starboard/audio_sink/audio_sink_internal.h" void SbAudioSinkDestroy(SbAudioSink audio_sink) { + using ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl; + if (audio_sink == kSbAudioSinkInvalid) { return; } - SbAudioSinkPrivate::Type* type = SbAudioSinkPrivate::GetPrimaryType(); + SbAudioSinkPrivate::Type* type = SbAudioSinkImpl::GetPrimaryType(); if (type && type->IsValid(audio_sink)) { type->Destroy(audio_sink); return; } - type = SbAudioSinkPrivate::GetFallbackType(); + type = SbAudioSinkImpl::GetFallbackType(); if (type && type->IsValid(audio_sink)) { type->Destroy(audio_sink); return; diff --git a/starboard/shared/starboard/audio_sink/audio_sink_internal.cc b/starboard/shared/starboard/audio_sink/audio_sink_internal.cc index 450c4186928..5fb1ded88b8 100644 --- a/starboard/shared/starboard/audio_sink/audio_sink_internal.cc +++ b/starboard/shared/starboard/audio_sink/audio_sink_internal.cc @@ -21,6 +21,10 @@ #include "starboard/shared/starboard/audio_sink/stub_audio_sink_type.h" #include "starboard/shared/starboard/command_line.h" +namespace starboard { +namespace shared { +namespace starboard { +namespace audio_sink { namespace { using std::placeholders::_1; @@ -28,8 +32,8 @@ using std::placeholders::_2; using std::placeholders::_3; bool is_fallback_to_stub_enabled; -SbAudioSinkPrivate::Type* primary_audio_sink_type; -SbAudioSinkPrivate::Type* fallback_audio_sink_type; +SbAudioSinkImpl::Type* primary_audio_sink_type; +SbAudioSinkImpl::Type* fallback_audio_sink_type; // Command line switch that controls whether we default to the stub audio sink, // even when the primary audio sink may be available. @@ -45,36 +49,34 @@ void WrapConsumeFramesFunc(SbAudioSinkConsumeFramesFunc sb_consume_frames_func, } // namespace -using starboard::shared::starboard::audio_sink::StubAudioSinkType; - // static -void SbAudioSinkPrivate::Initialize() { +void SbAudioSinkImpl::Initialize() { fallback_audio_sink_type = new StubAudioSinkType; PlatformInitialize(); } // static -void SbAudioSinkPrivate::TearDown() { +void SbAudioSinkImpl::TearDown() { PlatformTearDown(); delete fallback_audio_sink_type; fallback_audio_sink_type = NULL; } // static -void SbAudioSinkPrivate::SetPrimaryType(Type* type) { +void SbAudioSinkImpl::SetPrimaryType(Type* type) { primary_audio_sink_type = type; } // static -SbAudioSinkPrivate::Type* SbAudioSinkPrivate::GetPrimaryType() { +SbAudioSinkImpl::Type* SbAudioSinkImpl::GetPrimaryType() { return primary_audio_sink_type; } // static -void SbAudioSinkPrivate::EnableFallbackToStub() { +void SbAudioSinkImpl::EnableFallbackToStub() { is_fallback_to_stub_enabled = true; } // static -SbAudioSinkPrivate::Type* SbAudioSinkPrivate::GetFallbackType() { +SbAudioSinkImpl::Type* SbAudioSinkImpl::GetFallbackType() { if (is_fallback_to_stub_enabled) { return fallback_audio_sink_type; } @@ -82,17 +84,16 @@ SbAudioSinkPrivate::Type* SbAudioSinkPrivate::GetFallbackType() { } // static -SbAudioSinkPrivate::Type* SbAudioSinkPrivate::GetPreferredType() { - SbAudioSinkPrivate::Type* audio_sink_type = NULL; - auto command_line = - starboard::shared::starboard::Application::Get()->GetCommandLine(); +SbAudioSinkImpl::Type* SbAudioSinkImpl::GetPreferredType() { + SbAudioSinkImpl::Type* audio_sink_type = NULL; + auto command_line = Application::Get()->GetCommandLine(); if (!command_line->HasSwitch(kUseStubAudioSink)) { - audio_sink_type = SbAudioSinkPrivate::GetPrimaryType(); + audio_sink_type = SbAudioSinkImpl::GetPrimaryType(); } if (!audio_sink_type) { SB_LOG(WARNING) << "Primary audio sink type not selected or missing, " "opting to use Fallback instead."; - audio_sink_type = SbAudioSinkPrivate::GetFallbackType(); + audio_sink_type = SbAudioSinkImpl::GetFallbackType(); } if (audio_sink_type == NULL) { SB_LOG(WARNING) << "Fallback audio sink type is not enabled."; @@ -100,7 +101,7 @@ SbAudioSinkPrivate::Type* SbAudioSinkPrivate::GetPreferredType() { return audio_sink_type; } -SbAudioSink SbAudioSinkPrivate::Create( +SbAudioSink SbAudioSinkImpl::Create( int channels, int sampling_frequency_hz, SbMediaAudioSampleType audio_sample_type, @@ -169,7 +170,7 @@ SbAudioSink SbAudioSinkPrivate::Create( } SB_LOG(WARNING) << "Try to create AudioSink using fallback type."; - if (auto fallback_type = SbAudioSinkPrivate::GetFallbackType()) { + if (auto fallback_type = SbAudioSinkImpl::GetFallbackType()) { auto audio_sink = fallback_type->Create( channels, sampling_frequency_hz, audio_sample_type, audio_frame_storage_type, frame_buffers, frame_buffers_size_in_frames, @@ -186,7 +187,7 @@ SbAudioSink SbAudioSinkPrivate::Create( } // static -SbAudioSink SbAudioSinkPrivate::Create( +SbAudioSink SbAudioSinkImpl::Create( int channels, int sampling_frequency_hz, SbMediaAudioSampleType audio_sample_type, @@ -201,8 +202,13 @@ SbAudioSink SbAudioSinkPrivate::Create( audio_frame_storage_type, frame_buffers, frame_buffers_size_in_frames, update_source_status_func, sb_consume_frames_func - ? std::bind(&::WrapConsumeFramesFunc, - sb_consume_frames_func, _1, _2, _3) + ? std::bind(&WrapConsumeFramesFunc, sb_consume_frames_func, + _1, _2, _3) : ConsumeFramesFunc(), error_func, context); } + +} // namespace audio_sink +} // namespace starboard +} // namespace shared +} // namespace starboard diff --git a/starboard/shared/starboard/audio_sink/audio_sink_internal.h b/starboard/shared/starboard/audio_sink/audio_sink_internal.h index fa0cc550d1f..35c628c9f01 100644 --- a/starboard/shared/starboard/audio_sink/audio_sink_internal.h +++ b/starboard/shared/starboard/audio_sink/audio_sink_internal.h @@ -56,12 +56,19 @@ struct SbAudioSinkPrivate { }; virtual ~SbAudioSinkPrivate() {} - virtual void SetPlaybackRate(double playback_rate) = 0; + virtual void SetPlaybackRate(double playback_rate) = 0; virtual void SetVolume(double volume) = 0; - virtual bool IsType(Type* type) = 0; +}; + +namespace starboard { +namespace shared { +namespace starboard { +namespace audio_sink { +class SbAudioSinkImpl : public SbAudioSinkPrivate { + public: // The following two functions will be called during application startup and // termination. static void Initialize(); @@ -112,4 +119,9 @@ struct SbAudioSinkPrivate { static void PlatformTearDown(); }; +} // namespace audio_sink +} // namespace starboard +} // namespace shared +} // namespace starboard + #endif // STARBOARD_SHARED_STARBOARD_AUDIO_SINK_AUDIO_SINK_INTERNAL_H_ diff --git a/starboard/shared/starboard/audio_sink/audio_sink_is_valid.cc b/starboard/shared/starboard/audio_sink/audio_sink_is_valid.cc index 05b2ceab052..7e608a46026 100644 --- a/starboard/shared/starboard/audio_sink/audio_sink_is_valid.cc +++ b/starboard/shared/starboard/audio_sink/audio_sink_is_valid.cc @@ -17,11 +17,13 @@ #include "starboard/shared/starboard/audio_sink/audio_sink_internal.h" bool SbAudioSinkIsValid(SbAudioSink audio_sink) { - SbAudioSinkPrivate::Type* type = SbAudioSinkPrivate::GetPrimaryType(); + using ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl; + + SbAudioSinkPrivate::Type* type = SbAudioSinkImpl::GetPrimaryType(); if (type && type->IsValid(audio_sink)) { return true; } - type = SbAudioSinkPrivate::GetFallbackType(); + type = SbAudioSinkImpl::GetFallbackType(); if (type && type->IsValid(audio_sink)) { return true; } diff --git a/starboard/shared/starboard/player/filter/audio_renderer_sink_impl.cc b/starboard/shared/starboard/player/filter/audio_renderer_sink_impl.cc index 1f7ad987833..d0569f3c78d 100644 --- a/starboard/shared/starboard/player/filter/audio_renderer_sink_impl.cc +++ b/starboard/shared/starboard/player/filter/audio_renderer_sink_impl.cc @@ -39,7 +39,7 @@ AudioRendererSinkImpl::AudioRendererSinkImpl() SbAudioSinkPrivate::ConsumeFramesFunc consume_frames_func, SbAudioSinkPrivate::ErrorFunc error_func, void* context) { - return SbAudioSinkPrivate::Create( + return audio_sink::SbAudioSinkImpl::Create( channels, sampling_frequency_hz, audio_sample_type, audio_frame_storage_type, frame_buffers, frame_buffers_size_in_frames, update_source_status_func, diff --git a/starboard/shared/x11/application_x11.cc b/starboard/shared/x11/application_x11.cc index c6a838efa5d..bac7cd07450 100644 --- a/starboard/shared/x11/application_x11.cc +++ b/starboard/shared/x11/application_x11.cc @@ -701,12 +701,12 @@ ApplicationX11::ApplicationX11(SbEventHandleCallback sb_event_handle_callback) composite_event_id_(kSbEventIdInvalid), display_(NULL), paste_buffer_key_release_pending_(false) { - SbAudioSinkPrivate::Initialize(); + ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl::Initialize(); NetworkNotifier::GetOrCreateInstance(); } ApplicationX11::~ApplicationX11() { - SbAudioSinkPrivate::TearDown(); + ::starboard::shared::starboard::audio_sink::SbAudioSinkImpl::TearDown(); } SbWindow ApplicationX11::CreateWindow(const SbWindowOptions* options) { From 173ecc39274e037981eda5416fb216a4a7d901f4 Mon Sep 17 00:00:00 2001 From: Bo-Rong Chen Date: Sun, 5 Jan 2025 20:23:17 -0800 Subject: [PATCH 08/12] [android] Enable low end device mode (#4625) 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 (https://github.com/youtube/cobalt/pull/4636, https://github.com/youtube/cobalt/pull/4637) were cherry picked. b/380310632 --- .../app/src/main/java/dev/cobalt/coat/CobaltActivity.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cobalt/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java b/cobalt/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java index 7b36fbbf092..5ee31281fbf 100644 --- a/cobalt/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java +++ b/cobalt/android/apk/app/src/main/java/dev/cobalt/coat/CobaltActivity.java @@ -104,11 +104,8 @@ protected void createContent(final Bundle savedInstanceState) { if (!CommandLine.isInitialized()) { ((CobaltApplication) getApplication()).initCommandLine(); - // Note that appendSwitchesAndArguments excludes cobaltCommandLineParams[0] - // as the program name, and all other arguments SHOULD start with '--'. String[] cobaltCommandLineParams = new String[] { - "", // Disable first run experience. "--disable-fre", // Disable user prompts in the first run. @@ -125,6 +122,8 @@ protected void createContent(final Bundle savedInstanceState) { "--enable-features=LogJsConsoleMessages", // Disable rescaling Webpage. "--force-device-scale-factor=1", + // Enable low end device mode. + "--enable-low-end-device-mode", }; CommandLine.getInstance().appendSwitchesAndArguments(cobaltCommandLineParams); From 3b068ece6d8ba4ef68058d5f9679d6465766ab9c Mon Sep 17 00:00:00 2001 From: Dana Dahlstrom Date: Sat, 4 Jan 2025 00:00:00 -0800 Subject: [PATCH 09/12] Simplify cleanup script and use correct path Issue: 365150653 Reviewed-on: https://github.com/youtube/cobalt/pull/4641 --- cobalt/devinfra/kokoro/bin/cleanup.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cobalt/devinfra/kokoro/bin/cleanup.sh b/cobalt/devinfra/kokoro/bin/cleanup.sh index 725d0f29ef9..a3a880a924b 100755 --- a/cobalt/devinfra/kokoro/bin/cleanup.sh +++ b/cobalt/devinfra/kokoro/bin/cleanup.sh @@ -1,12 +1,9 @@ set -x -REPO_ROOT="$(dirname "$0")/../../.." # After kokoro build finishes, changed files in c:/tmpfs get rsync'ed to a borg executor, # from where the artifacts and reports are processed. # Especially on Windows, the rsync can take long time, so we cleanup the cobalt workspace # after finishing each build. set +e -pushd $REPO_ROOT -time git clean -fdx -popd +time git -C "${WORKSPACE_COBALT}" clean -dfx exit 0 From 5a10f254343edf94cb48a720510877b75add3dd8 Mon Sep 17 00:00:00 2001 From: Dana Dahlstrom Date: Sat, 4 Jan 2025 00:00:00 -0800 Subject: [PATCH 10/12] Add safe.directory path in Kokoro cleanup script Issue: 365150653 Reviewed-on: https://github.com/youtube/cobalt/pull/4643 --- cobalt/devinfra/kokoro/bin/cleanup.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/cobalt/devinfra/kokoro/bin/cleanup.sh b/cobalt/devinfra/kokoro/bin/cleanup.sh index a3a880a924b..7df9b12f3c0 100755 --- a/cobalt/devinfra/kokoro/bin/cleanup.sh +++ b/cobalt/devinfra/kokoro/bin/cleanup.sh @@ -5,5 +5,6 @@ set -x # Especially on Windows, the rsync can take long time, so we cleanup the cobalt workspace # after finishing each build. set +e +git config --global --add safe.directory "${WORKSPACE_COBALT}" time git -C "${WORKSPACE_COBALT}" clean -dfx exit 0 From 387b23b0a4362a43cad5e4797d037cc59862f449 Mon Sep 17 00:00:00 2001 From: Holden Warriner Date: Mon, 6 Jan 2025 16:01:19 -0500 Subject: [PATCH 11/12] Add nplb sources to fix include check on android (#4638) 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 --- starboard/nplb/BUILD.gn | 4 ++-- starboard/nplb/maximum_player_configuration_explorer.h | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/starboard/nplb/BUILD.gn b/starboard/nplb/BUILD.gn index a6c3b9e0a2e..6910b4e0255 100644 --- a/starboard/nplb/BUILD.gn +++ b/starboard/nplb/BUILD.gn @@ -47,6 +47,8 @@ test("nplb") { "//starboard/nplb/sabi/struct_alignment_test.cc", "//starboard/shared/starboard/drm/drm_test_helpers.cc", "//starboard/shared/starboard/drm/drm_test_helpers.h", + "//starboard/testing/fake_graphics_context_provider.cc", + "//starboard/testing/fake_graphics_context_provider.h", "align_test.cc", "audio_sink_create_test.cc", "audio_sink_destroy_test.cc", @@ -219,8 +221,6 @@ test("nplb") { if (!is_android) { sources += [ - "//starboard/testing/fake_graphics_context_provider.cc", - "//starboard/testing/fake_graphics_context_provider.h", "maximum_player_configuration_explorer_test.cc", "media_set_audio_write_duration_test.cc", "multiple_player_test.cc", diff --git a/starboard/nplb/maximum_player_configuration_explorer.h b/starboard/nplb/maximum_player_configuration_explorer.h index 80ddf7b34bc..bf74aea0309 100644 --- a/starboard/nplb/maximum_player_configuration_explorer.h +++ b/starboard/nplb/maximum_player_configuration_explorer.h @@ -23,8 +23,7 @@ #include "starboard/nplb/player_test_util.h" #include "starboard/player.h" #include "starboard/shared/starboard/media/media_util.h" -// TODO(cobalt, b/377295011): remove the nogncheck annotation. -#include "starboard/testing/fake_graphics_context_provider.h" // nogncheck +#include "starboard/testing/fake_graphics_context_provider.h" namespace starboard { namespace nplb { From 7e87947f2c50a82e3cd8facd135bed84f6361da1 Mon Sep 17 00:00:00 2001 From: Dana Dahlstrom Date: Sat, 4 Jan 2025 00:00:00 -0800 Subject: [PATCH 12/12] Stop calling configure_environment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … in main Kokoro build scripts. Issue: 365150653 Reviewed-on: https://github.com/youtube/cobalt/pull/4644 --- cobalt/devinfra/kokoro/bin/dind_build.sh | 4 ---- cobalt/devinfra/kokoro/bin/dind_builder_runner.sh | 4 ---- .../devinfra/kokoro/bin/dind_py/main_build_image_and_run.py | 2 -- cobalt/devinfra/kokoro/bin/dind_py/main_pull_image_and_run.py | 2 -- cobalt/devinfra/kokoro/bin/dind_runner.sh | 4 ---- 5 files changed, 16 deletions(-) diff --git a/cobalt/devinfra/kokoro/bin/dind_build.sh b/cobalt/devinfra/kokoro/bin/dind_build.sh index cc41bed4a4d..bbc8a7796aa 100755 --- a/cobalt/devinfra/kokoro/bin/dind_build.sh +++ b/cobalt/devinfra/kokoro/bin/dind_build.sh @@ -8,13 +8,11 @@ # Kokoro Instance # └── Generic DinD Image # ├── dind_builder_runner.sh -# │ ├── configure_environment (common.sh) # │ ├── main_build_image_and_run.py # │ │ └── Specific Cobalt Image # │ │ └── dind_build.sh <= THIS SCRIPT # │ └── run_package_release_pipeline (common.sh) # └── dind_runner.sh -# ├── configure_environment (common.sh) # ├── main_pull_image_and_run.py # │ └── Specific Cobalt Image # │ └── dind_build.sh <= THIS SCRIPT @@ -44,8 +42,6 @@ fi WORKSPACE_COBALT="${KOKORO_ARTIFACTS_DIR}/git/src" -configure_environment - pipeline () { local out_dir="${WORKSPACE_COBALT}/out/${TARGET_PLATFORM}_${CONFIG}" local gclient_root="${KOKORO_ARTIFACTS_DIR}/chromium" diff --git a/cobalt/devinfra/kokoro/bin/dind_builder_runner.sh b/cobalt/devinfra/kokoro/bin/dind_builder_runner.sh index 302a395106d..efe6f5998b9 100755 --- a/cobalt/devinfra/kokoro/bin/dind_builder_runner.sh +++ b/cobalt/devinfra/kokoro/bin/dind_builder_runner.sh @@ -9,13 +9,11 @@ # Kokoro Instance # └── Generic DinD Image # ├── dind_builder_runner.sh <== THIS SCRIPT -# │ ├── configure_environment (common.sh) # │ ├── main_build_image_and_run.py # │ │ └── Specific Cobalt Image # │ │ └── dind_build.sh # │ └── run_package_release_pipeline (common.sh) # └── dind_runner.sh -# ├── configure_environment (common.sh) # ├── main_pull_image_and_run.py # │ └── Specific Cobalt Image # │ └── dind_build.sh @@ -34,8 +32,6 @@ trap "bash ${WORKSPACE_COBALT}/cobalt/devinfra/kokoro/bin/cleanup.sh" EXIT INT T configure_dind_environment -configure_environment - set -x # The python script is responsible for running containerized Cobalt builds. python3 "${WORKSPACE_COBALT}/cobalt/devinfra/kokoro/bin/dind_py/main_build_image_and_run.py" diff --git a/cobalt/devinfra/kokoro/bin/dind_py/main_build_image_and_run.py b/cobalt/devinfra/kokoro/bin/dind_py/main_build_image_and_run.py index a3f3cd07be9..196d161b165 100644 --- a/cobalt/devinfra/kokoro/bin/dind_py/main_build_image_and_run.py +++ b/cobalt/devinfra/kokoro/bin/dind_py/main_build_image_and_run.py @@ -24,13 +24,11 @@ Kokoro Instance └── Generic DinD Image ├── dind_builder_runner.sh - │ ├── configure_environment (common.sh) │ ├── main_build_image_and_run.py <= THIS SCRIPT │ │ └── Specific Cobalt Image │ │ └── dind_build.sh │ └── run_package_release_pipeline (common.sh) └── dind_runner.sh - ├── configure_environment (common.sh) ├── main_pull_image_and_run.py │ └── Specific Cobalt Image │ └── dind_build.sh diff --git a/cobalt/devinfra/kokoro/bin/dind_py/main_pull_image_and_run.py b/cobalt/devinfra/kokoro/bin/dind_py/main_pull_image_and_run.py index a7e14582f22..852c11137c7 100644 --- a/cobalt/devinfra/kokoro/bin/dind_py/main_pull_image_and_run.py +++ b/cobalt/devinfra/kokoro/bin/dind_py/main_pull_image_and_run.py @@ -21,13 +21,11 @@ Kokoro Instance └── Generic DinD Image ├── dind_builder_runner.sh - │ ├── configure_environment (common.sh) │ ├── main_build_image_and_run.py │ │ └── Specific Cobalt Image │ │ └── dind_build.sh │ └── run_package_release_pipeline (common.sh) └── dind_runner.sh - ├── configure_environment (common.sh) ├── main_pull_image_and_run.py <= THIS SCRIPT │ └── Specific Cobalt Image │ └── dind_build.sh diff --git a/cobalt/devinfra/kokoro/bin/dind_runner.sh b/cobalt/devinfra/kokoro/bin/dind_runner.sh index 157aa6141ca..012fc58fa94 100755 --- a/cobalt/devinfra/kokoro/bin/dind_runner.sh +++ b/cobalt/devinfra/kokoro/bin/dind_runner.sh @@ -9,13 +9,11 @@ # Kokoro Instance # └── Generic DinD Image # ├── dind_builder_runner.sh -# │ ├── configure_environment (common.sh) # │ ├── main_build_image_and_run.py # │ │ └── Specific Cobalt Image # │ │ └── dind_build.sh # │ └── run_package_release_pipeline (common.sh) # └── dind_runner.sh <== THIS SCRIPT -# ├── configure_environment (common.sh) # ├── main_pull_image_and_run.py # │ └── Specific Cobalt Image # │ └── dind_build.sh @@ -34,8 +32,6 @@ trap "bash ${WORKSPACE_COBALT}/cobalt/devinfra/kokoro/bin/cleanup.sh" EXIT INT T configure_dind_environment -configure_environment - set -x # The python script is responsible for running containerized Cobalt builds. python3 "${WORKSPACE_COBALT}/cobalt/devinfra/kokoro/bin/dind_py/main_pull_image_and_run.py"