Skip to content

Commit 6c08575

Browse files
committed
dash: fix crash-safety, catalog initData, late-track, and shutdown races
Resolve the confirmed findings from the PR #15 review of the CTE LL-DASH ingest publisher: - Guard the ingest worker thread so a malformed fragment (e.g. a moof referencing an unknown track) is dropped instead of escaping the thread and calling std::terminate. - Embed each track's base64 CMAF init segment in the catalog via a reusable track_init_data_base64 helper so subscribers can initialize decoders. - Freeze the announced track set when source() is taken; paths whose init segments arrive afterwards are ignored rather than enqueued against an unknown track (which aborted the whole publish session). - Add a bounded poll plus an is_finished predicate on LiveObjectSource so the publisher services control messages during media gaps and ends only on close, instead of blocking indefinitely. - Give each catalog emission a monotonic group id so a re-published catalog no longer collides with an already-delivered object id. - Erase a client fd from the active set before closing it, and defer closing the listen fd until after the accept thread joins, to close two fd-reuse races on shutdown. - Reject trailing garbage in --dash-listen/--dash-queue-depth with clear messages and require --dash-path/--dash-queue-depth to accompany --live-source dash. Add regression tests for crash-safety, catalog initData, late-path rejection, and the CLI validation.
1 parent 3456cfb commit 6c08575

9 files changed

Lines changed: 287 additions & 20 deletions

File tree

include/openmoq/publisher/cmsf_packager.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,12 @@ LiveCatalog build_live_catalog(const std::vector<TrackDescription>& tracks,
6060
std::span<const std::uint8_t> init_segment,
6161
bool is_live = true);
6262

63+
// Base64-encoded, track-specific CMAF init segment (ftyp + single-track moov)
64+
// for the track at track_index within init_segment. This is exactly the value
65+
// build_live_catalog embeds in each track's "initData" field, exposed so live
66+
// sources that assemble catalogs from per-path init segments can reuse it.
67+
std::string track_init_data_base64(std::span<const std::uint8_t> init_segment,
68+
const TrackDescription& track,
69+
std::size_t track_index);
70+
6371
} // namespace openmoq::publisher

include/openmoq/publisher/live_dash_ingest.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ class LiveDashIngestSession {
6969
LiveObjectSource source();
7070
std::optional<LiveObject> try_next_object();
7171

72+
bool finished() const;
73+
7274
private:
7375
struct PathState {
7476
StreamingMp4Reader reader;
@@ -79,18 +81,32 @@ class LiveDashIngestSession {
7981
bool initialized = false;
8082
};
8183

84+
// A track announced to subscribers, paired with the base64 CMAF init
85+
// segment carried in the catalog so subscribers can initialize decoders.
86+
struct RegisteredTrack {
87+
TrackDescription description;
88+
std::string init_data_base64;
89+
};
90+
8291
std::optional<LiveObject> next_object_blocking();
8392
void process_box_locked(PathState& path_state,
8493
std::string_view path,
8594
const StreamingBoxResult& box);
8695
void enqueue_locked(LiveObject object);
96+
bool track_published_locked(std::string_view track_name) const;
8797
std::vector<LiveTrack> snapshot_tracks_locked() const;
88-
LiveObject build_catalog_locked() const;
98+
LiveObject build_catalog_locked();
8999

90100
std::size_t queue_depth_ = 0;
91101
std::map<std::string, PathState> paths_;
92-
std::vector<TrackDescription> tracks_;
102+
std::vector<RegisteredTrack> tracks_;
93103
std::deque<LiveObject> queue_;
104+
// Once source() hands the track snapshot to the publisher, the announced
105+
// track set is frozen: paths that appear later cannot be added (the
106+
// publisher's alias table is fixed), so their media is dropped rather than
107+
// aborting the whole session on an unknown track.
108+
bool tracks_frozen_ = false;
109+
std::size_t catalog_group_id_ = 0;
94110
bool catalog_dirty_ = false;
95111
bool closed_ = false;
96112
mutable std::mutex mutex_;

include/openmoq/publisher/live_object.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ struct LiveObject {
2727
struct LiveObjectSource {
2828
std::vector<LiveTrack> tracks;
2929
std::function<std::optional<LiveObject>()> next_object;
30+
// Optional liveness predicate. When set, a nullopt from next_object() is
31+
// treated as a transient gap (the publisher keeps polling and servicing
32+
// control messages) as long as this returns false; it means end-of-stream
33+
// only once this returns true. When unset, a nullopt means end-of-stream,
34+
// preserving the behavior of finite sources.
35+
std::function<bool()> is_finished;
3036
};
3137

3238
} // namespace openmoq::publisher

src/cli_options.cpp

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,21 +123,41 @@ LiveSourceKind parse_live_source(std::string_view value) {
123123
throw std::runtime_error("unsupported --live-source value: expected auto, stdin, srt, or dash");
124124
}
125125

126+
int parse_strict_int(std::string_view value, std::string_view option_name) {
127+
// Reject empty, non-numeric, or trailing-garbage input with a clear,
128+
// option-specific message rather than a bare std::stoi("stoi") exception,
129+
// and so "80abc" is not silently accepted as 80.
130+
if (value.empty()) {
131+
throw std::runtime_error(std::string(option_name) + " requires a numeric value");
132+
}
133+
std::size_t consumed = 0;
134+
int parsed = 0;
135+
try {
136+
parsed = std::stoi(std::string(value), &consumed);
137+
} catch (const std::exception&) {
138+
throw std::runtime_error(std::string(option_name) + " must be a valid integer");
139+
}
140+
if (consumed != value.size()) {
141+
throw std::runtime_error(std::string(option_name) + " must be a valid integer");
142+
}
143+
return parsed;
144+
}
145+
126146
std::pair<std::string, std::uint16_t> parse_host_port(std::string_view value, std::string_view option_name) {
127147
const std::size_t colon = value.rfind(':');
128148
if (colon == std::string_view::npos || colon == 0 || colon + 1 >= value.size()) {
129149
throw std::runtime_error(std::string(option_name) + " must be in host:port form");
130150
}
131151
const std::string host(value.substr(0, colon));
132-
const int port = std::stoi(std::string(value.substr(colon + 1)));
152+
const int port = parse_strict_int(value.substr(colon + 1), std::string(option_name) + " port");
133153
if (port <= 0 || port > 65535) {
134154
throw std::runtime_error(std::string(option_name) + " port must be between 1 and 65535");
135155
}
136156
return {host, static_cast<std::uint16_t>(port)};
137157
}
138158

139159
std::size_t parse_queue_depth(std::string_view value) {
140-
const int depth = std::stoi(std::string(value));
160+
const int depth = parse_strict_int(value, "--dash-queue-depth");
141161
if (depth <= 0) {
142162
throw std::runtime_error("--dash-queue-depth must be greater than zero");
143163
}
@@ -148,6 +168,8 @@ std::size_t parse_queue_depth(std::string_view value) {
148168

149169
CliOptions parse_cli_options(int argc, char** argv) {
150170
CliOptions options;
171+
bool dash_path_set = false;
172+
bool dash_queue_depth_set = false;
151173

152174
for (int index = 1; index < argc; ++index) {
153175
const std::string_view argument = argv[index];
@@ -174,8 +196,10 @@ CliOptions parse_cli_options(int argc, char** argv) {
174196
options.dash_listen_port = port;
175197
} else if (argument == "--dash-path") {
176198
options.dash_path_prefix = std::string(require_value("--dash-path"));
199+
dash_path_set = true;
177200
} else if (argument == "--dash-queue-depth") {
178201
options.dash_queue_depth = parse_queue_depth(require_value("--dash-queue-depth"));
202+
dash_queue_depth_set = true;
179203
} else if (argument == "--transport") {
180204
options.transport = parse_transport_kind(require_value("--transport"));
181205
} else if (argument == "--endpoint") {
@@ -261,6 +285,12 @@ CliOptions parse_cli_options(int argc, char** argv) {
261285
if (!live_source_uses_dash && options.dash_listen.has_value()) {
262286
throw std::runtime_error("--dash-listen requires --live-source dash");
263287
}
288+
if (!live_source_uses_dash && dash_path_set) {
289+
throw std::runtime_error("--dash-path requires --live-source dash");
290+
}
291+
if (!live_source_uses_dash && dash_queue_depth_set) {
292+
throw std::runtime_error("--dash-queue-depth requires --live-source dash");
293+
}
264294
if (!options.dash_path_prefix.empty() && options.dash_path_prefix.front() != '/') {
265295
throw std::runtime_error("--dash-path must start with /");
266296
}

src/cmsf_packager.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,4 +795,10 @@ LiveCatalog build_live_catalog(const std::vector<TrackDescription>& tracks,
795795
return result;
796796
}
797797

798+
std::string track_init_data_base64(std::span<const std::uint8_t> init_segment,
799+
const TrackDescription& track,
800+
std::size_t track_index) {
801+
return base64_encode(build_track_specific_init_segment(init_segment, track, track_index));
802+
}
803+
798804
} // namespace openmoq::publisher

0 commit comments

Comments
 (0)