Skip to content

Commit

Permalink
Clear StreamPlayers when doing global file operations
Browse files Browse the repository at this point in the history
Summary: In global filter/copy operations, we better make sure that there isn't any previously registered StreamPlayer that would be unrelated.

Reviewed By: kiminoue7

Differential Revision: D51573459

fbshipit-source-id: d988e7f2d8b656601b871eeae073f86ece71d7c5
  • Loading branch information
Georges Berenger authored and facebook-github-bot committed Nov 29, 2023
1 parent da0a9b5 commit eb171ff
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 11 deletions.
3 changes: 0 additions & 3 deletions vrs/RecordFileReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,6 @@ int RecordFileReader::closeFile() {
}

int RecordFileReader::clearStreamPlayers() {
if (file_->isOpened()) {
return INVALID_REQUEST;
}
streamPlayers_.clear();
return 0;
}
Expand Down
3 changes: 1 addition & 2 deletions vrs/RecordFileReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ class RecordFileReader {
void setStreamPlayer(StreamId streamId, StreamPlayer* streamPlayer);

/// Remove all registered stream players.
/// @return O on success, or an error if the file isn't closed, and stream players were not
/// removed.
/// @return O on success.
int clearStreamPlayers();

/// When streaming a VRS file from the cloud, it may be very beneficial to tell before hand which
Expand Down
1 change: 1 addition & 0 deletions vrs/utils/AudioTrackExtractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ string extractAudioTrack(FilteredFileReader& filteredReader, const std::string&
return failure(doc, jsonFilePath);
}
}
filteredReader.reader.clearStreamPlayers();
bool stop = false;
unique_ptr<AudioTrackExtractor> audioExtractor;
StreamId streamId;
Expand Down
3 changes: 3 additions & 0 deletions vrs/utils/FilterCopy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ int filterCopy(
ThrottledWriter throttledWriter(copyOptions, *throttledFileDelegate);
RecordFileWriter& writer = throttledWriter.getWriter();
writer.addTags(filteredReader.reader.getTags());
filteredReader.reader.clearStreamPlayers();
vector<unique_ptr<StreamPlayer>> filters;
filters.reserve(filteredReader.filter.streams.size());
{
Expand Down Expand Up @@ -196,6 +197,7 @@ int filterMerge(
// setup the record file writer and hook-up the readers to record copiers, and copy/merge tags
ThrottledWriter throttledWriter(copyOptions, *throttledFileDelegate);
RecordFileWriter& recordFileWriter = throttledWriter.getWriter();
firstRecordFilter.reader.clearStreamPlayers();
list<NoDuplicateCopier> copiers; // to delete copiers on exit
// track copiers by recordable type id in sequence/instance order in the output file
map<RecordableTypeId, vector<NoDuplicateCopier*>> copiersMap;
Expand All @@ -210,6 +212,7 @@ int filterMerge(
firstRecordFilter.getTimeRange(startTimestamp, endTimestamp);
for (auto* recordFilter : moreRecordFilters) {
RecordFileReader& reader = recordFilter->reader;
reader.clearStreamPlayers();
recordFilter->expandTimeRange(startTimestamp, endTimestamp);
// add the global tags
map<string, string> tags;
Expand Down
4 changes: 1 addition & 3 deletions vrs/utils/FilteredFileReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -623,9 +623,7 @@ uint32_t FilteredFileReader::iterateAdvanced(ThrottledWriter* throttledWriter) {
return true;
},
throttledWriter);
for (auto id : filter.streams) {
reader.setStreamPlayer(id, nullptr);
}
reader.clearStreamPlayers();
return readCounter;
}

Expand Down
8 changes: 5 additions & 3 deletions vrs/utils/Validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,7 @@ bool iterateChecker(
return outNoError;
},
throttledWriter);
for (auto id : reader.filter.streams) {
reader.reader.setStreamPlayer(id, nullptr);
}
reader.reader.clearStreamPlayers();
outDuration = os::getTimestampSec() - beforeTime;
return true;
}
Expand Down Expand Up @@ -295,6 +293,7 @@ class DecodeChecker : public VideoRecordFormatStreamPlayer {
string decodeValidation(FilteredFileReader& filteredReader, const CopyOptions& copyOptions) {
uint32_t decodeErrorCount{0};
uint32_t imageCount{0};
filteredReader.reader.clearStreamPlayers();
vector<unique_ptr<DecodeChecker>> checkers;
for (auto id : filteredReader.filter.streams) {
checkers.emplace_back(make_unique<DecodeChecker>(decodeErrorCount, imageCount));
Expand Down Expand Up @@ -350,6 +349,7 @@ string checkRecords(
if (!filteredReader.reader.isOpened()) {
return {};
}
filteredReader.reader.clearStreamPlayers();
if (checkType == CheckType::Decode) {
return decodeValidation(filteredReader, copyOptions);
}
Expand Down Expand Up @@ -770,6 +770,8 @@ bool compareVRSfiles(
if (!match) {
return false;
}
first.reader.clearStreamPlayers();
second.reader.clearStreamPlayers();
atomic<int> diffCounter{0};
bool noError;
vector<unique_ptr<RecordHolder>> holders;
Expand Down
2 changes: 2 additions & 0 deletions vrs/utils/cli/DataExtraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ void extractImages(
imageNamer = &defaultImageNamer;
}
imageNamer->init(filteredReader.reader);
filteredReader.reader.clearStreamPlayers();
uint32_t imageCounter = 0;
deque<unique_ptr<StreamPlayer>> extractors;
for (auto id : filteredReader.filter.streams) {
Expand Down Expand Up @@ -73,6 +74,7 @@ int extractAudio(const string& path, FilteredFileReader& filteredReader) {
return FAILURE;
}
}
filteredReader.reader.clearStreamPlayers();
uint32_t audioFileCount = 0;
uint32_t streamCount = 0;
deque<unique_ptr<StreamPlayer>> extractors;
Expand Down
1 change: 1 addition & 0 deletions vrs/utils/cli/ListRecords.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class RecordLister : public StreamPlayer {
namespace vrs::utils {

void listRecords(utils::FilteredFileReader& filteredReader) {
filteredReader.reader.clearStreamPlayers();
RecordLister lister;
for (auto id : filteredReader.filter.streams) {
filteredReader.reader.setStreamPlayer(id, &lister);
Expand Down
1 change: 1 addition & 0 deletions vrs/utils/cli/PrintRecordFormatRecords.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ class DataLayoutPrinter : public RecordFormatStreamPlayer {
namespace vrs::utils {

void printRecordFormatRecords(FilteredFileReader& filteredReader, PrintoutType type) {
filteredReader.reader.clearStreamPlayers();
DataLayoutPrinter lister(type);
for (auto id : filteredReader.filter.streams) {
filteredReader.reader.setStreamPlayer(id, &lister);
Expand Down

0 comments on commit eb171ff

Please sign in to comment.