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

Fix crash in audio source #3

Merged
merged 1 commit into from
Jan 21, 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: 6 additions & 3 deletions src/DateTimeSources.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ std::shared_ptr<DateSource> DateSource::Create() {
}

void DateSource::Evaluate() {
// TODO: Check dirty
// TODO: To save some redraws the redraw flag should check if date has changed since last draw
m_published = true; // No need to publish
m_drawn = false;
}

std::shared_ptr<TimeSource> TimeSource::Create(MainLoop& mainLoop,
Expand Down Expand Up @@ -53,7 +55,8 @@ bool TimeSource::OnRead() {
// Either block or no events
return false;
}
m_sourceDirtyFlag = true;
m_published = true; // No need to publish
m_drawn = false;
m_dateSource->Evaluate();
return m_sourceDirtyFlag;
return !m_drawn;
}
2 changes: 2 additions & 0 deletions src/DateTimeSources.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class DateSource : public Source {
static std::shared_ptr<DateSource> Create();
void Evaluate();
virtual ~DateSource() {}
void Publish(const std::string_view, ScriptContext&) override {}

private:
DateSource() {}
Expand All @@ -19,6 +20,7 @@ class TimeSource : public Source, public IoHandler {
std::shared_ptr<DateSource> dateSource);
virtual ~TimeSource();
virtual bool OnRead() override;
void Publish(const std::string_view, ScriptContext&) override {}

private:
TimeSource(int fd, std::shared_ptr<DateSource> dateSource)
Expand Down
13 changes: 7 additions & 6 deletions src/MainLoop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@ void MainLoop::Run() {
}
}
}
if (anyDirty) {
for (auto& handler : m_batchHandlers) {
handler->OnBatchProcessed();
}
if (anyDirty && m_batchHandler) {
m_batchHandler->OnBatchProcessed();
}
} while (m_polls.size() > 0);
}
Expand All @@ -66,8 +64,11 @@ void MainLoop::Register(int fd, const std::string_view name, std::shared_ptr<IoH
spdlog::debug("Registering {} in main loop for fd {}", name, fd);
}

void MainLoop::RegisterBatchHandler(std::shared_ptr<IoBatchHandler> ioBatchHandler) {
m_batchHandlers.push_back(ioBatchHandler);
void MainLoop::RegisterBatchHandler(std::shared_ptr<IoBatchHandler> batchHandler) {
if (m_batchHandler) {
spdlog::error("Only one batch handler supported");
}
m_batchHandler = batchHandler;
}

void MainLoop::Wakeup() {
Expand Down
2 changes: 1 addition & 1 deletion src/MainLoop.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ class MainLoop {
std::mutex m_wakupMutex;
std::vector<pollfd> m_polls;
std::map<int, std::shared_ptr<IoHandler>> m_handlers;
std::vector<std::shared_ptr<IoBatchHandler>> m_batchHandlers;
std::shared_ptr<IoBatchHandler> m_batchHandler;
};
27 changes: 6 additions & 21 deletions src/Manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,8 @@
#include "spdlog/spdlog.h"
#include "src/Registry.h"

std::shared_ptr<Manager> Manager::Create(std::shared_ptr<Registry> registry,
std::string_view sourceName, MainLoop& mainLoop,
std::unique_ptr<Sources> sources,
std::shared_ptr<ScriptContext> scriptContext) {
auto manager = std::shared_ptr<Manager>(
new Manager(registry, sourceName, std::move(sources), scriptContext));
// Register as a source
manager->m_sources->Register(sourceName, manager);
// Register source batch handler
mainLoop.RegisterBatchHandler(manager);
std::shared_ptr<Manager> Manager::Create(std::shared_ptr<Registry> registry) {
auto manager = std::shared_ptr<Manager>(new Manager(registry));
// Register click handler
if (!registry->seat) {
spdlog::error("No seat in registry");
Expand All @@ -29,39 +21,32 @@ void Manager::ClickSurface(wl_surface* surface, int x, int y) {
}

void Manager::OnBatchProcessed() {
// Always publish sources
m_sources->PublishAll();
// No need to redraw when not visible and not in transition
if (!m_visibilityChanged && !m_isVisible) return;

spdlog::debug("Processing batch of dirty sources");
if (m_visibilityChanged) {
m_visibilityChanged = false;
if (m_isVisible) {
m_sources->DirtyAll();
m_sources->ForceRedraw();
} else {
m_registry->BorrowOutputs().Hide(*m_registry);
return;
}
}
m_registry->BorrowOutputs().Draw(*m_registry, *m_sources);
m_sources->CleanAll();
}

void Manager::Publish(const Displays& displays) {
m_scriptContext->Publish(m_sourceName, displays);
m_sourceDirtyFlag = true;
OnBatchProcessed();
m_sources->SetAllDrawn();
}

void Manager::Hide() {
m_isVisible = false;
m_visibilityChanged = true;
m_sourceDirtyFlag = true;
OnBatchProcessed();
}

void Manager::Show() {
m_isVisible = true;
m_visibilityChanged = true;
m_sourceDirtyFlag = true;
OnBatchProcessed();
}
23 changes: 7 additions & 16 deletions src/Manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,26 @@
#include "src/ScriptContext.h"
#include "src/Sources.h"

class Manager : public IoBatchHandler, public Source {
class Manager : public IoBatchHandler {
public:
static std::shared_ptr<Manager> Create(std::shared_ptr<Registry> registry,
std::string_view sourceName, MainLoop& mainLoop,
std::unique_ptr<Sources> sources,
std::shared_ptr<ScriptContext> scriptContext);
static std::shared_ptr<Manager> Create(std::shared_ptr<Registry> registry);
void SetSources(std::unique_ptr<Sources> sources) { m_sources = std::move(sources); }
virtual ~Manager() {}
// When a batch of IO events has been processed and sources are potentially dirty
// When a batch of IO events has been processed and sources needs to be published and/or needs
// to redrawn
void OnBatchProcessed() override;

// Used by compositor implementation
// Compositors tells manager when the overlays should be visible
void Show();
void Hide();
void Publish(const Displays& displays);

void ClickSurface(wl_surface* surface, int x, int y);

private:
Manager(std::shared_ptr<Registry> registry, std::string_view sourceName,
std::unique_ptr<Sources> sources, std::shared_ptr<ScriptContext> scriptContext)
: m_registry(registry),
m_sourceName(sourceName),
m_sources(std::move(sources)),
m_scriptContext(scriptContext) {}
Manager(std::shared_ptr<Registry> registry) : m_registry(registry) {}

std::shared_ptr<Registry> m_registry;
std::string m_sourceName;
bool m_isVisible;
bool m_visibilityChanged;
std::unique_ptr<Sources> m_sources;
std::shared_ptr<ScriptContext> m_scriptContext;
};
21 changes: 12 additions & 9 deletions src/NetworkSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ static char *get_ip_str(const struct sockaddr *sa, char *s, size_t maxlen) {

return s;
}
std::shared_ptr<NetworkSource> NetworkSource::Create(std::string_view name, MainLoop &mainLoop,
std::shared_ptr<ScriptContext> scriptContext) {
std::shared_ptr<NetworkSource> NetworkSource::Create(MainLoop &mainLoop) {
auto sock = socket(PF_INET, SOCK_DGRAM, 0);
if (sock < 0) {
return nullptr;
Expand All @@ -48,8 +47,8 @@ std::shared_ptr<NetworkSource> NetworkSource::Create(std::string_view name, Main
spdlog::error("Failed to set timer: {}", strerror(errno));
return nullptr;
}
auto source = std::shared_ptr<NetworkSource>(new NetworkSource(name, sock, fd, scriptContext));
mainLoop.Register(fd, name, source);
auto source = std::shared_ptr<NetworkSource>(new NetworkSource(sock, fd));
mainLoop.Register(fd, "NetworkSource", source);
return source;
}

Expand Down Expand Up @@ -90,10 +89,9 @@ void NetworkSource::ReadState() {
network.address = get_ip_str(addr, ip, sizeof(ip));
networks.push_back(std::move(network));
}
m_sourceDirtyFlag = true; // m_networks != networks;
if (m_sourceDirtyFlag) {
if (m_networks != networks) {
m_drawn = m_published = false;
m_networks = networks;
m_scriptContext->Publish(m_name, m_networks);
}
}

Expand All @@ -105,6 +103,11 @@ bool NetworkSource::OnRead() {
// Either block or no events
return false;
}
ReadState();
return m_sourceDirtyFlag;
return !m_published;
}

void NetworkSource::Publish(const std::string_view sourceName, ScriptContext &scriptContext) {
if (m_published) return;
scriptContext.Publish(sourceName, m_networks);
m_published = true;
}
9 changes: 3 additions & 6 deletions src/NetworkSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,16 @@

class NetworkSource : public Source, public IoHandler {
public:
static std::shared_ptr<NetworkSource> Create(std::string_view name, MainLoop& mainLoop,
std::shared_ptr<ScriptContext> scriptContext);
static std::shared_ptr<NetworkSource> Create(MainLoop& mainLoop);
void Initialize();
void ReadState();
virtual bool OnRead() override;
void Publish(const std::string_view sourceName, ScriptContext& scriptContext) override;
virtual ~NetworkSource() { close(m_timerfd); }

private:
NetworkSource(std::string_view name, int socket, int timerfd,
std::shared_ptr<ScriptContext> scriptContext)
: m_name(name), m_socket(socket), m_timerfd(timerfd), m_scriptContext(scriptContext) {}
NetworkSource(int socket, int timerfd) : m_socket(socket), m_timerfd(timerfd) {}

const std::string m_name;
int m_socket;
int m_timerfd;
std::shared_ptr<ScriptContext> m_scriptContext;
Expand Down
2 changes: 1 addition & 1 deletion src/Output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ void Outputs::Draw(const Registry &registry, const Sources &sources) {
for (const auto &panelConfig : m_config->panels) {
bool dirty = false;
for (const auto &widgetConfig : panelConfig.widgets) {
if (sources.IsDirty(widgetConfig.sources)) {
if (sources.NeedsRedraw(widgetConfig.sources)) {
dirty = true;
break;
}
Expand Down
27 changes: 15 additions & 12 deletions src/PowerSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
#include <optional>
#include <string>

std::shared_ptr<PowerSource> PowerSource::Create(std::string_view name, MainLoop& mainLoop,
std::shared_ptr<ScriptContext> scriptContext) {
std::shared_ptr<PowerSource> PowerSource::Create(MainLoop& mainLoop) {
// Use non blocking to make sure we never hang on read
auto fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC | TFD_NONBLOCK);
if (fd == -1) {
Expand All @@ -23,15 +22,14 @@ std::shared_ptr<PowerSource> PowerSource::Create(std::string_view name, MainLoop
spdlog::error("Failed to set timer: {}", strerror(errno));
return nullptr;
}
auto source = std::shared_ptr<PowerSource>(new PowerSource(name, fd, scriptContext));
mainLoop.Register(fd, name, source);
auto source = std::shared_ptr<PowerSource>(new PowerSource(fd));
mainLoop.Register(fd, "PowerSource", source);
return source;
}

bool PowerSource::Initialize() {
// Publish initial value to make sure that something is published
m_scriptContext->Publish(m_name,
PowerState{.IsPluggedIn = false, .IsCharging = false, .Capacity = 0});
m_drawn = m_published = false;
m_sourceState = PowerState{.IsPluggedIn = false, .IsCharging = false, .Capacity = 0};
// TODO: Probe that these exists
m_ac = "/sys/class/power_supply/AC/online";
// TODO: Always BAT0?
Expand Down Expand Up @@ -91,12 +89,11 @@ void PowerSource::ReadState() {
if (maybeString) {
state.IsPluggedIn = std::stoi(*maybeString) != 0;
}
m_sourceDirtyFlag = state != m_sourceState;
if (m_sourceDirtyFlag) {
m_sourceState = state;
m_scriptContext->Publish(m_name, m_sourceState);
if (state != m_sourceState) {
spdlog::info("Power status changed, capacity {}, charging {}, plugged in {}",
state.Capacity, state.IsCharging, state.IsPluggedIn);
m_sourceState = state;
m_drawn = m_published = false;
}
}

Expand All @@ -111,5 +108,11 @@ bool PowerSource::OnRead() {
return false;
}
ReadState();
return m_sourceDirtyFlag;
return !m_published;
}

void PowerSource::Publish(const std::string_view sourceName, ScriptContext& scriptContext) {
if (m_published) return;
scriptContext.Publish(sourceName, m_sourceState);
m_published = true;
}
9 changes: 3 additions & 6 deletions src/PowerSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,18 @@

class PowerSource : public Source, public IoHandler {
public:
static std::shared_ptr<PowerSource> Create(std::string_view name, MainLoop& mainLoop,
std::shared_ptr<ScriptContext> scriptContext);
static std::shared_ptr<PowerSource> Create(MainLoop& mainLoop);
bool Initialize();
void ReadState();
virtual bool OnRead() override;
void Publish(const std::string_view sourceName, ScriptContext& scriptContext) override;
virtual ~PowerSource();

private:
PowerSource(std::string_view name, int fd, std::shared_ptr<ScriptContext> scriptContext)
: m_name(name), m_timerfd(fd), m_scriptContext(scriptContext) {}
const std::string m_name;
PowerSource(int fd) : m_timerfd(fd) {}
std::filesystem::path m_batteryCapacity;
std::filesystem::path m_batteryStatus;
std::filesystem::path m_ac;
int m_timerfd;
std::shared_ptr<ScriptContext> m_scriptContext;
PowerState m_sourceState;
};
28 changes: 18 additions & 10 deletions src/PulseAudioSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ static void on_subscribe(pa_context* ctx, pa_subscription_event_type_t event_and
}
}

std::unique_ptr<PulseAudioSource> PulseAudioSource::Create(
std::string_view name, std::shared_ptr<MainLoop> zenMainloop,
std::shared_ptr<ScriptContext> scriptContext) {
std::unique_ptr<PulseAudioSource> PulseAudioSource::Create(std::shared_ptr<MainLoop> zenMainloop) {
auto mainloop = pa_threaded_mainloop_new();
if (!mainloop) return nullptr;
pa_threaded_mainloop_lock(mainloop);
Expand All @@ -59,7 +57,7 @@ std::unique_ptr<PulseAudioSource> PulseAudioSource::Create(
return nullptr;
}
auto backend = std::unique_ptr<PulseAudioSource>(
new PulseAudioSource(name, zenMainloop, mainloop, scriptContext, api, context));
new PulseAudioSource(zenMainloop, mainloop, api, context));
// From now on the backend will free on error

if (pa_context_connect(context, nullptr, PA_CONTEXT_NOFAIL, nullptr) < 0) {
Expand Down Expand Up @@ -144,10 +142,20 @@ void PulseAudioSource::OnSinkChange(const pa_sink_info* info) {
break;
}
}
if (newState == m_sourceState) return;
spdlog::debug("Audio source is dirty");
m_sourceDirtyFlag = true;
m_sourceState = newState;
m_scriptContext->Publish(m_name, m_sourceState);
m_zenMainloop->Wakeup();
{
std::lock_guard<std::mutex> lock(m_mutex);
if (newState == m_sourceState) return;
spdlog::debug("Audio source is dirty");
m_drawn = m_published = false;
m_sourceState = newState;
m_zenMainloop->Wakeup();
}
}

// This is invoked on main thread. Can not publish on another thread
void PulseAudioSource::Publish(const std::string_view sourceName, ScriptContext& scriptContext) {
std::lock_guard<std::mutex> lock(m_mutex);
if (m_published) return;
scriptContext.Publish(sourceName, m_sourceState);
m_published = true;
}
Loading