fix: replace retained report sessions when re-adding queue items
This commit is contained in:
@@ -348,6 +348,33 @@ int main(int argc, char **argv) {
|
|||||||
expect(firstRows[0], @"progress", firstURL, 12, true);
|
expect(firstRows[0], @"progress", firstURL, 12, true);
|
||||||
expect(firstRows[2], @"progress", firstURL, 24, true);
|
expect(firstRows[2], @"progress", firstURL, 24, true);
|
||||||
expect(secondRows[0], @"ended", secondURL, 100, true);
|
expect(secondRows[0], @"ended", secondURL, 100, true);
|
||||||
|
const auto retired = root + "/retired.jsonl", replacement = root + "/replacement.jsonl";
|
||||||
|
{
|
||||||
|
playback::ProgressReports reports;
|
||||||
|
reports.registerFile(firstURL, retired);
|
||||||
|
reports.begin(firstURL);
|
||||||
|
reports.observe(sample(PLAYER_PAUSED, 12));
|
||||||
|
bool accepted = true;
|
||||||
|
try { reports.registerFile(firstURL, replacement); } catch (...) { accepted = false; }
|
||||||
|
require(accepted, "a removed identity can register a fresh reporting session");
|
||||||
|
auto retiredRows = records(retired);
|
||||||
|
require(retiredRows.count == 2, "replacement drains the retired writer");
|
||||||
|
expect(retiredRows[1], @"closed", firstURL, 12, true);
|
||||||
|
NSData *retiredBytes = [NSData dataWithContentsOfFile:nativePath(retired)];
|
||||||
|
reports.observe(sample(PLAYER_PAUSED, 50));
|
||||||
|
reports.begin(firstURL);
|
||||||
|
reports.observe(sample(PLAYER_PAUSED, 24));
|
||||||
|
reports.finish();
|
||||||
|
require([[NSData dataWithContentsOfFile:nativePath(retired)] isEqual:retiredBytes],
|
||||||
|
"replacement never appends to or overwrites the retired report");
|
||||||
|
auto replacementRows = records(replacement);
|
||||||
|
require(replacementRows.count == 2, "fresh session starts only when its media begins");
|
||||||
|
expect(replacementRows[0], @"progress", firstURL, 24, true);
|
||||||
|
expect(replacementRows[1], @"closed", firstURL, 24, true);
|
||||||
|
bool refused = false;
|
||||||
|
try { reports.registerFile(secondURL, retired); } catch (...) { refused = true; }
|
||||||
|
require(refused, "retired report paths remain reserved");
|
||||||
|
}
|
||||||
const auto liveSingle = root + "/live-single.jsonl", liveNext = root + "/live-next.jsonl";
|
const auto liveSingle = root + "/live-single.jsonl", liveNext = root + "/live-next.jsonl";
|
||||||
const auto rollbackPath = root + "/rollback.jsonl";
|
const auto rollbackPath = root + "/rollback.jsonl";
|
||||||
{
|
{
|
||||||
@@ -357,9 +384,9 @@ int main(int argc, char **argv) {
|
|||||||
const char *identities[] = {secondURL.c_str()}, *destinations[] = {liveNext.c_str()};
|
const char *identities[] = {secondURL.c_str()}, *destinations[] = {liveNext.c_str()};
|
||||||
reports.registerFiles(identities, destinations, 1);
|
reports.registerFiles(identities, destinations, 1);
|
||||||
reports.observe(sample(PLAYER_PAUSED, 8));
|
reports.observe(sample(PLAYER_PAUSED, 8));
|
||||||
const char *conflictingIDs[] = {"miruro:new:1", secondURL.c_str()};
|
const char *conflictingIDs[] = {firstURL.c_str(), secondURL.c_str()};
|
||||||
const auto unused = root + "/unused.jsonl";
|
const auto unused = root + "/unused.jsonl";
|
||||||
const char *conflictingPaths[] = {rollbackPath.c_str(), unused.c_str()};
|
const char *conflictingPaths[] = {rollbackPath.c_str(), liveNext.c_str()};
|
||||||
bool refused = false;
|
bool refused = false;
|
||||||
try { reports.registerFiles(conflictingIDs, conflictingPaths, 2); } catch (...) { refused = true; }
|
try { reports.registerFiles(conflictingIDs, conflictingPaths, 2); } catch (...) { refused = true; }
|
||||||
require(refused, "conflicting later entry rejects the complete registration batch");
|
require(refused, "conflicting later entry rejects the complete registration batch");
|
||||||
@@ -370,10 +397,17 @@ int main(int argc, char **argv) {
|
|||||||
reports.observe(sample(PLAYER_PAUSED, 9));
|
reports.observe(sample(PLAYER_PAUSED, 9));
|
||||||
reports.begin("miruro:unregistered:1");
|
reports.begin("miruro:unregistered:1");
|
||||||
reports.observe(sample(PLAYER_PAUSED, 55));
|
reports.observe(sample(PLAYER_PAUSED, 55));
|
||||||
refused = false;
|
NSData *singleBytes = [NSData dataWithContentsOfFile:nativePath(liveSingle)];
|
||||||
try { reports.registerFile(firstURL, unused); } catch (...) { refused = true; }
|
reports.registerFile(firstURL, unused);
|
||||||
require(refused, "enqueue cannot rebind the original single identity after its queue row is removed");
|
reports.begin(firstURL);
|
||||||
|
reports.observe(sample(PLAYER_PAUSED, 11));
|
||||||
reports.finish();
|
reports.finish();
|
||||||
|
require([[NSData dataWithContentsOfFile:nativePath(liveSingle)] isEqual:singleBytes],
|
||||||
|
"re-adding the original standalone identity leaves its old report unchanged");
|
||||||
|
auto reboundRows = records(unused);
|
||||||
|
require(reboundRows.count == 2, "standalone identity can start a fresh queue report");
|
||||||
|
expect(reboundRows[0], @"progress", firstURL, 11, true);
|
||||||
|
expect(reboundRows[1], @"closed", firstURL, 11, true);
|
||||||
require(reports.error().empty(), "enqueue preserves single reporting and supports owned report revisits");
|
require(reports.error().empty(), "enqueue preserves single reporting and supports owned report revisits");
|
||||||
}
|
}
|
||||||
auto singleRows = records(liveSingle), nextRows = records(liveNext);
|
auto singleRows = records(liveSingle), nextRows = records(liveNext);
|
||||||
|
|||||||
@@ -273,7 +273,7 @@ struct ResolvedPlaybackCheck {
|
|||||||
require(model.commandError != nil && model.queue.items.count == 4,
|
require(model.commandError != nil && model.queue.items.count == 4,
|
||||||
"registered report-path conflicts reject complete enqueue before files exist")
|
"registered report-path conflicts reject complete enqueue before files exist")
|
||||||
try deliver([(URL(string: "https://www.youtube.com/watch?v=DDDDDDDDDDD")!, directory.appendingPathComponent("valid.jsonl")),
|
try deliver([(URL(string: "https://www.youtube.com/watch?v=DDDDDDDDDDD")!, directory.appendingPathComponent("valid.jsonl")),
|
||||||
(URL(string: "https://example.com/unsupported")!, directory.appendingPathComponent("invalid.jsonl"))])
|
(URL(string: "file:///not-a-stream")!, directory.appendingPathComponent("invalid.jsonl"))])
|
||||||
require(model.commandError != nil && model.queue.items.count == 4,
|
require(model.commandError != nil && model.queue.items.count == 4,
|
||||||
"invalid later manifest entry never partially appends or registers reports")
|
"invalid later manifest entry never partially appends or registers reports")
|
||||||
try deliver([(URL(string: "https://www.youtube.com/watch?v=DDDDDDDDDDD")!, directory.appendingPathComponent("valid.jsonl"))])
|
try deliver([(URL(string: "https://www.youtube.com/watch?v=DDDDDDDDDDD")!, directory.appendingPathComponent("valid.jsonl"))])
|
||||||
@@ -302,7 +302,38 @@ struct ResolvedPlaybackCheck {
|
|||||||
try await until("original single video remains navigable", model: model) {
|
try await until("original single video remains navigable", model: model) {
|
||||||
!model.isBusy && model.state.status == PLAYER_PLAYING && model.fileName == first.absoluteString
|
!model.isBusy && model.state.status == PLAYER_PLAYING && model.fileName == first.absoluteString
|
||||||
}
|
}
|
||||||
|
var retiredReports: [(URL, Data, URL, URL)] = []
|
||||||
|
for (url, oldReport) in [(first, originalReport), (third, thirdReport)] {
|
||||||
|
let oldID = model.queue.items.first(where: { $0.url == url })!.id
|
||||||
|
model.removeQueueItem(oldID)
|
||||||
|
let currentID = model.queue.currentID
|
||||||
|
let freshReport = directory.appendingPathComponent(oldReport.lastPathComponent + ".fresh")
|
||||||
|
try deliver([(url, freshReport)])
|
||||||
|
require(model.commandError == nil && model.queue.items.count == 5 &&
|
||||||
|
model.queue.currentID == currentID, "removed video re-enqueues without interrupting current playback")
|
||||||
|
let retiredBytes = try Data(contentsOf: oldReport)
|
||||||
|
retiredReports.append((oldReport, retiredBytes, freshReport, url))
|
||||||
|
require(!FileManager.default.fileExists(atPath: freshReport.path), "re-added report remains lazy")
|
||||||
|
model.selectQueueItem(model.queue.items.last!.id)
|
||||||
|
try await until("re-added video decodes with a fresh session", model: model) {
|
||||||
|
!model.isBusy && model.state.status == PLAYER_PLAYING &&
|
||||||
|
model.queue.currentURL == url && model.state.position > 0.2
|
||||||
|
}
|
||||||
|
require(model.setPlayback(playing: false), "pause re-added video")
|
||||||
|
try await until("re-added pause acknowledged", model: model) { model.state.status == PLAYER_PAUSED }
|
||||||
|
}
|
||||||
model.shutdown()
|
model.shutdown()
|
||||||
|
for (oldReport, retiredBytes, freshReport, url) in retiredReports {
|
||||||
|
let finalBytes = try Data(contentsOf: oldReport)
|
||||||
|
require(finalBytes == retiredBytes, "re-added playback leaves the retired report unchanged")
|
||||||
|
let freshRows = try String(contentsOf: freshReport, encoding: .utf8).split(separator: "\n").map {
|
||||||
|
try JSONSerialization.jsonObject(with: Data($0.utf8)) as! [String: Any]
|
||||||
|
}
|
||||||
|
require(freshRows.contains { $0["event"] as? String == "closed" } &&
|
||||||
|
freshRows.contains { $0["event"] as? String == "progress" } &&
|
||||||
|
freshRows.allSatisfy { $0["media_url"] as? String == url.absoluteString },
|
||||||
|
"re-added video writes its acknowledged progress and close to the fresh report only")
|
||||||
|
}
|
||||||
for (file, expected) in [(originalReport, first.absoluteString), (thirdReport, third.absoluteString)] {
|
for (file, expected) in [(originalReport, first.absoluteString), (thirdReport, third.absoluteString)] {
|
||||||
let rows = try String(contentsOf: file, encoding: .utf8).split(separator: "\n").map {
|
let rows = try String(contentsOf: file, encoding: .utf8).split(separator: "\n").map {
|
||||||
try JSONSerialization.jsonObject(with: Data($0.utf8)) as! [String: Any]
|
try JSONSerialization.jsonObject(with: Data($0.utf8)) as! [String: Any]
|
||||||
|
|||||||
@@ -417,7 +417,7 @@ final class PlayerModel: ObservableObject {
|
|||||||
return item.url.absoluteString
|
return item.url.absoluteString
|
||||||
})
|
})
|
||||||
guard !streamQueue.entries.contains(where: { existing.contains($0.identity) }) else {
|
guard !streamQueue.entries.contains(where: { existing.contains($0.identity) }) else {
|
||||||
commandError = "Cannot enqueue a video already present in this queue; reports cannot be rebound."
|
commandError = "Cannot enqueue a video already present in this queue."
|
||||||
revealControls()
|
revealControls()
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -40,8 +40,9 @@ Player *player_create(void);
|
|||||||
// Opt-in reporting must be initialized before opening media. Creates a new 0600
|
// Opt-in reporting must be initialized before opening media. Creates a new 0600
|
||||||
// file exclusively; errors never change playback state. Error buffers hold 512 bytes.
|
// file exclusively; errors never change playback state. Error buffers hold 512 bytes.
|
||||||
bool player_set_progress_file(Player *player, const char *path, char *error);
|
bool player_set_progress_file(Player *player, const char *path, char *error);
|
||||||
// Register immutable queue destinations without changing current playback. Each path is new;
|
// Register queue destinations without changing playback. Each path is new and stays reserved.
|
||||||
// revisits append only to the original owned inode. Conflicting identities/paths are rejected.
|
// A fresh path replaces an identity's previous session after draining its writer; callers
|
||||||
|
// reject identities still queued. Revisits append only to the registered session's owned inode.
|
||||||
// Atomic batch: all registrations succeed or none do. Strings are copied before returning.
|
// Atomic batch: all registrations succeed or none do. Strings are copied before returning.
|
||||||
bool player_register_progress_files(Player *player, const char *const *identities,
|
bool player_register_progress_files(Player *player, const char *const *identities,
|
||||||
const char *const *paths, int count, char *error);
|
const char *const *paths, int count, char *error);
|
||||||
|
|||||||
@@ -305,20 +305,34 @@ void ProgressReports::registerFile(const std::string &identity, const std::strin
|
|||||||
void ProgressReports::registerFiles(const char *const *identities, const char *const *destinations, int count) {
|
void ProgressReports::registerFiles(const char *const *identities, const char *const *destinations, int count) {
|
||||||
std::lock_guard lock(mutex);
|
std::lock_guard lock(mutex);
|
||||||
if (!identities || !destinations || count < 1 || count > 4096 ||
|
if (!identities || !destinations || count < 1 || count > 4096 ||
|
||||||
entries.size() + count > 4096)
|
paths.size() + count > 4096)
|
||||||
throw std::runtime_error("Queue reports exceed the 4096 retained registration limit");
|
throw std::runtime_error("Queue reports exceed the 4096 retained registration limit");
|
||||||
std::map<std::string, Entry> additions;
|
std::map<std::string, Entry> additions;
|
||||||
std::set<std::string> newPaths;
|
std::set<std::string> newPaths;
|
||||||
for (int index = 0; index < count; ++index) {
|
for (int index = 0; index < count; ++index) {
|
||||||
const auto *identity = identities[index], *path = destinations[index];
|
const auto *identity = identities[index], *path = destinations[index];
|
||||||
if (!identity || !path || !ProgressReporter::validMediaIdentity(identity) ||
|
if (!identity || !path || !ProgressReporter::validMediaIdentity(identity) ||
|
||||||
!*path || *path != '/' || singleIdentity == identity || entries.contains(identity) || additions.contains(identity))
|
!*path || *path != '/' || additions.contains(identity))
|
||||||
throw std::runtime_error("Invalid or conflicting queue progress identity");
|
throw std::runtime_error("Invalid or conflicting queue progress identity");
|
||||||
if (singlePath == path || paths.contains(path) || !newPaths.insert(path).second)
|
if (singlePath == path || paths.contains(path) || !newPaths.insert(path).second)
|
||||||
throw std::runtime_error("Conflicting queue progress report path");
|
throw std::runtime_error("Conflicting queue progress report path");
|
||||||
additions.emplace(identity, Entry{path, nullptr});
|
additions.emplace(identity, Entry{path, nullptr});
|
||||||
}
|
}
|
||||||
// Node merges allocate nothing: failed validation cannot partially register a batch.
|
// Validate and allocate the whole batch before retiring any previous sessions.
|
||||||
|
auto retire = [this](ProgressReporter *reporter) {
|
||||||
|
if (!reporter) return;
|
||||||
|
reporter->finish();
|
||||||
|
if (active == reporter) active = nullptr;
|
||||||
|
try { if (failure.empty()) failure = reporter->error(); } catch (...) {}
|
||||||
|
};
|
||||||
|
for (auto &[identity, entry] : additions) {
|
||||||
|
if (singleIdentity == identity) retire(single.get());
|
||||||
|
if (auto found = entries.find(identity); found != entries.end()) {
|
||||||
|
retire(found->second.reporter.get());
|
||||||
|
std::swap(found->second, entry);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Node merges allocate nothing; replaced nodes remain in additions for destruction.
|
||||||
entries.merge(additions);
|
entries.merge(additions);
|
||||||
paths.merge(newPaths);
|
paths.merge(newPaths);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -49,13 +49,13 @@ Use canonical YouTube video pages or supported Miruro episode pages, never expir
|
|||||||
|
|
||||||
The supplied order is preserved (local-file batches still use natural filename order). Existing sidebar reorder/remove/play and previous/next navigation apply. Natural end advances without clearing the queue. Sources resolve only when selected, resolve afresh on revisits, and canceled/stale results cannot replace a newer selection. Labels start with descriptive page information and update from resolved titles/optional metadata.
|
The supplied order is preserved (local-file batches still use natural filename order). Existing sidebar reorder/remove/play and previous/next navigation apply. Natural end advances without clearing the queue. Sources resolve only when selected, resolve afresh on revisits, and canceled/stale results cannot replace a newer selection. Labels start with descriptive page information and update from resolved titles/optional metadata.
|
||||||
|
|
||||||
Enqueue uses native macOS URL-open delivery to the explicit application, not launch arguments: repeated requests reach the existing process. Both one-item and multi-item requests use the same manifest. The current selection, playback/pause state, timeline, pending resolution and metadata stay intact; an empty player selects and starts the first entry. Original single-file, single-stream and resolved-source playback remain navigable when items are appended. Enqueue never silently deduplicates: any identity already in the queue or retained report registrations, or any report-path collision, rejects the complete batch with an error in the player. Removed items retain their report registrations for the process lifetime; at most 4096 report identities may be registered. Existing files are never adopted as reporting sessions. Keep manifests and private sessions available until delivery; CLI success acknowledges macOS delivery, not GUI acceptance or playback.
|
Enqueue uses native macOS URL-open delivery to the explicit application, not launch arguments: repeated requests reach the existing process. Both one-item and multi-item requests use the same manifest. The current selection, playback/pause state, timeline, pending resolution and metadata stay intact; an empty player selects and starts the first entry. Original single-file, single-stream and resolved-source playback remain navigable when items are appended. Enqueue never silently deduplicates: any identity still in the queue, or any report-path collision, rejects the complete batch with an error in the player. Removed videos can be re-added with fresh report destinations, including the original standalone video. Old report paths remain reserved for the process lifetime; at most 4096 queue-report destinations may be registered, counting retired sessions. Existing files are never adopted as reporting sessions. Keep manifests and private sessions available until delivery; CLI success acknowledges macOS delivery, not GUI acceptance or playback.
|
||||||
|
|
||||||
Each report is immutably bound to its stable identity in the native decoder. Registration is atomic for a complete batch and can occur during playback without redirecting its current report. Switching entries drains/closes the previous writer before beginning the next; revisits append only after verifying the original private report inode, owner, permissions and link count. An unrelated or replaced file is never reopened as an owned session, and an unregistered identity is never written to a previously selected queue report. Only the active item owns a running writer/open report descriptor. A reporting failure is shown separately without rerouting facts or changing acknowledged playback/completion semantics. An original single-stream `--progress-file` keeps its destination when navigating back from appended queue entries.
|
Each report is immutably bound to its stable identity in the native decoder. Registration is atomic for a complete batch and can occur during playback without redirecting unrelated current reporting. Re-registering a removed identity drains/closes its old writer before replacing the session; the old report is never overwritten or appended to afterward, and the fresh report is created only when the re-added video begins. Switching entries drains/closes the previous writer before beginning the next; revisits within a session append only after verifying the original private report inode, owner, permissions and link count. An unrelated or replaced file is never reopened as an owned session, and an unregistered identity is never written to a previously selected queue report. Only the active item owns a running writer/open report descriptor. Retiring a failed writer preserves its reporting error. Reporting failures remain separate from acknowledged playback/completion semantics. An original single-stream `--progress-file` keeps its destination when navigating back from appended queue entries, unless its row was removed and re-added with a fresh destination.
|
||||||
|
|
||||||
Runnable headless queue checks: `make check-cli check-queue check-reporting check-resolved`. The existing `check-resolved` local HTTP/HLS/VTT harness drives a real native `PlayerModel` through mixed YouTube/Miruro queues with injected fixture resolution, stale-result cancellation, manual navigation, natural-end advancement, fresh revisits/resume and isolated v1/v2 reports. It also delivers repeated enqueue URLs while paused and playing, checks original standalone selection/timeline preservation, lazy append ordering, atomic invalid/conflicting requests, independent single/queue report revisits and empty-player startup. These checks require no browser or live streaming service. Actual LaunchServices/UI delivery remains a separate user-owned verification step; no GUI smoke result is claimed for enqueue.
|
Runnable headless queue checks: `make check-cli check-queue check-reporting check-resolved`. The existing `check-resolved` local HTTP/HLS/VTT harness drives a real native `PlayerModel` through mixed YouTube/Miruro queues with injected fixture resolution, stale-result cancellation, manual navigation, natural-end advancement, fresh revisits/resume and isolated v1/v2 reports. It also delivers repeated enqueue URLs while paused and playing, checks original standalone selection/timeline preservation, lazy append ordering, atomic invalid/conflicting requests, independent single/queue report revisits, removal/re-addition with fresh reports and unchanged retired files, and empty-player startup. These checks require no browser or live streaming service. Actual LaunchServices/UI delivery remains a separate user-owned verification step; no GUI smoke result is claimed for enqueue.
|
||||||
|
|
||||||
Enqueue verification: `check-cli`, `check-queue`, `check-reporting` and `make all` passed. An added reporting regression failed before protecting the original single-video identity from rebinding and passed afterward; unknown identities also cannot leak into that report after queue registration. The full `check-resolved` run stopped in the older mixed-queue natural-end scenario with `Could not start Core Audio (560227702)`, before enqueue ran. A focused `.build/resolved-playback-check /absolute/source.json --enqueue-only` smoke using local video-only HLS passed paused/playing append, unchanged current revision/selection, lazy ordering, natural-end advancement, navigation/revisits, atomic conflicts/rollback, isolated reports and empty-player startup. The source document still requires `headers` and `subtitles` (empty values are valid). No full A/V-suite or packaged-GUI/LaunchServices pass is claimed; the app was built, not installed. Actual UI confirmation remains user-owned; temporary fixture/server files were removed. Tubular's handoff document records its terminal/capture and Rust checks separately.
|
Removal/re-addition verification: the new native registration regression failed before the fix and passed afterward. `make check-reporting check-queue .build/resolved-playback-check all` passed. A focused `.build/resolved-playback-check /absolute/source.json --enqueue-only` smoke using local video-only H.264 HLS passed paused/playing append, natural-end advancement, atomic conflicts/rollback, removal and re-addition of the current standalone video and an inactive queued video, fresh-session playback, byte-for-byte retired-report preservation, and empty-player startup. The source document requires `headers` and `subtitles` (empty values are valid). No full A/V-suite or packaged-GUI/LaunchServices pass is claimed; the app was built, not installed. Actual UI confirmation remains user-owned; temporary fixture/server files were removed.
|
||||||
|
|
||||||
### Optional native metadata
|
### Optional native metadata
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user