From 0bb350515768a179bc18af99960b48ebbfd10238 Mon Sep 17 00:00:00 2001
From: Richard Nichols <[email protected]>
Date: Tue, 21 Jul 2026 06:48:16 -0700
Subject: [PATCH] [chromecast] Own audio extra_data in DemuxerStreamReader

StarboardAudioSampleInfo::audio_specific_config is a non-owning pointer
into an AudioDecoderConfig's extra_data. The DemuxerStreamReader already
has a chromium_audio_config_ member intended to back this pointer, but
it was only populated on kConfigChanged, not for the initial config
supplied at construction time. As a result, audio_specific_config
referenced the extra_data of a stack-local AudioDecoderConfig in
StarboardPlayerManager::Create() that went out of scope before the first
sample was written to starboard.

Populate chromium_audio_config_ in the constructor and re-point
audio_specific_config at the owned copy, matching what
UpdateAudioConfig() already does for subsequent configs.

Change-Id: I80ba0b17e7ec83d86fa3d5e27b92ff6788d910a2
Bug: 513261751
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8102143
Commit-Queue: Richard Nichols <[email protected]>
Reviewed-by: Simeon Anfinrud <[email protected]>
Reviewed-by: Shawn Quereshi <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1665437}
---

diff --git a/chromecast/starboard/media/renderer/demuxer_stream_reader.cc b/chromecast/starboard/media/renderer/demuxer_stream_reader.cc
index b388fd3..4e77b0c3 100644
--- a/chromecast/starboard/media/renderer/demuxer_stream_reader.cc
+++ b/chromecast/starboard/media/renderer/demuxer_stream_reader.cc
@@ -107,13 +107,23 @@
   }
 
   if (audio_stream_) {
-    ::media::AudioDecoderConfig audio_config =
-        audio_stream_->audio_decoder_config();
+    chromium_audio_config_ = audio_stream_->audio_decoder_config();
+    if (audio_sample_info_) {
+      // audio_sample_info_'s audio_specific_config may currently point to the
+      // extra data of an AudioDecoderConfig owned by the caller. Re-point it
+      // at our own copy so that it remains valid for the lifetime of this
+      // object.
+      audio_sample_info_->audio_specific_config_size =
+          chromium_audio_config_.extra_data().size();
+      audio_sample_info_->audio_specific_config =
+          chromium_audio_config_.extra_data().data();
+    }
 
-    if (IsResamplingNecessary(audio_config)) {
+    if (IsResamplingNecessary(chromium_audio_config_)) {
       convert_audio_fn_ = base::BindRepeating(
-          &ConvertPcmAudioBufferToS16, audio_config.codec(),
-          audio_config.sample_format(), audio_config.channels());
+          &ConvertPcmAudioBufferToS16, chromium_audio_config_.codec(),
+          chromium_audio_config_.sample_format(),
+          chromium_audio_config_.channels());
     } else {
       convert_audio_fn_ = base::BindRepeating(&DoNotConvertBuffer);
     }
diff --git a/chromecast/starboard/media/renderer/starboard_player_manager_test.cc b/chromecast/starboard/media/renderer/starboard_player_manager_test.cc
index cc960f7..f827580 100644
--- a/chromecast/starboard/media/renderer/starboard_player_manager_test.cc
+++ b/chromecast/starboard/media/renderer/starboard_player_manager_test.cc
@@ -721,6 +721,83 @@
 }
 
 TEST_F(StarboardPlayerManagerTest,
+       WritesInitialAudioSpecificConfigToStarboard) {
+  // Verify that when the initial audio config has extra data, the bytes that
+  // audio_specific_config points to are still readable and correct by the time
+  // the first sample is written to starboard.
+  constexpr auto kSeekTime = base::Seconds(10);
+  constexpr auto kAudioData = std::to_array<uint8_t>({9, 8, 7});
+  const std::vector<uint8_t> kExtraData = {0x12, 0x10, 0x56, 0xE5, 0x00};
+
+  audio_stream_.set_audio_decoder_config(::media::AudioDecoderConfig(
+      ::media::AudioCodec::kAAC, ::media::SampleFormat::kSampleFormatS16,
+      ::media::ChannelLayoutConfig::Stereo(), 44100, kExtraData,
+      ::media::EncryptionScheme::kUnencrypted));
+
+  // This will be updated whenever the player manager seeks in starboard.
+  int seek_ticket = -1;
+  ON_CALL(starboard_, SeekTo(&sb_player_, _, _))
+      .WillByDefault(SaveArg<2>(&seek_ticket));
+
+  // This will be set to the callbacks received by the mock Starboard.
+  const StarboardPlayerCallbackHandler* callbacks = nullptr;
+  EXPECT_CALL(starboard_, CreatePlayer(NotNull(), _))
+      .WillOnce(DoAll(SaveArg<1>(&callbacks), Return(&sb_player_)));
+
+  scoped_refptr<::media::DecoderBuffer> audio_buffer =
+      ::media::DecoderBuffer::CopyFrom(kAudioData);
+  EXPECT_CALL(audio_stream_, OnRead)
+      .WillOnce(RunOnceCallback<0>(
+          DemuxerStream::Status::kOk,
+          std::vector<scoped_refptr<::media::DecoderBuffer>>({audio_buffer})));
+
+  // Capture a copy of the bytes that starboard would read from
+  // audio_specific_config.
+  std::vector<uint8_t> captured_audio_specific_config;
+  EXPECT_CALL(
+      starboard_,
+      WriteSample(&sb_player_, StarboardMediaType::kStarboardMediaTypeAudio, _))
+      .WillOnce(WithArg<2>(
+          [&captured_audio_specific_config](
+              base::span<const StarboardSampleInfo> sample_infos) {
+            ASSERT_EQ(sample_infos.size(), 1u);
+            const StarboardAudioSampleInfo& audio_info =
+                sample_infos[0].audio_sample_info;
+            ASSERT_THAT(audio_info.audio_specific_config, NotNull());
+            const uint8_t* config_bytes =
+                static_cast<const uint8_t*>(audio_info.audio_specific_config);
+            // SAFETY: audio_specific_config points to
+            // audio_specific_config_size bytes per the Starboard API contract.
+            UNSAFE_BUFFERS(captured_audio_specific_config.assign(
+                config_bytes,
+                config_bytes + audio_info.audio_specific_config_size));
+          }));
+
+  std::unique_ptr<StarboardPlayerManager> player_manager =
+      StarboardPlayerManager::Create(
+          &starboard_, &audio_stream_, /*video_stream=*/nullptr,
+          &renderer_client_, &metrics_helper_,
+          base::SequencedTaskRunner::GetCurrentDefault(),
+          /*enable_buffering=*/true);
+  ASSERT_THAT(player_manager, NotNull());
+
+  player_manager->StartPlayingFrom(kSeekTime);
+
+  // Simulate Starboard requesting an audio buffer.
+  ASSERT_THAT(callbacks, NotNull());
+  ASSERT_THAT(callbacks->decoder_status_fn, NotNull());
+  ASSERT_THAT(callbacks->context, NotNull());
+  callbacks->decoder_status_fn(
+      &sb_player_, callbacks->context,
+      StarboardMediaType::kStarboardMediaTypeAudio,
+      StarboardDecoderState::kStarboardDecoderStateNeedsData, seek_ticket);
+
+  RunPendingTasks();
+
+  EXPECT_EQ(captured_audio_specific_config, kExtraData);
+}
+
+TEST_F(StarboardPlayerManagerTest,
        CreatePlayerReturnsNullIfBothDemuxerStreamsAreNull) {
   EXPECT_THAT(StarboardPlayerManager::Create(
                   &starboard_, /*audio_stream=*/nullptr,
diff --git a/chromecast/starboard/media/renderer/starboard_renderer_test.cc b/chromecast/starboard/media/renderer/starboard_renderer_test.cc
index 8f1ed6c..1683c07f 100644
--- a/chromecast/starboard/media/renderer/starboard_renderer_test.cc
+++ b/chromecast/starboard/media/renderer/starboard_renderer_test.cc
@@ -25,6 +25,7 @@
 #include "base/task/sequenced_task_runner.h"
 #include "base/test/bind.h"
 #include "base/test/gmock_callback_support.h"
+#include "base/test/gtest_util.h"
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
 #include "base/unguessable_token.h"

diff --git a/chromecast/starboard/media/renderer/starboard_player_manager_test.cc b/chromecast/starboard/media/renderer/starboard_player_manager_test.cc
index cc960f7..f827580 100644
--- a/chromecast/starboard/media/renderer/starboard_player_manager_test.cc
+++ b/chromecast/starboard/media/renderer/starboard_player_manager_test.cc
@@ -721,6 +721,83 @@
 }
 
 TEST_F(StarboardPlayerManagerTest,
+       WritesInitialAudioSpecificConfigToStarboard) {
+  // Verify that when the initial audio config has extra data, the bytes that
+  // audio_specific_config points to are still readable and correct by the time
+  // the first sample is written to starboard.
+  constexpr auto kSeekTime = base::Seconds(10);
+  constexpr auto kAudioData = std::to_array<uint8_t>({9, 8, 7});
+  const std::vector<uint8_t> kExtraData = {0x12, 0x10, 0x56, 0xE5, 0x00};
+
+  audio_stream_.set_audio_decoder_config(::media::AudioDecoderConfig(
+      ::media::AudioCodec::kAAC, ::media::SampleFormat::kSampleFormatS16,
+      ::media::ChannelLayoutConfig::Stereo(), 44100, kExtraData,
+      ::media::EncryptionScheme::kUnencrypted));
+
+  // This will be updated whenever the player manager seeks in starboard.
+  int seek_ticket = -1;
+  ON_CALL(starboard_, SeekTo(&sb_player_, _, _))
+      .WillByDefault(SaveArg<2>(&seek_ticket));
+
+  // This will be set to the callbacks received by the mock Starboard.
+  const StarboardPlayerCallbackHandler* callbacks = nullptr;
+  EXPECT_CALL(starboard_, CreatePlayer(NotNull(), _))
+      .WillOnce(DoAll(SaveArg<1>(&callbacks), Return(&sb_player_)));
+
+  scoped_refptr<::media::DecoderBuffer> audio_buffer =
+      ::media::DecoderBuffer::CopyFrom(kAudioData);
+  EXPECT_CALL(audio_stream_, OnRead)
+      .WillOnce(RunOnceCallback<0>(
+          DemuxerStream::Status::kOk,
+          std::vector<scoped_refptr<::media::DecoderBuffer>>({audio_buffer})));
+
+  // Capture a copy of the bytes that starboard would read from
+  // audio_specific_config.
+  std::vector<uint8_t> captured_audio_specific_config;
+  EXPECT_CALL(
+      starboard_,
+      WriteSample(&sb_player_, StarboardMediaType::kStarboardMediaTypeAudio, _))
+      .WillOnce(WithArg<2>(
+          [&captured_audio_specific_config](
+              base::span<const StarboardSampleInfo> sample_infos) {
+            ASSERT_EQ(sample_infos.size(), 1u);
+            const StarboardAudioSampleInfo& audio_info =
+                sample_infos[0].audio_sample_info;
+            ASSERT_THAT(audio_info.audio_specific_config, NotNull());
+            const uint8_t* config_bytes =
+                static_cast<const uint8_t*>(audio_info.audio_specific_config);
+            // SAFETY: audio_specific_config points to
+            // audio_specific_config_size bytes per the Starboard API contract.
+            UNSAFE_BUFFERS(captured_audio_specific_config.assign(
+                config_bytes,
+                config_bytes + audio_info.audio_specific_config_size));
+          }));
+
+  std::unique_ptr<StarboardPlayerManager> player_manager =
+      StarboardPlayerManager::Create(
+          &starboard_, &audio_stream_, /*video_stream=*/nullptr,
+          &renderer_client_, &metrics_helper_,
+          base::SequencedTaskRunner::GetCurrentDefault(),
+          /*enable_buffering=*/true);
+  ASSERT_THAT(player_manager, NotNull());
+
+  player_manager->StartPlayingFrom(kSeekTime);
+
+  // Simulate Starboard requesting an audio buffer.
+  ASSERT_THAT(callbacks, NotNull());
+  ASSERT_THAT(callbacks->decoder_status_fn, NotNull());
+  ASSERT_THAT(callbacks->context, NotNull());
+  callbacks->decoder_status_fn(
+      &sb_player_, callbacks->context,
+      StarboardMediaType::kStarboardMediaTypeAudio,
+      StarboardDecoderState::kStarboardDecoderStateNeedsData, seek_ticket);
+
+  RunPendingTasks();
+
+  EXPECT_EQ(captured_audio_specific_config, kExtraData);
+}
+
+TEST_F(StarboardPlayerManagerTest,
        CreatePlayerReturnsNullIfBothDemuxerStreamsAreNull) {
   EXPECT_THAT(StarboardPlayerManager::Create(
                   &starboard_, /*audio_stream=*/nullptr,
diff --git a/chromecast/starboard/media/renderer/starboard_renderer_test.cc b/chromecast/starboard/media/renderer/starboard_renderer_test.cc
index 8f1ed6c..1683c07f 100644
--- a/chromecast/starboard/media/renderer/starboard_renderer_test.cc
+++ b/chromecast/starboard/media/renderer/starboard_renderer_test.cc
@@ -25,6 +25,7 @@
 #include "base/task/sequenced_task_runner.h"
 #include "base/test/bind.h"
 #include "base/test/gmock_callback_support.h"
+#include "base/test/gtest_util.h"
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
 #include "base/unguessable_token.h"