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.

Sign up for GitHub

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

Added status full loaded into FileData #1246

Merged
merged 10 commits into from
Mar 20, 2024
Merged

Added status full loaded into FileData #1246

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1d004c2
Added FileData::Status::FullLoaded
KKQ-KKQ Feb 13, 2024
1177796
Improved setting FilData::Status
KKQ-KKQ Feb 14, 2024
18c7300
Improved status check in garbage collection process
KKQ-KKQ Feb 14, 2024
5e9c18a
Fixed status check order in garbage collection process
KKQ-KKQ Feb 14, 2024
5108c3b
Merge branch 'develop' into addedStatusFullLoaded
KKQ-KKQ Feb 15, 2024
b0b3467
FullyLoaded status becomes independent. Added GarbageCollecting status.
KKQ-KKQ Feb 16, 2024
d7f1d81
Improved check for garbage collection
KKQ-KKQ Feb 16, 2024
795fe2e
Improved check in garbage collection
KKQ-KKQ Feb 16, 2024
1563428
Fixed a error in garbage collection
KKQ-KKQ Feb 16, 2024
3985fe7
Improved process for maxOffset in FilePool::setPreloadSize()
KKQ-KKQ Feb 16, 2024
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
81 changes: 61 additions & 20 deletions src/sfizz/FilePool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,19 +332,24 @@ bool sfz::FilePool::preloadFile(const FileId& fileId, uint32_t maxOffset) noexce

const auto existingFile = preloadedFiles.find(fileId);
if (existingFile != preloadedFiles.end()) {
if (framesToLoad > existingFile->second.preloadedData.getNumFrames()) {
auto& fileData = existingFile->second;
if (framesToLoad > fileData.preloadedData.getNumFrames()) {
preloadedFiles[fileId].information.maxOffset = maxOffset;
preloadedFiles[fileId].preloadedData = readFromFile(*reader, framesToLoad);
if (frames == framesToLoad && fileData.status != FileData::Status::FullLoaded)
// Forced overwrite status
fileData.status = FileData::Status::FullLoaded;
}
existingFile->second.preloadCallCount++;
fileData.preloadCallCount++;
} else {
fileInformation->sampleRate = static_cast<double>(reader->sampleRate());
auto insertedPair = preloadedFiles.insert_or_assign(fileId, {
readFromFile(*reader, framesToLoad),
*fileInformation
});

insertedPair.first->second.status = FileData::Status::Preloaded;
// Initially set status
insertedPair.first->second.status = frames == framesToLoad ? FileData::Status::FullLoaded : FileData::Status::Preloaded;
insertedPair.first->second.preloadCallCount++;
}

Expand Down Expand Up @@ -399,7 +404,8 @@ sfz::FileDataHolder sfz::FilePool::loadFile(const FileId& fileId) noexcept
readFromFile(*reader, frames),
*fileInformation
});
insertedPair.first->second.status = FileData::Status::Preloaded;
// Initially set status
insertedPair.first->second.status = FileData::Status::FullLoaded;
insertedPair.first->second.preloadCallCount++;
return { &insertedPair.first->second };
}
Expand All @@ -417,7 +423,8 @@ sfz::FileDataHolder sfz::FilePool::loadFromRam(const FileId& fileId, const std::
readFromFile(*reader, frames),
*fileInformation
});
insertedPair.first->second.status = FileData::Status::Preloaded;
// Initially set status
insertedPair.first->second.status = FileData::Status::FullLoaded;
insertedPair.first->second.preloadCallCount++;
DBG("Added a file " << fileId.filename());
return { &insertedPair.first->second };
Expand All @@ -435,8 +442,9 @@ sfz::FileDataHolder sfz::FilePool::getFilePromise(const std::shared_ptr<FileId>&
return {};
}

if (!loadInRam) {
QueuedFileData queuedData { fileId, &preloaded->second };
auto& fileData = preloaded->second;
if (fileData.status != FileData::Status::FullLoaded) {
QueuedFileData queuedData { fileId, &fileData };
if (!filesToLoad->try_push(queuedData)) {
DBG("[sfizz] Could not enqueue the file to load for " << fileId << " (queue capacity " << filesToLoad->capacity() << ")");
return {};
Expand All @@ -458,10 +466,25 @@ void sfz::FilePool::setPreloadSize(uint32_t preloadSize) noexcept

// Update all the preloaded sizes
for (auto& preloadedFile : preloadedFiles) {
const auto maxOffset = preloadedFile.second.information.maxOffset;
fs::path file { rootDirectory / preloadedFile.first.filename() };
AudioReaderPtr reader = createAudioReader(file, preloadedFile.first.isReverse());
preloadedFile.second.preloadedData = readFromFile(*reader, preloadSize + maxOffset);
auto& fileId = preloadedFile.first;
auto& fileData = preloadedFile.second;
const uint32_t maxOffset = fileData.information.maxOffset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we need to make the maxOffset in information a uint32_t. I don't recall why I put an int64_t, maybe for full generality and to better support integer arithmetics since you tend to add or remove offsets around. Was there a reason for you to force uint32_t here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I will fix this.

fs::path file { rootDirectory / fileId.filename() };
AudioReaderPtr reader = createAudioReader(file, fileId.isReverse());
const uint32_t frames = static_cast<uint32_t>(reader->frames());
const uint32_t framesToLoad = min(frames, maxOffset + preloadSize);
fileData.preloadedData = readFromFile(*reader, framesToLoad);

FileData::Status status = fileData.status;
bool fullLoaded = frames == framesToLoad;
if (fullLoaded && status != FileData::Status::FullLoaded) {
// Forced overwrite status
fileData.status = FileData::Status::FullLoaded;
}
else if (!fullLoaded && status == FileData::Status::FullLoaded) {
// Exchange status
fileData.status.compare_exchange_strong(status, FileData::Status::Preloaded);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as below re: compare_exchange without a while loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this process, the status should not be overwritten if the status is changed at the moment.

}
}
}

Expand Down Expand Up @@ -509,7 +532,10 @@ void sfz::FilePool::loadingJob(const QueuedFileData& data) noexcept

streamFromFile(*reader, data.data->fileData, &data.data->availableFrames);

data.data->status = FileData::Status::Done;
currentStatus = data.data->status.load();
if (currentStatus == FileData::Status::Streaming) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the compare_exchange below, shouldn't this be while (currentStatus != FileData::Status::Done)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

status should not be changed to Done when the status is changed to FullLoaded.
but it is a little bit dangerous in the following situation:

  1. fileData starts streaming
  2. status is changed to FullLoaded (e.g. setRamLoading(true))
  3. status is changed to Preloaded (e.g. setRamLoading(false))
  4. fileData finishes streaming

I will consider more.

data.data->status.compare_exchange_strong(currentStatus, FileData::Status::Done);
}

std::lock_guard<SpinMutex> guard { garbageAndLastUsedMutex };
if (absl::c_find(lastUsedFiles, *id) == lastUsedFiles.end())
Expand Down Expand Up @@ -619,10 +645,13 @@ void sfz::FilePool::setRamLoading(bool loadInRam) noexcept
for (auto& preloadedFile : preloadedFiles) {
fs::path file { rootDirectory / preloadedFile.first.filename() };
AudioReaderPtr reader = createAudioReader(file, preloadedFile.first.isReverse());
preloadedFile.second.preloadedData = readFromFile(
auto& fileData = preloadedFile.second;
fileData.preloadedData = readFromFile(
*reader,
preloadedFile.second.information.end
fileData.information.end
);
// Initially set status
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this comment I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added comments on every status change.

fileData.status = FileData::Status::FullLoaded;
}
} else {
setPreloadSize(preloadSize);
Expand All @@ -648,21 +677,33 @@ void sfz::FilePool::triggerGarbageCollection() noexcept
}

sfz::FileData& data = it->second;
if (data.status == FileData::Status::Preloaded)
return true;

if (data.status != FileData::Status::Done)
return false;

if (data.readerCount != 0)
return false;

auto status = data.status.load();
if (data.availableFrames == 0) {
return status == FileData::Status::Preloaded || status == FileData::Status::FullLoaded;
}

switch(status) {
paulfd marked this conversation as resolved.
Show resolved Hide resolved
case FileData::Status::Invalid:
case FileData::Status::Streaming:
return false;
default:
break;
}

const auto secondsIdle = std::chrono::duration_cast<std::chrono::seconds>(now - data.lastViewerLeftAt).count();
if (secondsIdle < config::fileClearingPeriod)
return false;

// Queue garbage collection
// At first set availableFromes to 0
// Then forced set status
data.availableFrames = 0;
data.status = FileData::Status::Preloaded;
if (status != FileData::Status::FullLoaded)
data.status = FileData::Status::Preloaded;
garbageToCollect.push_back(std::move(data.fileData));
return true;
});
Expand Down
2 changes: 1 addition & 1 deletion src/sfizz/FilePool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct FileInformation {
// Strict C++11 disallows member initialization if aggregate initialization is to be used...
struct FileData
{
enum class Status { Invalid, Preloaded, Streaming, Done };
enum class Status { Invalid, Preloaded, Streaming, Done, FullLoaded };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's call this FullyLoaded.

Copy link
Contributor Author

@KKQ-KKQ KKQ-KKQ Feb 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better that we have a boolean fullyLoaded variable on FileData instead of adding it's status enum.

FileData() = default;
FileData(FileAudioBuffer preloaded, FileInformation info)
: preloadedData(std::move(preloaded)), information(std::move(info))
Expand Down
Loading