Skip to content

Commit 1931288

Browse files
authored
player: Remove CreateInstance() from some player classes (youtube#9244)
These methods were useful back when thread management was a more manual process and classes like PlayerWorker::CreateInstance() could return nullptr and this had to trickle up the caller hierarchy. This is no longer the case since PRs like youtube#9027 and youtube#8064, and the constructors can be called directly now. Fixed: 477798131
1 parent b8160a9 commit 1931288

File tree

6 files changed

+38
-107
lines changed

6 files changed

+38
-107
lines changed

starboard/android/shared/player_create.cc

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -204,24 +204,14 @@ SbPlayer SbPlayerCreate(SbWindow /*window*/,
204204
handler->SetMaxVideoInputSize(
205205
starboard::GetMaxVideoInputSizeForCurrentThread());
206206
handler->SetVideoSurfaceView(starboard::GetSurfaceViewForCurrentThread());
207-
SbPlayer player = starboard::SbPlayerPrivateImpl::CreateInstance(
207+
auto player = std::make_unique<starboard::SbPlayerPrivateImpl>(
208208
audio_codec, video_codec, sample_deallocate_func, decoder_status_func,
209209
player_status_func, player_error_func, context, std::move(handler));
210-
211-
if (SbPlayerIsValid(player)) {
212-
if (creation_param->output_mode != kSbPlayerOutputModeDecodeToTexture) {
213-
// TODO: accomplish this through more direct means.
214-
// Set the bounds to initialize the VideoSurfaceView. The initial values
215-
// don't matter.
216-
SbPlayerSetBounds(player, 0, 0, 0, 0, 0);
217-
}
218-
return player;
210+
if (creation_param->output_mode != kSbPlayerOutputModeDecodeToTexture) {
211+
// TODO: accomplish this through more direct means.
212+
// Set the bounds to initialize the VideoSurfaceView. The initial values
213+
// don't matter.
214+
SbPlayerSetBounds(player.get(), 0, 0, 0, 0, 0);
219215
}
220-
221-
SB_LOG(ERROR)
222-
<< "Invalid player returned by SbPlayerPrivateImpl::CreateInstance().";
223-
player_error_func(
224-
kSbPlayerInvalid, context, kSbPlayerErrorDecode,
225-
"Invalid player returned by SbPlayerPrivateImpl::CreateInstance()");
226-
return kSbPlayerInvalid;
216+
return player.release();
227217
}

starboard/shared/starboard/player/player_create.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,14 +206,14 @@ SbPlayer SbPlayerCreate(SbWindow /*window*/,
206206
std::make_unique<starboard::FilterBasedPlayerWorkerHandler>(
207207
creation_param, provider);
208208

209-
SbPlayer player = starboard::SbPlayerPrivateImpl::CreateInstance(
209+
auto player = std::make_unique<starboard::SbPlayerPrivateImpl>(
210210
audio_codec, video_codec, sample_deallocate_func, decoder_status_func,
211211
player_status_func, player_error_func, context, std::move(handler));
212212

213213
#if SB_PLAYER_ENABLE_VIDEO_DUMPER
214-
starboard::VideoDmpWriter::OnPlayerCreate(player, audio_codec, video_codec,
215-
drm_system, &audio_stream_info);
214+
starboard::VideoDmpWriter::OnPlayerCreate(
215+
player.get(), audio_codec, video_codec, drm_system, &audio_stream_info);
216216
#endif // SB_PLAYER_ENABLE_VIDEO_DUMPER
217217

218-
return player;
218+
return player.release();
219219
}

starboard/shared/starboard/player/player_internal.cc

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,6 @@
3131
namespace starboard {
3232
namespace {
3333

34-
using std::placeholders::_1;
35-
using std::placeholders::_2;
36-
using std::placeholders::_3;
37-
using std::placeholders::_4;
38-
3934
int64_t CalculateMediaTime(int64_t media_time,
4035
int64_t media_time_update_time,
4136
double playback_rate) {
@@ -58,40 +53,24 @@ SbPlayerPrivateImpl::SbPlayerPrivateImpl(
5853
std::unique_ptr<PlayerWorker::Handler> player_worker_handler)
5954
: sample_deallocate_func_(sample_deallocate_func),
6055
context_(context),
61-
media_time_updated_at_(CurrentMonotonicTime()) {
62-
worker_ = std::unique_ptr<PlayerWorker>(PlayerWorker::CreateInstance(
63-
audio_codec, video_codec, std::move(player_worker_handler),
64-
std::bind(&SbPlayerPrivateImpl::UpdateMediaInfo, this, _1, _2, _3, _4),
65-
decoder_status_func, player_status_func, player_error_func, this,
66-
context));
67-
56+
media_time_updated_at_(CurrentMonotonicTime()),
57+
worker_(std::make_unique<PlayerWorker>(
58+
audio_codec,
59+
video_codec,
60+
std::move(player_worker_handler),
61+
[this](auto&&... args) {
62+
UpdateMediaInfo(std::forward<decltype(args)>(args)...);
63+
},
64+
decoder_status_func,
65+
player_status_func,
66+
player_error_func,
67+
/*player=*/this,
68+
context)) {
6869
++number_of_players_;
6970
SB_LOG(INFO) << "Creating SbPlayerPrivateImpl. There are "
7071
<< number_of_players_ << " players.";
7172
}
7273

73-
// static
74-
SbPlayerPrivate* SbPlayerPrivateImpl::CreateInstance(
75-
SbMediaAudioCodec audio_codec,
76-
SbMediaVideoCodec video_codec,
77-
SbPlayerDeallocateSampleFunc sample_deallocate_func,
78-
SbPlayerDecoderStatusFunc decoder_status_func,
79-
SbPlayerStatusFunc player_status_func,
80-
SbPlayerErrorFunc player_error_func,
81-
void* context,
82-
std::unique_ptr<PlayerWorker::Handler> player_worker_handler) {
83-
SbPlayerPrivateImpl* ret = new SbPlayerPrivateImpl(
84-
audio_codec, video_codec, sample_deallocate_func, decoder_status_func,
85-
player_status_func, player_error_func, context,
86-
std::move(player_worker_handler));
87-
88-
if (ret && ret->worker_) {
89-
return ret;
90-
}
91-
delete ret;
92-
return nullptr;
93-
}
94-
9574
void SbPlayerPrivateImpl::Seek(int64_t seek_to_time, int ticket) {
9675
{
9776
std::lock_guard lock(mutex_);

starboard/shared/starboard/player/player_internal.h

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ namespace starboard {
6262

6363
class SbPlayerPrivateImpl final : public SbPlayerPrivate {
6464
public:
65-
static SbPlayerPrivate* CreateInstance(
65+
SbPlayerPrivateImpl(
6666
SbMediaAudioCodec audio_codec,
6767
SbMediaVideoCodec video_codec,
6868
SbPlayerDeallocateSampleFunc sample_deallocate_func,
@@ -94,16 +94,6 @@ class SbPlayerPrivateImpl final : public SbPlayerPrivate {
9494
~SbPlayerPrivateImpl() final;
9595

9696
private:
97-
SbPlayerPrivateImpl(
98-
SbMediaAudioCodec audio_codec,
99-
SbMediaVideoCodec video_codec,
100-
SbPlayerDeallocateSampleFunc sample_deallocate_func,
101-
SbPlayerDecoderStatusFunc decoder_status_func,
102-
SbPlayerStatusFunc player_status_func,
103-
SbPlayerErrorFunc player_error_func,
104-
void* context,
105-
std::unique_ptr<PlayerWorker::Handler> player_worker_handler);
106-
10797
void UpdateMediaInfo(int64_t media_time,
10898
int dropped_video_frames,
10999
int ticket,
@@ -126,7 +116,7 @@ class SbPlayerPrivateImpl final : public SbPlayerPrivate {
126116
// we may extrapolate the media time in GetInfo().
127117
bool is_progressing_ = false;
128118

129-
std::unique_ptr<PlayerWorker> worker_;
119+
const std::unique_ptr<PlayerWorker> worker_;
130120

131121
std::mutex audio_configurations_mutex_;
132122
std::vector<SbMediaAudioConfiguration> audio_configurations_;

starboard/shared/starboard/player/player_worker.cc

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,22 +42,6 @@ DECLARE_INSTANCE_COUNTER(PlayerWorker)
4242

4343
} // namespace
4444

45-
PlayerWorker* PlayerWorker::CreateInstance(
46-
SbMediaAudioCodec audio_codec,
47-
SbMediaVideoCodec video_codec,
48-
std::unique_ptr<Handler> handler,
49-
UpdateMediaInfoCB update_media_info_cb,
50-
SbPlayerDecoderStatusFunc decoder_status_func,
51-
SbPlayerStatusFunc player_status_func,
52-
SbPlayerErrorFunc player_error_func,
53-
SbPlayer player,
54-
void* context) {
55-
return new PlayerWorker(audio_codec, video_codec, std::move(handler),
56-
update_media_info_cb, decoder_status_func,
57-
player_status_func, player_error_func, player,
58-
context);
59-
}
60-
6145
PlayerWorker::~PlayerWorker() {
6246
ON_INSTANCE_RELEASED(PlayerWorker);
6347

starboard/shared/starboard/player/player_worker.h

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -109,19 +109,20 @@ class PlayerWorker {
109109
Handler& operator=(const Handler&) = delete;
110110
};
111111

112-
static PlayerWorker* CreateInstance(
113-
SbMediaAudioCodec audio_codec,
114-
SbMediaVideoCodec video_codec,
115-
std::unique_ptr<Handler> handler,
116-
UpdateMediaInfoCB update_media_info_cb,
117-
SbPlayerDecoderStatusFunc decoder_status_func,
118-
SbPlayerStatusFunc player_status_func,
119-
SbPlayerErrorFunc player_error_func,
120-
SbPlayer player,
121-
void* context);
122-
112+
PlayerWorker(SbMediaAudioCodec audio_codec,
113+
SbMediaVideoCodec video_codec,
114+
std::unique_ptr<Handler> handler,
115+
UpdateMediaInfoCB update_media_info_cb,
116+
SbPlayerDecoderStatusFunc decoder_status_func,
117+
SbPlayerStatusFunc player_status_func,
118+
SbPlayerErrorFunc player_error_func,
119+
SbPlayer player,
120+
void* context);
123121
~PlayerWorker();
124122

123+
PlayerWorker(const PlayerWorker&) = delete;
124+
PlayerWorker& operator=(const PlayerWorker&) = delete;
125+
125126
void Seek(int64_t seek_to_time, int ticket) {
126127
job_thread_->Schedule(
127128
[this, seek_to_time, ticket] { DoSeek(seek_to_time, ticket); });
@@ -173,19 +174,6 @@ class PlayerWorker {
173174
}
174175

175176
private:
176-
PlayerWorker(SbMediaAudioCodec audio_codec,
177-
SbMediaVideoCodec video_codec,
178-
std::unique_ptr<Handler> handler,
179-
UpdateMediaInfoCB update_media_info_cb,
180-
SbPlayerDecoderStatusFunc decoder_status_func,
181-
SbPlayerStatusFunc player_status_func,
182-
SbPlayerErrorFunc player_error_func,
183-
SbPlayer player,
184-
void* context);
185-
186-
PlayerWorker(const PlayerWorker&) = delete;
187-
PlayerWorker& operator=(const PlayerWorker&) = delete;
188-
189177
void UpdateMediaInfo(int64_t time, int dropped_video_frames, bool underflow);
190178

191179
SbPlayerState player_state() const { return player_state_; }

0 commit comments

Comments
 (0)