Skip to content

Fix #538: Fix playback of sound clones #539

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

Merged
merged 2 commits into from
Apr 13, 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
6 changes: 6 additions & 0 deletions include/scratchcpp/sound.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace libscratchcpp
{

class Target;
class SoundPrivate;

/*! \brief The Sound class represents a Scratch sound. */
Expand All @@ -32,12 +33,17 @@ class LIBSCRATCHCPP_EXPORT Sound : public Asset

virtual bool isPlaying();

Target *target() const;
void setTarget(Target *target);

std::shared_ptr<Sound> clone() const;

protected:
void processData(unsigned int size, void *data) override;

private:
void stopCloneSounds();

spimpl::unique_impl_ptr<SoundPrivate> impl;
};

Expand Down
7 changes: 2 additions & 5 deletions src/engine/internal/engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -459,14 +459,11 @@ void Engine::deinitClone(std::shared_ptr<Sprite> clone)

void Engine::stopSounds()
{
// TODO: Loop through clones
for (auto target : m_targets) {
const auto &sounds = target->sounds();

for (auto sound : sounds) {
if (sound->isPlaying())
sound->stop();
}
for (auto sound : sounds)
sound->stop();
}
}

Expand Down
44 changes: 44 additions & 0 deletions src/scratch/sound.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: Apache-2.0

#include <scratchcpp/sound.h>
#include <scratchcpp/sprite.h>
#include <iostream>

#include "sound_p.h"
Expand Down Expand Up @@ -47,12 +48,16 @@ void Sound::setVolume(double volume)
/*! Starts the playback of the sound. */
void Sound::start()
{
// Stop sounds in clones (#538)
stopCloneSounds();
impl->player->start();
}

/*! Stops the playback of the sound. */
void Sound::stop()
{
// Stop sounds in clones (#538)
stopCloneSounds();
impl->player->stop();
}

Expand All @@ -62,6 +67,18 @@ bool Sound::isPlaying()
return impl->player->isPlaying();
}

/*! Returns the sprite or stage this variable belongs to. */
Target *Sound::target() const
{
return impl->target;
}

/*! Sets the sprite or stage this variable belongs to. */
void Sound::setTarget(Target *target)
{
impl->target = target;
}

/*! Returns an independent copy of the sound which is valid for as long as the original sound exists. */
std::shared_ptr<Sound> Sound::clone() const
{
Expand All @@ -83,3 +100,30 @@ void Sound::processData(unsigned int size, void *data)
if (!impl->player->load(size, data, impl->rate))
std::cerr << "Failed to load sound " << name() << std::endl;
}

void Sound::stopCloneSounds()
{
if (impl->target && !impl->target->isStage()) {
Sprite *sprite = static_cast<Sprite *>(impl->target);

if (sprite->isClone())
sprite = sprite->cloneSprite();

// Stop the sound in the sprite (clone root)
auto sound = sprite->soundAt(sprite->findSound(name()));

if (sound && sound.get() != this)
sound->impl->player->stop();

// Stop the sound in clones
const auto &clones = sprite->clones();

for (auto clone : clones) {
auto sound = clone->soundAt(clone->findSound(name()));
assert(sound);

if (sound && sound.get() != this)
sound->impl->player->stop();
}
}
}
3 changes: 3 additions & 0 deletions src/scratch/sound_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
namespace libscratchcpp
{

class Target;

struct SoundPrivate
{
SoundPrivate();
Expand All @@ -19,6 +21,7 @@ struct SoundPrivate
int sampleCount = 0;
static IAudioOutput *audioOutput;
std::shared_ptr<IAudioPlayer> player = nullptr;
Target *target = nullptr;
};

} // namespace libscratchcpp
4 changes: 3 additions & 1 deletion src/scratch/target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,10 @@ int Target::addSound(std::shared_ptr<Sound> sound)

assert(sound);

if (sound)
if (sound) {
sound->setTarget(this);
sound->setVolume(impl->volume);
}

impl->sounds.push_back(sound);
return impl->sounds.size() - 1;
Expand Down
70 changes: 60 additions & 10 deletions test/assets/sound_test.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#include <scratchcpp/sound.h>
#include <scratchcpp/sprite.h>
#include <scratch/sound_p.h>
#include <audiooutputmock.h>
#include <audioplayermock.h>
#include <enginemock.h>

#include "../common.h"

Expand Down Expand Up @@ -106,30 +108,78 @@ TEST_F(SoundTest, IsPlaying)
SoundPrivate::audioOutput = nullptr;
}

TEST_F(SoundTest, Clone)
TEST_F(SoundTest, Target)
{
Sound sound("sound1", "a", "wav");
sound.setRate(44100);
sound.setSampleCount(10000);
ASSERT_EQ(sound.target(), nullptr);

Target target;
sound.setTarget(&target);
ASSERT_EQ(sound.target(), &target);
}

TEST_F(SoundTest, Clone)
{
auto sound = std::make_shared<Sound>("sound1", "a", "wav");
sound->setRate(44100);
sound->setSampleCount(10000);

const char *data = "abc";
void *dataPtr = const_cast<void *>(static_cast<const void *>(data));

EXPECT_CALL(*m_player, isLoaded()).WillOnce(Return(false));
EXPECT_CALL(*m_player, load(3, dataPtr, 44100)).WillOnce(Return(true));
sound.setData(3, dataPtr);
sound->setData(3, dataPtr);

auto clonePlayer = std::make_shared<AudioPlayerMock>();
EXPECT_CALL(m_playerFactory, createAudioPlayer()).WillOnce(Return(clonePlayer));
EXPECT_CALL(*clonePlayer, loadCopy(m_player.get())).WillOnce(Return(true));
EXPECT_CALL(*m_player, volume()).WillOnce(Return(0.45));
EXPECT_CALL(*clonePlayer, setVolume(0.45));
EXPECT_CALL(*clonePlayer, isLoaded()).WillOnce(Return(true));
auto clone = sound.clone();
auto clone = sound->clone();
ASSERT_TRUE(clone);
ASSERT_EQ(clone->name(), sound.name());
ASSERT_EQ(clone->id(), sound.id());
ASSERT_EQ(clone->dataFormat(), sound.dataFormat());
ASSERT_EQ(clone->rate(), sound.rate());
ASSERT_EQ(clone->sampleCount(), sound.sampleCount());
ASSERT_EQ(clone->name(), sound->name());
ASSERT_EQ(clone->id(), sound->id());
ASSERT_EQ(clone->dataFormat(), sound->dataFormat());
ASSERT_EQ(clone->rate(), sound->rate());
ASSERT_EQ(clone->sampleCount(), sound->sampleCount());

// Stopping/starting the sound should stop its clones
auto anotherPlayer = std::make_shared<AudioPlayerMock>();
EXPECT_CALL(m_playerFactory, createAudioPlayer()).WillOnce(Return(anotherPlayer));
auto another = std::make_shared<Sound>("another", "c", "mp3");
Sprite sprite;
EngineMock engine;
sprite.setEngine(&engine);

EXPECT_CALL(engine, cloneLimit()).WillOnce(Return(-1));
EXPECT_CALL(engine, initClone);
EXPECT_CALL(engine, requestRedraw);
EXPECT_CALL(engine, moveSpriteBehindOther);
auto spriteClone = sprite.clone();

EXPECT_CALL(*anotherPlayer, setVolume).Times(2);
EXPECT_CALL(*m_player, setVolume);
EXPECT_CALL(*clonePlayer, setVolume);
sprite.addSound(another);
sprite.addSound(sound);
spriteClone->addSound(another);
spriteClone->addSound(clone);

EXPECT_CALL(*m_player, stop());
EXPECT_CALL(*clonePlayer, stop());
sound->stop();

EXPECT_CALL(*m_player, stop());
EXPECT_CALL(*clonePlayer, stop());
clone->stop();

EXPECT_CALL(*m_player, start());
EXPECT_CALL(*clonePlayer, stop());
sound->start();

EXPECT_CALL(*m_player, stop());
EXPECT_CALL(*clonePlayer, start());
clone->start();
}
8 changes: 5 additions & 3 deletions test/blocks/sound_blocks_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
#include <scratchcpp/block.h>
#include <scratchcpp/input.h>
#include <scratchcpp/field.h>
#include <scratchcpp/target.h>
#include <scratchcpp/sound.h>
#include <scratch/sound_p.h>
#include <enginemock.h>
#include <audiooutputmock.h>
#include <audioplayermock.h>
#include <targetmock.h>

#include "../common.h"
#include "blocks/soundblocks.h"
Expand Down Expand Up @@ -212,7 +212,8 @@ TEST_F(SoundBlocksTest, PlayImpl)
EXPECT_CALL(*player1, setVolume);
EXPECT_CALL(*player2, setVolume);
EXPECT_CALL(*player3, setVolume);
Target target;
TargetMock target;
EXPECT_CALL(target, isStage()).WillRepeatedly(Return(true));
target.addSound(std::make_shared<Sound>("some sound", "", ""));
target.addSound(std::make_shared<Sound>("test", "", ""));
target.addSound(std::make_shared<Sound>("another sound", "", ""));
Expand Down Expand Up @@ -412,7 +413,8 @@ TEST_F(SoundBlocksTest, PlayUntilDoneImpl)
EXPECT_CALL(*player1, setVolume);
EXPECT_CALL(*player2, setVolume);
EXPECT_CALL(*player3, setVolume);
Target target;
TargetMock target;
EXPECT_CALL(target, isStage()).WillRepeatedly(Return(true));
target.addSound(std::make_shared<Sound>("some sound", "", ""));
target.addSound(std::make_shared<Sound>("test", "", ""));
target.addSound(std::make_shared<Sound>("another sound", "", ""));
Expand Down
4 changes: 1 addition & 3 deletions test/engine/engine_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,7 @@ TEST(EngineTest, StopSounds)

auto engine = p.engine();

EXPECT_CALL(*player1, isPlaying()).WillOnce(Return(false));
EXPECT_CALL(*player2, isPlaying()).WillOnce(Return(true));
EXPECT_CALL(*player3, isPlaying()).WillOnce(Return(true));
EXPECT_CALL(*player1, stop());
EXPECT_CALL(*player2, stop());
EXPECT_CALL(*player3, stop());
engine->stopSounds();
Expand Down
4 changes: 4 additions & 0 deletions test/scratch_classes/target_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,10 @@ TEST(TargetTest, Sounds)
ASSERT_EQ(target.findSound("sound2"), 1);
ASSERT_EQ(target.findSound("sound3"), 2);

ASSERT_EQ(s1->target(), &target);
ASSERT_EQ(s2->target(), &target);
ASSERT_EQ(s3->target(), &target);

SoundPrivate::audioOutput = nullptr;
}

Expand Down