From 26701c8f44f4b3895763b31f6cf2616b95348976 Mon Sep 17 00:00:00 2001 From: Xiaoming Shi Date: Thu, 11 Apr 2024 16:01:36 -0700 Subject: [PATCH] Deprecate SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) was introduced to allow platforms to use `kSbMediaAudioSampleTypeInt16` after it's deprecated. Now SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) was removed from Starboard 16, and platforms can no longer use `kSbMediaAudioSampleTypeInt16`. All platforms have to support float sample type in their AudioSink implementations, which will be verified by nplb. `kSbMediaAudioSampleTypeInt16Deprecated` is kept but will no longer be used by Cobalt, so the AudioSink implementations can continue to support int16 sample type for internal use. We may remove `SbMediaAudioSampleType` and `SbAudioSinkIsAudioSampleTypeSupported()` completely in a future release, and assume that the only sample type supported by AudioSink across the Starboard interface is int 16. This only affects the usage of int 16 samples in Cobalt, where the Starboard interface is the only channel to communicate to platform implementations. Platforms using AudioSink for video playback that also need to support int 16 sample type internally can continue to do so by defining their own types and enum values for int 16 sample type below Starboard. b/332054094 Change-Id: I8924ce5717e9d815aa46050c6e85488a8cdedc13 --- cobalt/audio/audio_helpers.h | 9 +++++---- .../reference/starboard/configuration-public.md | 7 ------- starboard/CHANGELOG.md | 9 +++++++++ starboard/configuration.h | 6 ++++++ starboard/media.h | 4 ++-- ..._sink_is_audio_sample_type_supported_test.cc | 17 +++++++++++++++++ starboard/shared/opus/opus_audio_decoder.cc | 12 ++++++------ .../testing/audio_renderer_internal_test.cc | 4 +--- starboard/shared/win32/audio_decoder.cc | 7 +------ starboard/shared/win32/audio_sink.cc | 8 ++------ ...audio_sink_is_audio_sample_type_supported.cc | 4 +--- starboard/stub/configuration_public.h | 2 ++ starboard/xb1/shared/configuration_public.h | 6 ------ 13 files changed, 52 insertions(+), 43 deletions(-) diff --git a/cobalt/audio/audio_helpers.h b/cobalt/audio/audio_helpers.h index 7ed581e26eff..8580b75ebcdf 100644 --- a/cobalt/audio/audio_helpers.h +++ b/cobalt/audio/audio_helpers.h @@ -68,7 +68,10 @@ inline size_t GetSampleTypeSize(SampleType sample_type) { // an internal SampleType. If we are not running on starboard or using the // starboard media pipeline, then the preferred sample type is always float32. inline SampleType GetPreferredOutputSampleType() { -#if defined(STARBOARD) +#if SB_API_VERSION >= 16 + DCHECK(SbAudioSinkIsAudioSampleTypeSupported(kSbMediaAudioSampleTypeFloat32)); + return kSampleTypeFloat32; +#else // SB_API_VERSION >= 16 if (SbAudioSinkIsAudioSampleTypeSupported(kSbMediaAudioSampleTypeFloat32)) { return kSampleTypeFloat32; } @@ -77,9 +80,7 @@ inline SampleType GetPreferredOutputSampleType() { << "At least one starboard audio sample type must be supported if using " "starboard media pipeline."; return kSampleTypeInt16; -#else // defined(STARBOARD) - return kSampleTypeFloat32; -#endif // defined(STARBOARD) +#endif // SB_API_VERSION >= 16 } #if defined(STARBOARD) diff --git a/cobalt/site/docs/reference/starboard/configuration-public.md b/cobalt/site/docs/reference/starboard/configuration-public.md index 9c57bb9da38c..64605bb01580 100644 --- a/cobalt/site/docs/reference/starboard/configuration-public.md +++ b/cobalt/site/docs/reference/starboard/configuration-public.md @@ -3,13 +3,6 @@ Book: /youtube/cobalt/_book.yaml # Starboard Configuration Reference Guide -## Media Configuration - -| Properties | -| :--- | -| **`SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES`**

The implementation is allowed to support kSbMediaAudioSampleTypeInt16 only when this macro is defined.

By default, this property is undefined. | - - ## Memory Configuration | Properties | diff --git a/starboard/CHANGELOG.md b/starboard/CHANGELOG.md index a4e494c80b6d..9d5675413e89 100644 --- a/starboard/CHANGELOG.md +++ b/starboard/CHANGELOG.md @@ -175,6 +175,15 @@ deprecated. ### Deprecated SbStringScan and SbStringScanF The APIs defined in `starboard/string.h` are deprecated and the standard API `vsscanf` and `sscanf` are used instead. +### Deprecated SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES + +`SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES` can no longer be used to enable the +use of `kSbMediaAudioSampleTypeInt16`. The platform has to support AudioSink in +float sample and is verified by nplb test. The enum value of +`kSbMediaAudioSampleTypeInt16Deprecated` was kept so the platform may still +choose to implement int16 sample support. It will be removed in a future +version. + ## Version 15 ### SbMemoryReporter is no longer used diff --git a/starboard/configuration.h b/starboard/configuration.h index 39cf7b23a715..736ae904c0e7 100644 --- a/starboard/configuration.h +++ b/starboard/configuration.h @@ -357,6 +357,12 @@ struct CompileAssert {}; #endif #endif +#if SB_API_VERSION >= 16 +#if defined(SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES) +#error "SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES is deprecated in SB16 or later" +#endif // defined(SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES) +#endif // SB_API_VERSION >= 16 + // SB_EXPORT_PLATFORM annotates symbols as exported from shared libraries. #if SB_API_VERSION < 16 #if !defined(SB_EXPORT_PLATFORM) diff --git a/starboard/media.h b/starboard/media.h index c9b736936937..fca1792933f6 100644 --- a/starboard/media.h +++ b/starboard/media.h @@ -135,9 +135,9 @@ typedef enum SbMediaAudioCodingType { typedef enum SbMediaAudioSampleType { kSbMediaAudioSampleTypeInt16Deprecated, kSbMediaAudioSampleTypeFloat32, -#if SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) +#if SB_API_VERSION <= 15 && SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) kSbMediaAudioSampleTypeInt16 = kSbMediaAudioSampleTypeInt16Deprecated, -#endif // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) +#endif // SB_API_VERSION <= 15 && SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) } SbMediaAudioSampleType; // Possible audio frame storage types. diff --git a/starboard/nplb/audio_sink_is_audio_sample_type_supported_test.cc b/starboard/nplb/audio_sink_is_audio_sample_type_supported_test.cc index 5c8b6758515a..73cbab0bdd96 100644 --- a/starboard/nplb/audio_sink_is_audio_sample_type_supported_test.cc +++ b/starboard/nplb/audio_sink_is_audio_sample_type_supported_test.cc @@ -18,6 +18,21 @@ namespace starboard { namespace nplb { +#if SB_API_VERSION >= 16 + +TEST(SbAudioSinkIsAudioSampleTypeSupportedTest, SunnyDay) { + bool float32_supported = + SbAudioSinkIsAudioSampleTypeSupported(kSbMediaAudioSampleTypeFloat32); + // All platforms must support the float32 sample type. + EXPECT_TRUE(float32_supported); + + // It's ok for a platform to not support int16 sample type, but the call with + // `kSbMediaAudioSampleTypeInt16Deprecated` as a parameter shouldn't crash. + SbAudioSinkIsAudioSampleTypeSupported(kSbMediaAudioSampleTypeInt16Deprecated); +} + +#else // SB_API_VERSION >= 16 + TEST(SbAudioSinkIsAudioSampleTypeSupportedTest, SunnyDay) { bool int16_supported = SbAudioSinkIsAudioSampleTypeSupported( kSbMediaAudioSampleTypeInt16Deprecated); @@ -27,5 +42,7 @@ TEST(SbAudioSinkIsAudioSampleTypeSupportedTest, SunnyDay) { EXPECT_TRUE(int16_supported || float32_supported); } +#endif // SB_API_VERSION >= 16 + } // namespace nplb } // namespace starboard diff --git a/starboard/shared/opus/opus_audio_decoder.cc b/starboard/shared/opus/opus_audio_decoder.cc index fa8e00a69e2f..683e7fd1b29d 100644 --- a/starboard/shared/opus/opus_audio_decoder.cc +++ b/starboard/shared/opus/opus_audio_decoder.cc @@ -133,19 +133,19 @@ bool OpusAudioDecoder::DecodeInternal( audio_stream_info_.number_of_channels * frames_per_au_ * starboard::media::GetBytesPerSample(GetSampleType())); -#if SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) +#if SB_API_VERSION <= 15 && SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) const char kDecodeFunctionName[] = "opus_multistream_decode"; int decoded_frames = opus_multistream_decode( decoder_, static_cast(input_buffer->data()), input_buffer->size(), reinterpret_cast(decoded_audio->data()), frames_per_au_, 0); -#else // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) +#else // SB_API_VERSION <= 15 && SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) const char kDecodeFunctionName[] = "opus_multistream_decode_float"; int decoded_frames = opus_multistream_decode_float( decoder_, static_cast(input_buffer->data()), input_buffer->size(), reinterpret_cast(decoded_audio->data()), frames_per_au_, 0); -#endif // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) +#endif // SB_API_VERSION <= 15 && SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) if (decoded_frames == OPUS_BUFFER_TOO_SMALL && frames_per_au_ < kMaxOpusFramesPerAU) { frames_per_au_ = kMaxOpusFramesPerAU; @@ -274,11 +274,11 @@ bool OpusAudioDecoder::is_valid() const { SbMediaAudioSampleType OpusAudioDecoder::GetSampleType() const { SB_DCHECK(BelongsToCurrentThread()); -#if SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) +#if SB_API_VERSION <= 15 && SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) return kSbMediaAudioSampleTypeInt16; -#else // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) +#else // SB_API_VERSION <= 15 && SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) return kSbMediaAudioSampleTypeFloat32; -#endif // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) +#endif // SB_API_VERSION <= 15 && SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) } } // namespace opus diff --git a/starboard/shared/starboard/player/filter/testing/audio_renderer_internal_test.cc b/starboard/shared/starboard/player/filter/testing/audio_renderer_internal_test.cc index 7c6fbca5cd7c..58a9555232cd 100644 --- a/starboard/shared/starboard/player/filter/testing/audio_renderer_internal_test.cc +++ b/starboard/shared/starboard/player/filter/testing/audio_renderer_internal_test.cc @@ -382,7 +382,6 @@ TEST_F(AudioRendererTest, SunnyDay) { EXPECT_TRUE(audio_renderer_->IsEndOfStreamPlayed()); } -#if SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) TEST_F(AudioRendererTest, SunnyDayWithDoublePlaybackRateAndInt16Samples) { if (HasAsyncAudioFramesReporting()) { SB_LOG(INFO) << "Platform has async audio frames reporting. Test skipped."; @@ -393,7 +392,7 @@ TEST_F(AudioRendererTest, SunnyDayWithDoublePlaybackRateAndInt16Samples) { // Resets |audio_renderer_sink_|, so all the gtest codes need to be below // this line. - ResetToFormat(kSbMediaAudioSampleTypeInt16, + ResetToFormat(kSbMediaAudioSampleTypeInt16Deprecated, kSbMediaAudioFrameStorageTypeInterleaved); { @@ -465,7 +464,6 @@ TEST_F(AudioRendererTest, SunnyDayWithDoublePlaybackRateAndInt16Samples) { EXPECT_TRUE(audio_renderer_->IsEndOfStreamPlayed()); } -#endif // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) TEST_F(AudioRendererTest, StartPlayBeforePreroll) { if (HasAsyncAudioFramesReporting()) { diff --git a/starboard/shared/win32/audio_decoder.cc b/starboard/shared/win32/audio_decoder.cc index 27d84e1e0c46..94453c561933 100644 --- a/starboard/shared/win32/audio_decoder.cc +++ b/starboard/shared/win32/audio_decoder.cc @@ -61,12 +61,7 @@ AudioDecoder::AudioDecoder(const AudioStreamInfo& audio_stream_info, drm_system_(drm_system), sample_type_((audio_stream_info.codec == kSbMediaAudioCodecAc3 || audio_stream_info.codec == kSbMediaAudioCodecEac3) - ? -#if SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) - kSbMediaAudioSampleTypeInt16 -#else - kSbMediaAudioSampleTypeInt16Deprecated -#endif // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) + ? kSbMediaAudioSampleTypeInt16Deprecated : kSbMediaAudioSampleTypeFloat32), stream_ended_(false) { SB_DCHECK(audio_stream_info.codec == kSbMediaAudioCodecAac || diff --git a/starboard/shared/win32/audio_sink.cc b/starboard/shared/win32/audio_sink.cc index db8e671f758b..e3b7909ef4f3 100644 --- a/starboard/shared/win32/audio_sink.cc +++ b/starboard/shared/win32/audio_sink.cc @@ -47,10 +47,8 @@ void CHECK_HRESULT_OK(HRESULT hr) { WORD SampleTypeToFormatTag(SbMediaAudioSampleType type) { switch (type) { -#if SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) - case kSbMediaAudioSampleTypeInt16: + case kSbMediaAudioSampleTypeInt16Deprecated: return WAVE_FORMAT_PCM; -#endif // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) case kSbMediaAudioSampleTypeFloat32: return WAVE_FORMAT_IEEE_FLOAT; default: @@ -61,10 +59,8 @@ WORD SampleTypeToFormatTag(SbMediaAudioSampleType type) { WORD SampleTypeToBitsPerSample(SbMediaAudioSampleType type) { switch (type) { -#if SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) - case kSbMediaAudioSampleTypeInt16: + case kSbMediaAudioSampleTypeInt16Deprecated: return 16; -#endif // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) case kSbMediaAudioSampleTypeFloat32: return 32; default: diff --git a/starboard/shared/win32/audio_sink_is_audio_sample_type_supported.cc b/starboard/shared/win32/audio_sink_is_audio_sample_type_supported.cc index ccba8400209c..7ea8f1ed5149 100644 --- a/starboard/shared/win32/audio_sink_is_audio_sample_type_supported.cc +++ b/starboard/shared/win32/audio_sink_is_audio_sample_type_supported.cc @@ -19,10 +19,8 @@ bool SbAudioSinkIsAudioSampleTypeSupported( SbMediaAudioSampleType audio_sample_type) { switch (audio_sample_type) { -#if SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) - case kSbMediaAudioSampleTypeInt16: + case kSbMediaAudioSampleTypeInt16Deprecated: return true; -#endif // SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) case kSbMediaAudioSampleTypeFloat32: return true; default: diff --git a/starboard/stub/configuration_public.h b/starboard/stub/configuration_public.h index 62c2915073df..4f1bc66bfa66 100644 --- a/starboard/stub/configuration_public.h +++ b/starboard/stub/configuration_public.h @@ -50,9 +50,11 @@ // --- Media Configuration --------------------------------------------------- +#if SB_API_VERSION <= 15 // The implementation is allowed to support kSbMediaAudioSampleTypeInt16 only // when this macro is defined. #undef SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES +#endif // SB_API_VERSION <= 15 // --- Memory Configuration -------------------------------------------------- diff --git a/starboard/xb1/shared/configuration_public.h b/starboard/xb1/shared/configuration_public.h index 38ecb4c6e5b6..d951637d8110 100644 --- a/starboard/xb1/shared/configuration_public.h +++ b/starboard/xb1/shared/configuration_public.h @@ -60,10 +60,4 @@ // Specifies whether this platform supports IPV6. #define SB_HAS_IPV6 1 -// --- Platform Specific Quirks ---------------------------------------------- - -// The implementation is allowed to support kSbMediaAudioSampleTypeInt16 only -// when this macro is defined. -#define SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES 1 - #endif // STARBOARD_XB1_SHARED_CONFIGURATION_PUBLIC_H_