Skip to content

Commit

Permalink
Deprecate SB_HAS_QUIRK(SUPPORT_INT16_AUDIO_SAMPLES)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
xiaomings committed Apr 15, 2024
1 parent eccdac4 commit b01981a
Show file tree
Hide file tree
Showing 13 changed files with 52 additions and 43 deletions.
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 > 15
DCHECK(SbAudioSinkIsAudioSampleTypeSupported(kSbMediaAudioSampleTypeFloat32));
return kSampleTypeFloat32;
#else // SB_API_VERSION > 15
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 > 15
}

#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 @@ -167,6 +167,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 > 15
#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 > 15

// 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 > 15

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 > 15

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 > 15

} // 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_

0 comments on commit b01981a

Please sign in to comment.