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

Deprecate SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES) #2915

Merged
merged 1 commit into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions cobalt/audio/audio_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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)
Expand Down
7 changes: 0 additions & 7 deletions cobalt/site/docs/reference/starboard/configuration-public.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,6 @@ Book: /youtube/cobalt/_book.yaml

# Starboard Configuration Reference Guide

## Media Configuration

| Properties |
| :--- |
| **`SB_HAS_QUIRK_SUPPORT_INT16_AUDIO_SAMPLES`**<br><br>The implementation is allowed to support kSbMediaAudioSampleTypeInt16 only when this macro is defined.<br><br>By default, this property is undefined. |


## Memory Configuration

| Properties |
Expand Down
9 changes: 9 additions & 0 deletions starboard/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions starboard/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions starboard/media.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 17 additions & 0 deletions starboard/nplb/audio_sink_is_audio_sample_type_supported_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -27,5 +42,7 @@ TEST(SbAudioSinkIsAudioSampleTypeSupportedTest, SunnyDay) {
EXPECT_TRUE(int16_supported || float32_supported);
}

#endif // SB_API_VERSION >= 16

} // namespace nplb
} // namespace starboard
12 changes: 6 additions & 6 deletions starboard/shared/opus/opus_audio_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<const unsigned char*>(input_buffer->data()),
input_buffer->size(),
reinterpret_cast<opus_int16*>(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<const unsigned char*>(input_buffer->data()),
input_buffer->size(), reinterpret_cast<float*>(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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.";
Expand All @@ -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);

{
Expand Down Expand Up @@ -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()) {
Expand Down
7 changes: 1 addition & 6 deletions starboard/shared/win32/audio_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
8 changes: 2 additions & 6 deletions starboard/shared/win32/audio_sink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions starboard/stub/configuration_public.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 --------------------------------------------------

Expand Down
6 changes: 0 additions & 6 deletions starboard/xb1/shared/configuration_public.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_
Loading