From 99a96fbdc367f1388e2645d4360ad452e387f7cd Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sat, 5 Sep 2026 18:12:25 +0200 Subject: [PATCH 01/19] mbstf: pin rt-libflute and rt-common-shared to the revisions this branch needs Problem This branch's FLUTE-layer obligations depend on work that is on rt-libflute's own feature branch and not in any release tag: the TS 26.346 annex L.6 profiled FDT schema, the scheme-specific FEC OTI, suppression of Transfer-Length under the MBMS Download Profile, the split expiry setter, and the RFC 5053 Raptor scheme. The wrap pointed at a revision carrying none of it. [code-derived] Basis No clause governs a dependency pin. code-derived only. Raised by Building this branch against the dependency it actually needs. Change Advances subprojects/rt-libflute.wrap to the commit on 5G-MAG's own feature/raptor-raptorq-fec carrying that work, and the rt-common-shared submodule to its consolidated tip. The wrap comment records why a tag cannot be used yet and what has to happen before it can be. Verification T0: the subproject is fetched and the tree builds against it. The behaviour that depends on these revisions is verified by the commits that use it, not here. Not in this change Moving the rt-libflute pin to a tag, which needs a 5G-MAG release carrying the work the comment names. No source change. --- subprojects/rt-common-shared | 2 +- subprojects/rt-libflute.wrap | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/subprojects/rt-common-shared b/subprojects/rt-common-shared index 5ed1458..f83f1bd 160000 --- a/subprojects/rt-common-shared +++ b/subprojects/rt-common-shared @@ -1 +1 @@ -Subproject commit 5ed1458f128e9bef1a2f654dabf5f19e275b2d9a +Subproject commit f83f1bdedf6f16275011760ce98958ce928301f5 diff --git a/subprojects/rt-libflute.wrap b/subprojects/rt-libflute.wrap index 8e5bf60..0efc9d2 100644 --- a/subprojects/rt-libflute.wrap +++ b/subprojects/rt-libflute.wrap @@ -1,6 +1,18 @@ +# Pinned to a commit on 5G-MAG's own feature/raptor-raptorq-fec (pull request #61, which is based on +# #62). That branch carries the MBS-compliance work this repository's FLUTE-layer obligations depend +# on: the TS 26.346 annex L.6 profiled FDT schema, the scheme-specific FEC OTI, suppression of +# Transfer-Length under the MBMS Download Profile, the split expiry setter, and the RFC 5053 Raptor +# scheme. None of it is in a release tag yet. +# +# Repoint this at a tag once #62 and #61 have merged and a release carries them. +# +# The revision below is a real, fetchable commit on 5G-MAG. Nothing here is patched locally: if +# subprojects/rt-libflute exists as a checkout it must be detached at exactly this commit, because +# meson's subproject directory precedence means an existing checkout silently wins over this file. +# `git -C subprojects/rt-libflute rev-parse HEAD` should print the revision below and nothing else. [wrap-git] url = https://github.com/5G-MAG/rt-libflute.git -revision = rt-libflute-0.12.3 +revision = 45ac74c14e02d58a4c9cef96f28d99fc598ac06d method = cmake #diff_files = rt-libflute/IpSec-xfrm_algo.patch From a0192ad0dfef0a5d2dd7ee02757bb117046110f2 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sat, 5 Sep 2026 18:12:31 +0200 Subject: [PATCH 02/19] mbstf: answer the SBI status codes TS 29.500 makes mandatory Problem The Nmbstf_DistSession surface answered a wrong Content-Type with 400 rather than 415, never checked a client's Accept header, never bounded a request body, and one PATCH handler parsed its body with no Content-Type check at all. [code-derived] Basis TS 29.500 V18.10.0 table 5.2.7.1-1 marks 415 mandatory for POST and PATCH, 406 mandatory for GET, and 413 mandatory where a body is accepted. Its table 5.2.7.2-1 defines no named cause for 415, so the numeric status is constructed directly, as this file already does for 405 and 501. TS 29.581 (TS29581_Nmbstf_DistSession.yaml) requires application/json-patch+json on both PATCH operations, not application/merge-patch+json. The individual resource's representation is DistSession for both the GET and the PATCH on /dist-sessions/{distSessionRef}; CreateReqData is the request body of the collection POST only, so an RFC 6902 pointer addresses the DistSession. Raised by reading the authority during this work, and observation of a live activation Change Adds NfServer::acceptsMediaType() and answers 406 when the client's Accept header cannot take application/json; answers 415 for an unexpected Content-Type on POST and PATCH; adds request_too_large() and a configurable maxRequestBodySize, answering 413; checks the Content-Type on the subscription PATCH. Applies a JSON Patch to the DistSession the stored CreateReqData holds, rebuilding the CreateReqData around the result, so the pointer a conformant peer sends resolves. Verification T2: the end-to-end demo activates a Distribution Session, which the previous patch target rejected outright. Not in this change Authentication on this surface. --- src/mbstf/Context.hh | 4 ++ src/mbstf/DistributionSession.cc | 116 ++++++++++++++++++++++++++++--- src/mbstf/DistributionSession.hh | 7 ++ src/mbstf/NfServer.cc | 36 ++++++++++ src/mbstf/NfServer.hh | 12 +++- src/mbstf/Open5GSSBIMessage.hh | 1 + src/mbstf/Open5GSSBIRequest.hh | 1 + 7 files changed, 167 insertions(+), 10 deletions(-) diff --git a/src/mbstf/Context.hh b/src/mbstf/Context.hh index 3dcde2a..a6229ab 100644 --- a/src/mbstf/Context.hh +++ b/src/mbstf/Context.hh @@ -82,6 +82,10 @@ public: */ std::optional manifestRepetitionRate = std::nullopt; } manifestGlobals; //< ManifestHandler global configuration (can be overridden by ManifestHandler implement specific config) + // TS 29.500 V18.10.0 cl.5.2.7.2/table 5.2.7.1-1: 413 (Payload Too Large) is mandatory for + // PATCH and POST. No clause, and no MBSTF documented default, names a byte limit (rule 12) + // -- unset means no limit is enforced, as before this option existed. + std::optional maxRequestBodySize; /** Parse a configuration time duration string * diff --git a/src/mbstf/DistributionSession.cc b/src/mbstf/DistributionSession.cc index c75286e..f55ccce 100644 --- a/src/mbstf/DistributionSession.cc +++ b/src/mbstf/DistributionSession.cc @@ -90,6 +90,7 @@ using reftools::mbstf::StatusSubscribeReqData; using reftools::mbstf::StatusSubscribeRspData; using reftools::mbstf::TunnelAddress; using reftools::mbstf::UpTrafficFlowInfo; +using reftools::mbstf::FECConfig; MBSTF_NAMESPACE_START @@ -107,6 +108,9 @@ static void send_model_params_error(const ModelParamsException &err, Open5GSSBIS const std::optional &api, const std::string &no_cause_reason, const std::string &log_prefix); static void _validate(const std::shared_ptr &dist_session); +static bool request_too_large(Open5GSSBIRequest &request, Open5GSSBIStream &stream, int path_segments, + Open5GSSBIMessage &message, const NfServer::AppMetadata &app_meta, + const std::optional &api); /**** public: ****/ @@ -428,6 +432,26 @@ bool DistributionSession::processEvent(Open5GSEvent &event) dist_event.releaseEventData(); return true; } + case LocalEvents::SUBSCRIPTION_EXPIRED: + { + /* Pushed by a subscription's expiry timer (SubscriptionExpiryTimerFunc in + * DistributionSessionSubscription.cc): its own expiryTime has passed, so remove it. + * Handled here, off the timer's call stack, so the subscription and the timer that + * fired can both be destroyed safely as part of the removal. */ + std::unique_ptr > expired( + reinterpret_cast*>(event.sbiData())); + const auto &dist_session = App::self().context()->findDistributionSession(expired->first); + if (dist_session) { + try { + dist_session->removeSubscription(expired->second); + ogs_debug("Removed expired subscription %s from Distribution Session %s", + expired->second.c_str(), expired->first.c_str()); + } catch (std::range_error &ex) { + /* already gone, e.g. deleted through the API before the timer fired */ + } + } + return true; + } default: break; } @@ -559,6 +583,13 @@ std::optional DistributionSession::getMbr() const return std::nullopt; } +std::optional> DistributionSession::getFecInformation() const +{ + std::shared_ptr create_req_data = distributionSessionReqData(); + std::shared_ptr dist_session = create_req_data->getDistSession(); + return dist_session->getFecInformation(); +} + const std::optional &DistributionSession::getObjectIngestBaseUrl() const { std::shared_ptr create_req_data = distributionSessionReqData(); @@ -982,10 +1013,15 @@ void DistributionSession::_apiSessionCreate(Open5GSSBIStream &stream, Open5GSSBI { /* static method */ if (request.headerValue(OGS_SBI_CONTENT_TYPE, std::string()) != "application/json") { - ogs_assert(true == NfServer::sendError(stream, ProblemCause::INVALID_MSG_FORMAT, 1, message, app_meta, api, + /* TS 29.500 V18.10.0 table 5.2.7.1-1 marks 415 mandatory for POST. Its table 5.2.7.2-1 + defines no named cause for 415, so the numeric status is constructed directly here, + the same pattern this file already uses for 405 and 501; ProblemCause::INVALID_MSG_FORMAT + would answer 400, which is a different condition. */ + ogs_assert(true == NfServer::sendError(stream, OGS_SBI_HTTP_STATUS_UNSUPPORTED_MEDIA_TYPE, 1, message, app_meta, api, "Unsupported Media Type", "Expected content type: application/json")); return; } + if (request_too_large(request, stream, 1, message, app_meta, api)) return; CJson distSession(CJson::Null); try { @@ -1118,13 +1154,17 @@ void DistributionSession::_apiSessionPatch(Open5GSSBIStream &stream, Open5GSSBIM { std::string content_type(message.contentType()); if (content_type != OGS_SBI_CONTENT_PATCH_TYPE) { + /* TS 29.500 V18.10.0 table 5.2.7.1-1 marks 415 mandatory for PATCH. This resource's + expected type is the merge-patch type, not the plain application/json the POST handlers + check, so the comparison differs from theirs while the status does not. */ std::ostringstream err; err << "Content-Type [" << message.contentType() << "] unknown for PATCH method, expecting " OGS_SBI_CONTENT_PATCH_TYPE; ogs_error("%s", err.str().c_str()); - ogs_assert(true == NfServer::sendError(stream, ProblemCause::INVALID_MSG_FORMAT, 2, message, app_meta, api, - "MBSTF Distribution Session patch bad MIME type", err.str())); + ogs_assert(true == NfServer::sendError(stream, OGS_SBI_HTTP_STATUS_UNSUPPORTED_MEDIA_TYPE, 2, message, app_meta, api, + "Unsupported Media Type", err.str())); return; } + if (request_too_large(request, stream, 2, message, app_meta, api)) return; /* parse body */ CJson patch_json(CJson::newNull()); @@ -1136,11 +1176,21 @@ void DistributionSession::_apiSessionPatch(Open5GSSBIStream &stream, Open5GSSBIM return; } - /* Apply patch */ - auto old_dist_sess = distributionSessionReqData(); + /* Apply patch. + + The JSON Pointers in the patch address this resource's own representation, which + TS 29.581 V18.6.0 (TS29581_Nmbstf_DistSession.yaml) gives as DistSession for both the + PATCH and the GET on /dist-sessions/{distSessionRef}; CreateReqData is the request body + of the collection POST only. So a peer sends "/distSessionState", not + "/distSession/distSessionState", and the patch is applied to the DistSession that + CreateReqData holds rather than to CreateReqData itself. The enclosing CreateReqData is + then rebuilt around the patched DistSession, since that is what this object stores. */ + auto old_req_data = distributionSessionReqData(); std::shared_ptr new_dist_sess{}; try { - new_dist_sess.reset(old_dist_sess->newWithJSONPatches(patch_json)); + std::shared_ptr patched_sess(old_req_data->getDistSession()->newWithJSONPatches(patch_json)); + new_dist_sess.reset(new reftools::mbstf::CreateReqData(*old_req_data)); + new_dist_sess->setDistSession(patched_sess); } catch (ModelException &err) { send_model_error(err, stream, 2, message, app_meta, api, "Unable to apply JSON Patch", "MBSTF Distribution Session patch failed to apply"); @@ -1175,8 +1225,19 @@ void DistributionSession::_apiSessionGet(Open5GSSBIStream &stream, Open5GSSBIMes const std::optional &api, const NfServer::AppMetadata &app_meta) { - CJson createdRspData_json(json(false)); - std::string body(createdRspData_json.serialise()); + /* TS 29.500 V18.10.0 table 5.2.7.1-1 marks 406 mandatory for GET. This response is always + application/json, so a client whose Accept header cannot take that is answered 406 rather + than sent a body it did not ask for. */ + std::optional accept_hdr; + if (message.accept()) accept_hdr = message.accept(); + if (!NfServer::acceptsMediaType(accept_hdr, "application/json")) { + ogs_assert(true == NfServer::sendError(stream, OGS_SBI_HTTP_STATUS_NOT_ACCEPTABLE, 1, message, app_meta, api, + "Not Acceptable", "This resource is only available as application/json")); + return; + } + + CJson dist_session_json(json()); + std::string body(dist_session_json.serialise()); ogs_debug("Generated JSON: %s", body.c_str()); std::optional content_type; if (!body.empty()) { @@ -1195,10 +1256,15 @@ void DistributionSession::_apiSubscriptionCreate(Open5GSSBIStream &stream, Open5 const NfServer::AppMetadata &app_meta) { if (request.headerValue(OGS_SBI_CONTENT_TYPE, std::string()) != "application/json") { - ogs_assert(true == NfServer::sendError(stream, ProblemCause::INVALID_MSG_FORMAT, 1, message, app_meta, api, + /* TS 29.500 V18.10.0 table 5.2.7.1-1 marks 415 mandatory for POST. Its table 5.2.7.2-1 + defines no named cause for 415, so the numeric status is constructed directly here, + the same pattern this file already uses for 405 and 501; ProblemCause::INVALID_MSG_FORMAT + would answer 400, which is a different condition. */ + ogs_assert(true == NfServer::sendError(stream, OGS_SBI_HTTP_STATUS_UNSUPPORTED_MEDIA_TYPE, 1, message, app_meta, api, "Unsupported Media Type", "Expected content type: application/json")); return; } + if (request_too_large(request, stream, 1, message, app_meta, api)) return; CJson dist_session_subsc_json(CJson::Null); std::string subsc_id; @@ -1266,6 +1332,21 @@ void DistributionSession::_apiSubscriptionPatch(const DistributionSessionSubscri const std::optional &api, const NfServer::AppMetadata &app_meta) { + /* TS 29.581 (TS29581_Nmbstf_DistSession.yaml, the + /dist-sessions/{distSessionRef}/subscriptions/{subscriptionId} PATCH operation) requires + application/json-patch+json for this resource. Both PATCH operations in that document + request this type, not application/merge-patch+json. */ + std::string content_type(message.contentType()); + if (content_type != OGS_SBI_CONTENT_PATCH_TYPE) { + std::ostringstream err; + err << "Content-Type [" << content_type << "] unknown for PATCH method, expecting " OGS_SBI_CONTENT_PATCH_TYPE; + ogs_error("%s", err.str().c_str()); + ogs_assert(true == NfServer::sendError(stream, OGS_SBI_HTTP_STATUS_UNSUPPORTED_MEDIA_TYPE, 4, message, app_meta, api, + "Unsupported Media Type", err.str())); + return; + } + if (request_too_large(request, stream, 4, message, app_meta, api)) return; + CJson req_json(CJson::Null); try { req_json = CJson::parse(request.content()); @@ -1309,6 +1390,22 @@ static std::shared_ptr get_object_distribution_data(const D } +static bool request_too_large(Open5GSSBIRequest &request, Open5GSSBIStream &stream, int path_segments, + Open5GSSBIMessage &message, const NfServer::AppMetadata &app_meta, + const std::optional &api) +{ + const auto &max_size = App::self().context()->maxRequestBodySize; + if (!max_size || request.contentLength() <= *max_size) return false; + + std::ostringstream err; + err << "Request body of " << request.contentLength() << " bytes exceeds the configured maximum of " + << *max_size << " bytes"; + ogs_error("%s", err.str().c_str()); + ogs_assert(true == NfServer::sendError(stream, OGS_SBI_HTTP_STATUS_PAYLOAD_TOO_LARGE, path_segments, message, + app_meta, api, "Payload Too Large", err.str())); + return true; +} + static void send_model_error(const ModelException &err, Open5GSSBIStream &stream, int path_segments, Open5GSSBIMessage &message, const NfServer::AppMetadata &app_meta, const std::optional &api, const std::string &no_cause_reason, const std::string &log_prefix) @@ -1438,6 +1535,7 @@ static void _validate(const std::shared_ptr &dist_session) } } + MBSTF_NAMESPACE_STOP /* vim:ts=8:sts=4:sw=4:expandtab: diff --git a/src/mbstf/DistributionSession.hh b/src/mbstf/DistributionSession.hh index 1c8aafc..b290f1d 100644 --- a/src/mbstf/DistributionSession.hh +++ b/src/mbstf/DistributionSession.hh @@ -35,6 +35,10 @@ #include "DistributionSessionSubscription.hh" #include "NfServer.hh" +namespace reftools::mbstf { + class FECConfig; +} + namespace fiveg_mag_reftools { class CJson; } @@ -93,6 +97,9 @@ public: in_port_t getTunnelPortNumber() const; uint32_t getRateLimit() const; std::optional getMbr() const; + /** DistSession.fecInformation (TS 29.581 clause 6.1.6.2.5, TS 29.580 V18.8.0 clause 6.2.6.2.14 + * FECConfig), unset when the create request did not carry it. */ + std::optional> getFecInformation() const; const std::optional &getObjectIngestBaseUrl() const; const std::string &getObjectAcquisitionMethod() const; void setObjectIngestBaseUrl(std::string ingestBaseUrl); diff --git a/src/mbstf/NfServer.cc b/src/mbstf/NfServer.cc index 014b8b7..02447be 100644 --- a/src/mbstf/NfServer.cc +++ b/src/mbstf/NfServer.cc @@ -20,6 +20,8 @@ #include "ogs-sbi.h" #include +#include +#include #include #include #include @@ -365,6 +367,40 @@ static char *build_json(Open5GSSBIMessage &message) return content; } +std::string to_lower(std::string s) +{ + std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) { return std::tolower(c); }); + return s; +} + +std::string trim(const std::string &s) +{ + size_t start = s.find_first_not_of(" \t"); + if (start == std::string::npos) return std::string(); + size_t end = s.find_last_not_of(" \t"); + return s.substr(start, end - start + 1); +} + +bool NfServer::acceptsMediaType(const std::optional &accept_header, const std::string &media_type) +{ + if (!accept_header.has_value() || accept_header->empty()) return true; + + const std::string wanted = to_lower(media_type); + const std::string wanted_type = wanted.substr(0, wanted.find('/')); + + std::istringstream ranges(*accept_header); + std::string range; + while (std::getline(ranges, range, ',')) { + std::string media_range = trim(range.substr(0, range.find(';'))); + media_range = to_lower(media_range); + if (media_range == "*/*" || media_range == wanted || + media_range == wanted_type + "/*") { + return true; + } + } + return false; +} + MBSTF_NAMESPACE_STOP /* vim:ts=8:sts=4:sw=4:expandtab: diff --git a/src/mbstf/NfServer.hh b/src/mbstf/NfServer.hh index 7d8dacc..2a227d0 100644 --- a/src/mbstf/NfServer.hh +++ b/src/mbstf/NfServer.hh @@ -59,7 +59,6 @@ public: const std::string &apiTitle() const { return m_apiTitle; }; const std::string &apiVersion() const { return m_apiVersion; }; - private: std::string m_apiTitle; std::string m_apiVersion; @@ -128,6 +127,17 @@ public: static std::map makeInvalidParams(const std::string ¶m, const std::string &reason); + + + // TS 29.500 V18.10.0 table 5.2.7.1-1 marks HTTP 406 mandatory for GET, generically across the 5GC + // SBI APIs (table 5.2.7.2-1 defines no named cause for it). RFC 9110 s12.5.1: "A request without + // any Accept header field implies that the user agent will accept any media type in response" -- + // only present-and-incompatible Accept values make a response unacceptable. This checks whether + // media_type (the single, fixed content type this NF is about to serve -- it never negotiates among + // several) is compatible with one of accept_header's comma-separated media ranges, ignoring any + // ";q=..."/other parameters (this NF has only one representation to offer, so relative preference + // never changes the outcome, only presence/absence of a compatible range does). + static bool acceptsMediaType(const std::optional &accept_header, const std::string &media_type); private: static bool __sendError(Open5GSSBIStream &stream, int status, const std::optional &cause, size_t number_of_components, diff --git a/src/mbstf/Open5GSSBIMessage.hh b/src/mbstf/Open5GSSBIMessage.hh index 2e09eeb..475e92a 100644 --- a/src/mbstf/Open5GSSBIMessage.hh +++ b/src/mbstf/Open5GSSBIMessage.hh @@ -61,6 +61,7 @@ public: const OpenAPI_nf_profile_t *nfProfile() const { return m_message?(m_message->NFProfile):nullptr; }; int resStatus() const { return m_message?(m_message->res_status):0; }; const char *contentType() const { return m_message?(m_message->http.content_type):nullptr; }; + const char *accept() const { return m_message?(m_message->http.accept):nullptr; }; OpenAPI_problem_details_t *problemDetails() { return m_message?(m_message->ProblemDetails):nullptr; }; const OpenAPI_problem_details_t *problemDetails() const { return m_message?(m_message->ProblemDetails):nullptr; }; diff --git a/src/mbstf/Open5GSSBIRequest.hh b/src/mbstf/Open5GSSBIRequest.hh index c8014e4..82b1d52 100644 --- a/src/mbstf/Open5GSSBIRequest.hh +++ b/src/mbstf/Open5GSSBIRequest.hh @@ -57,6 +57,7 @@ public: void parametersMap(const ParametersMap &map) const; const char *content() const { return m_request?m_request->http.content:nullptr; }; + size_t contentLength() const { return m_request?m_request->http.content_length:0; }; const char *uri() const { return m_request?m_request->h.uri:nullptr; }; void setOwner(bool owner) { m_owner = owner; }; bool getOwner() const { return m_owner; }; From a0266a90ed2851fc9e94c381584b779f865233ac Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sat, 5 Sep 2026 18:12:31 +0200 Subject: [PATCH 03/19] mbstf: stop notifying a Distribution Session subscription past its expiry time Problem DistributionSessionSubscription parsed expiryTime into m_expiryTime and never compared it against the clock anywhere, so a subscription carrying one kept receiving notifications until its Distribution Session was deleted or the process exited. [code-derived] Basis TS 29.581 V18.6.0, table 6.1.6.2.5-1, expiryTime row: "When present in the subscription creation request, it shall indicate the time up to which the subscription is desired to be kept active and after which the subscribed events shall stop generating notifications." Raised by reading the authority during this work Change Adds a per-subscription timer that removes the subscription when its expiryTime passes, scheduled wherever m_expiryTime is set and cancelled from the destructor. The callback does not remove the subscription directly: it runs from a timer that subscription owns, so it pushes LocalEvents::SUBSCRIPTION_EXPIRED carrying the two ids as plain strings and DistributionSession::processEvent() performs the removal off that call stack, the deferred dispatch SEND_NOTIFICATION already uses. A copy schedules its own timer, a callback being keyed to one object; a move takes the original's over. Timer-pool exhaustion is logged and leaves the subscription without a timer rather than crashing. Verification T2: the end-to-end demo runs with it in place, MBSTF logging no fatal or assertion lines and no timer-creation failure. No test scaffolding exists for this surface. Not in this change An expiryTime in a subscription response, which this MBSTF does not set. --- src/mbstf/DistributionSessionSubscription.cc | 98 ++++++++++++++++++++ src/mbstf/DistributionSessionSubscription.hh | 12 +++ src/mbstf/LocalEvents.hh | 4 +- 3 files changed, 113 insertions(+), 1 deletion(-) diff --git a/src/mbstf/DistributionSessionSubscription.cc b/src/mbstf/DistributionSessionSubscription.cc index 43bf48a..2aa5453 100644 --- a/src/mbstf/DistributionSessionSubscription.cc +++ b/src/mbstf/DistributionSessionSubscription.cc @@ -18,6 +18,8 @@ */ #include +#include +#include #include #include #include @@ -30,6 +32,10 @@ #include "App.hh" #include "DistributionSession.hh" #include "DistributionSessionNotificationEvent.hh" +#include "LocalEvents.hh" +#include "Open5GSNetworkFunction.hh" +#include "Open5GSTimer.hh" +#include "TimerFunc.hh" #include "openapi/model/DistSessionSubscription.h" #include "openapi/model/DistSessionEventReport.h" #include "openapi/model/DistSessionEventReportList.h" @@ -48,6 +54,48 @@ using fiveg_mag_reftools::ModelException; MBSTF_NAMESPACE_START +namespace { + +/* Fires once a DistributionSessionSubscription's expiryTime passes. Carries only plain string ids, + * not a pointer or reference to the DistributionSession or the subscription, so it depends on + * neither object's lifetime. + * + * trigger() does not call DistributionSession::removeSubscription() directly: this callback runs + * from the timer owned by the very DistributionSessionSubscription that would be erased, so + * removing it here would destroy this TimerFunc, and the Open5GSTimer executing it, while trigger() + * is still on the stack. It pushes a LocalEvents::SUBSCRIPTION_EXPIRED event instead and lets + * DistributionSession::processEvent() perform the removal off this call stack, the same deferred + * dispatch LocalEvents::SEND_NOTIFICATION and RELEASE_SUBSCRIPTION_SVC already use. */ +class SubscriptionExpiryTimerFunc : public TimerFunc { +public: + SubscriptionExpiryTimerFunc(const std::string &dist_session_id, const std::string &subscription_id) + :TimerFunc(), distSessionId(dist_session_id), subscriptionId(subscription_id) {}; + SubscriptionExpiryTimerFunc(SubscriptionExpiryTimerFunc &&) = delete; + SubscriptionExpiryTimerFunc(const SubscriptionExpiryTimerFunc &) = delete; + SubscriptionExpiryTimerFunc &operator=(SubscriptionExpiryTimerFunc &&) = delete; + SubscriptionExpiryTimerFunc &operator=(const SubscriptionExpiryTimerFunc &) = delete; + virtual ~SubscriptionExpiryTimerFunc() {}; + + virtual void trigger() + { + ogs_debug("Subscription %s expiry timer fired", subscriptionId.c_str()); + std::shared_ptr event(new Open5GSEvent(new ogs_event_t)); + event->ogsEvent()->id = LocalEvents::SUBSCRIPTION_EXPIRED; + event->setSbiData(new std::pair(distSessionId, subscriptionId)); + try { + App::self().ogsApp()->pushEvent(event); + } catch (std::exception &ex) { + ogs_error("Failed to push SUBSCRIPTION_EXPIRED event for subscription %s: %s", + subscriptionId.c_str(), ex.what()); + } + } + + std::string distSessionId; + std::string subscriptionId; +}; + +} + static int __notify_client_cb(int status, ogs_sbi_response_t *response, void *data); namespace { @@ -71,6 +119,7 @@ DistributionSessionSubscription::DistributionSessionSubscription(const std::weak _setSubscriptionId(); _setEventFlags(); _setExpiryTime(); + _scheduleExpiryTimer(); } DistributionSessionSubscription::DistributionSessionSubscription(const std::weak_ptr &dist_session, @@ -85,6 +134,7 @@ DistributionSessionSubscription::DistributionSessionSubscription(const std::weak _setSubscriptionId(); _setEventFlags(); _setExpiryTime(); + _scheduleExpiryTimer(); } DistributionSessionSubscription::DistributionSessionSubscription(DistributionSessionSubscription &&other) @@ -92,6 +142,8 @@ DistributionSessionSubscription::DistributionSessionSubscription(DistributionSes ,m_subscriptionId(std::move(other.m_subscriptionId)) ,m_eventTypes(std::move(other.m_eventTypes)) ,m_distSessionSubscription(std::move(other.m_distSessionSubscription)) + ,m_expiryTimer(std::move(other.m_expiryTimer)) + ,m_expiryTimerFunc(std::move(other.m_expiryTimerFunc)) ,m_expiryTime(std::move(other.m_expiryTime)) ,m_cache(other.m_cache) { @@ -106,10 +158,14 @@ DistributionSessionSubscription::DistributionSessionSubscription(const Distribut ,m_expiryTime(other.m_expiryTime) ,m_cache(new DistributionSessionSubscription::CacheType(*other.m_cache)) { + /* A timer's callback is keyed to one subscription object, so the copy gets its own rather + than sharing other's. */ + _scheduleExpiryTimer(); } DistributionSessionSubscription::~DistributionSessionSubscription() { + _cancelExpiryTimer(); if (m_cache) { delete m_cache; m_cache = nullptr; @@ -119,11 +175,14 @@ DistributionSessionSubscription::~DistributionSessionSubscription() /* operators */ DistributionSessionSubscription &DistributionSessionSubscription::operator=(DistributionSessionSubscription &&other) { + _cancelExpiryTimer(); m_distributionSession = std::move(other.m_distributionSession); m_subscriptionId = std::move(other.m_subscriptionId); m_eventTypes = std::move(other.m_eventTypes); m_distSessionSubscription = std::move(other.m_distSessionSubscription); m_expiryTime = std::move(other.m_expiryTime); + m_expiryTimer = std::move(other.m_expiryTimer); + m_expiryTimerFunc = std::move(other.m_expiryTimerFunc); if (m_cache) delete m_cache; m_cache = other.m_cache; other.m_cache = nullptr; @@ -132,12 +191,15 @@ DistributionSessionSubscription &DistributionSessionSubscription::operator=(Dist DistributionSessionSubscription &DistributionSessionSubscription::operator=(const DistributionSessionSubscription &other) { + _cancelExpiryTimer(); m_distributionSession = other.m_distributionSession; m_subscriptionId = other.m_subscriptionId; m_eventTypes = other.m_eventTypes; m_distSessionSubscription = other.m_distSessionSubscription; m_expiryTime = other.m_expiryTime; *m_cache = *other.m_cache; + /* see the copy constructor: an independent timer, not a shared one */ + _scheduleExpiryTimer(); return *this; } @@ -184,6 +246,7 @@ DistributionSessionSubscription &DistributionSessionSubscription::update(CJson & } _setEventFlags(); _setExpiryTime(); + _scheduleExpiryTimer(); return *this; } @@ -337,6 +400,41 @@ void DistributionSessionSubscription::_setExpiryTime() } } +void DistributionSessionSubscription::_scheduleExpiryTimer() +{ + _cancelExpiryTimer(); + + if (!m_expiryTime) return; /* no expiryTime set, nothing to enforce */ + + std::shared_ptr dist_session(m_distributionSession.lock()); + if (!dist_session) return; /* no parent DistributionSession to key the timer to */ + + const auto now = std::chrono::system_clock::now(); + long long delay_ms = 0; + if (m_expiryTime.value() > now) { + delay_ms = std::chrono::duration_cast(m_expiryTime.value() - now).count(); + } + if (delay_ms > std::numeric_limits::max()) delay_ms = std::numeric_limits::max(); + + m_expiryTimerFunc.reset(new SubscriptionExpiryTimerFunc(dist_session->distributionSessionId(), m_subscriptionId)); + m_expiryTimer = App::self().ogsApp()->addTimer(*m_expiryTimerFunc); + if (m_expiryTimer) { + m_expiryTimer->start(static_cast(delay_ms)); + } else { + ogs_error("Failed to create expiry timer for subscription %s", m_subscriptionId.c_str()); + m_expiryTimerFunc.reset(); + } +} + +void DistributionSessionSubscription::_cancelExpiryTimer() +{ + if (m_expiryTimer) { + App::self().ogsApp()->removeTimer(m_expiryTimer); + m_expiryTimer.reset(); + } + m_expiryTimerFunc.reset(); +} + void DistributionSessionSubscription::_setSubscriptionId() { uuid_t uuid; diff --git a/src/mbstf/DistributionSessionSubscription.hh b/src/mbstf/DistributionSessionSubscription.hh index e233ad8..3edc046 100644 --- a/src/mbstf/DistributionSessionSubscription.hh +++ b/src/mbstf/DistributionSessionSubscription.hh @@ -42,6 +42,8 @@ MBSTF_NAMESPACE_START class DistributionSession; class Open5GSEvent; +class Open5GSTimer; +class TimerFunc; class DistributionSessionSubscription { public: @@ -95,6 +97,11 @@ public: private: void _setEventFlags(); void _setExpiryTime(); + /** (Re)schedule, or cancel when no expiryTime is set, the timer that removes this + * subscription once its expiryTime passes. Call after any change to m_expiryTime. */ + void _scheduleExpiryTimer(); + /** Cancel and release any currently scheduled expiry timer. */ + void _cancelExpiryTimer(); void _setSubscriptionId(); std::weak_ptr m_distributionSession; /* Parent distribution session */ @@ -103,6 +110,11 @@ private: int m_eventTypes; /* ORed EventTypeBitMask */ reftools::mbstf::DistSessionSubscription m_distSessionSubscription; std::optional m_expiryTime; + /* Timer that removes this subscription once m_expiryTime passes; absent when no expiryTime is + set. TS 29.581's DistSessionSubscription carries expiryTime, and without this the value is + parsed and stored but never acted on. */ + std::shared_ptr m_expiryTimer; + std::unique_ptr m_expiryTimerFunc; std::optional m_subscriptionLocation; struct CacheType { diff --git a/src/mbstf/LocalEvents.hh b/src/mbstf/LocalEvents.hh index 7fe5e15..318f9a4 100644 --- a/src/mbstf/LocalEvents.hh +++ b/src/mbstf/LocalEvents.hh @@ -29,13 +29,15 @@ class LocalEvents { public: typedef enum { SEND_NOTIFICATION = OGS_MAX_NUM_OF_PROTO_EVENT+1000, - RELEASE_SUBSCRIPTION_SVC + RELEASE_SUBSCRIPTION_SVC, + SUBSCRIPTION_EXPIRED } LocalEventIds; static const char *getEventName(Open5GSEvent &event) { if (event.id() < OGS_MAX_NUM_OF_PROTO_EVENT) return ogs_event_get_name(event.ogsEvent()); if (event.id() == SEND_NOTIFICATION) return "SEND_NOTIFICATION"; if (event.id() == RELEASE_SUBSCRIPTION_SVC) return "RELEASE_SUBSCRIPTION_SVC"; + if (event.id() == SUBSCRIPTION_EXPIRED) return "SUBSCRIPTION_EXPIRED"; return "Unknown event"; }; }; From 5fab69a1300f20ad9b48d0c04184f42b22f6a0db Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sat, 5 Sep 2026 18:13:34 +0200 Subject: [PATCH 04/19] mbstf: keep an object's availability window and entity tag, and send by earliest deadline Problem Four related defects in how object metadata is held and ordered. ObjectStore::Metadata hand-writes its copy constructor, move constructor and both assignment operators, and three members were missing from them. The availability window (m_availabilityStartTime, m_availabilityEndTime) was absent from all four paths, so any copy taken out of the store lost it: build/tests/testObjectStore aborted with std::bad_optional_access. [observed: test output] The entity tag (m_entityTag) was absent from all four as well, which is quieter and not caught by any test: PullObjectIngester passes it to the conditional re-fetch, so losing it turns an If-None-Match into an unconditional GET and re-downloads an unchanged segment, and ObjectCarouselPackager copies it into the FLUTE file description, which then carries an empty ETag. [code-derived] PullObjectIngester::IngestItem has the same shape of defect in one direction only: its copy constructor carries the availability window and its move constructor did not. Items are moved on the ordinary queueing paths, so an item reaching the fetch queue by any of those routes lost what the copy path preserved. [code-derived] Nothing ordered the packaging queue by when an object has to arrive. [code-derived] Basis TS 26.517 V18.6.0, clause 6.2.3.5: "The MBSTF shall transmit each object in the object list such that the last packet of the delivered FLUTE transmission object (including any FEC recovery packets, when configured) is available at the MBSTF Client no later than its availability start time." The deadline a PackageItem carries is that availability start time, so ordering the queue by it is what implements the clause. The copy and move defects are code-derived: no clause governs a class's own copy semantics, only that a value the class stores survives being copied. Raised by The availability window by running the test suite, which aborted. The entity tag and the ingest item's move constructor by reading the same four paths afterwards, looking for the same mistake again. Both times it was there. Change Adds the missing members to all four ObjectStore::Metadata copy and move paths, in declaration order so initialisation order matches and -Wreorder stays quiet, and to IngestItem's move constructor. Adds earlierDeadlineFirst(), a named predicate rather than an inline comparator so the ordering can be tested directly, the queue itself being private and fed only through a live packager. An item with no deadline sorts after every item that has one: nothing is known about when it must arrive, so it cannot displace an object that does have a stated time. Verification T1: four suites executed and passing -- testObjectStore 16 cases, testPullObjectIngester 19, testObjectListPackager 7, testSubscriberSubscription. Both metadata defects were confirmed to be caught, not merely covered: testObjectStore aborts without the availability fix, and reports "copy=[] move=[] copy-assign=[] move-assign=[]", 15 pass 1 fail, without the entity tag fix. Not in this change IngestItem's own copy and move constructors have no direct test, which is the gap that let the move-constructor defect survive. ObjectStore::Metadata::operator== still does not compare m_entityTag; the equality semantics of the class are not touched here. --- src/mbstf/ObjectListPackager.hh | 22 ++++ src/mbstf/ObjectStore.cc | 12 ++ src/mbstf/ObjectStore.hh | 34 +++++ src/mbstf/PullObjectIngester.cc | 8 +- src/mbstf/PullObjectIngester.hh | 13 +- tests/meson.build | 12 +- tests/test_ObjectListPackager.cc | 214 ++++++++++++++----------------- tests/test_ObjectStore.cc | 83 ++++++++++++ tests/test_PullObjectIngester.cc | 182 ++++++++++++++++++++------ 9 files changed, 420 insertions(+), 160 deletions(-) diff --git a/src/mbstf/ObjectListPackager.hh b/src/mbstf/ObjectListPackager.hh index 17437a1..79ff6c8 100644 --- a/src/mbstf/ObjectListPackager.hh +++ b/src/mbstf/ObjectListPackager.hh @@ -59,6 +59,28 @@ public: PackageItem &deadline(const time_type &deadline) { m_deadline = deadline; return *this; } PackageItem &deadline(time_type &&deadline) { m_deadline = std::move(deadline); return *this; } + + /** Transmission order for the packaging queue. + * + * TS 26.517 V18.6.0 clause 6.2.3.5: "The MBSTF shall transmit each object in the object + * list such that the last packet of the delivered FLUTE transmission object (including any + * FEC recovery packets, when configured) is available at the MBSTF Client no later than its + * availability start time." + * + * The deadline carried by a PackageItem is that availability start time, so ordering the + * queue by it is what implements the clause. An item with no deadline sorts after every + * item that has one: nothing is known about when it must arrive, so it cannot be allowed to + * displace an object that does have a stated time. + * + * A named predicate rather than an inline comparator so that the ordering can be tested + * directly, the queue itself being private and fed only through a live packager. + */ + static bool earlierDeadlineFirst(const PackageItem &a, const PackageItem &b) { + if (a.m_deadline.has_value() && b.m_deadline.has_value()) { + return a.m_deadline < b.m_deadline; + } + return a.m_deadline.has_value(); + }; private: std::shared_ptr m_object; std::optional m_deadline; diff --git a/src/mbstf/ObjectStore.cc b/src/mbstf/ObjectStore.cc index b276642..6977dcb 100644 --- a/src/mbstf/ObjectStore.cc +++ b/src/mbstf/ObjectStore.cc @@ -89,7 +89,10 @@ ObjectStore::Metadata::Metadata(const Metadata &other) ,m_compressedSend(other.m_compressedSend) ,m_objIngestBaseUrl(other.m_objIngestBaseUrl) ,m_objDistributionBaseUrl(other.m_objDistributionBaseUrl) + ,m_entityTag(other.m_entityTag) ,m_cacheExpires(other.m_cacheExpires) + ,m_availabilityStartTime(other.m_availabilityStartTime) + ,m_availabilityEndTime(other.m_availabilityEndTime) ,m_receivedTime(other.m_receivedTime) ,m_created(other.m_created) ,m_modified(other.m_modified) @@ -108,7 +111,10 @@ ObjectStore::Metadata::Metadata(Metadata &&other) ,m_compressedSend(other.m_compressedSend) ,m_objIngestBaseUrl(std::move(other.m_objIngestBaseUrl)) ,m_objDistributionBaseUrl(std::move(other.m_objDistributionBaseUrl)) + ,m_entityTag(std::move(other.m_entityTag)) ,m_cacheExpires(std::move(other.m_cacheExpires)) + ,m_availabilityStartTime(std::move(other.m_availabilityStartTime)) + ,m_availabilityEndTime(std::move(other.m_availabilityEndTime)) ,m_receivedTime(std::move(other.m_receivedTime)) ,m_created(std::move(other.m_created)) ,m_modified(std::move(other.m_modified)) @@ -128,7 +134,10 @@ ObjectStore::Metadata &ObjectStore::Metadata::operator=(const ObjectStore::Metad m_compressedSend = other.m_compressedSend; m_objIngestBaseUrl = other.m_objIngestBaseUrl; m_objDistributionBaseUrl = other.m_objDistributionBaseUrl; + m_entityTag = other.m_entityTag; m_cacheExpires = other.m_cacheExpires; + m_availabilityStartTime = other.m_availabilityStartTime; + m_availabilityEndTime = other.m_availabilityEndTime; m_receivedTime = other.m_receivedTime; m_created = other.m_created; m_modified = other.m_modified; @@ -149,7 +158,10 @@ ObjectStore::Metadata &ObjectStore::Metadata::operator=(ObjectStore::Metadata && m_compressedSend = other.m_compressedSend; m_objIngestBaseUrl = std::move(other.m_objIngestBaseUrl); m_objDistributionBaseUrl = std::move(other.m_objDistributionBaseUrl); + m_entityTag = std::move(other.m_entityTag); m_cacheExpires = std::move(other.m_cacheExpires); + m_availabilityStartTime = std::move(other.m_availabilityStartTime); + m_availabilityEndTime = std::move(other.m_availabilityEndTime); m_receivedTime = std::move(other.m_receivedTime); m_created = std::move(other.m_created); m_modified = std::move(other.m_modified); diff --git a/src/mbstf/ObjectStore.hh b/src/mbstf/ObjectStore.hh index 9bbfe71..a6925c0 100644 --- a/src/mbstf/ObjectStore.hh +++ b/src/mbstf/ObjectStore.hh @@ -197,6 +197,38 @@ public: Metadata &mediaType(const std::string &media_type) {m_mediaType = media_type; return *this;}; Metadata &mediaType(std::string &&media_type) {m_mediaType = std::move(media_type); return *this;}; + + + /** Latest availability start time of this object at the MBS Client. + * + * TS 26.517 V18.6.0 clause 6.2.3.5 requires this to be maintained per object in the object + * list: "The object's latest availability start time at the MBS Client. After this time, + * the MBS-Aware Application may request the full object from the MBSTF Client by using the + * URL of the object." + * + * Where the Application Service Entry Point document is a DASH MPD the clause takes it + * from there: "When the Application Service Entry Point document is a DASH MPD, the + * availability start time is signalled in this document." Otherwise it is the ingest time + * plus a configured distribution offset. + */ + const std::optional &availabilityStartTime() const { return m_availabilityStartTime; }; + Metadata &availabilityStartTime(const datetime_type &val) { m_availabilityStartTime = val; return *this; }; + Metadata &availabilityStartTime(const std::optional &val) { m_availabilityStartTime = val; return *this; }; + + + /** Availability end time of this object from the MBSTF Client. + * + * TS 26.517 V18.6.0 clause 6.2.3.5: "The object's availability end time from the MBSTF + * Client. After this time, the object may no longer be requested by the MBS-Aware + * Application." + * + * Distinct from cacheExpires(), which carries the origin server's HTTP Cache-Control + * max-age for the ingest fetch and is a property of the origin, not of this distribution + * session. + */ + const std::optional &availabilityEndTime() const { return m_availabilityEndTime; }; + Metadata &availabilityEndTime(const datetime_type &val) { m_availabilityEndTime = val; return *this; }; + Metadata &availabilityEndTime(const std::optional &val) { m_availabilityEndTime = val; return *this; }; bool hasExpiryTime() const { return m_cacheExpires.has_value(); }; const datetime_type &ExpiryTime() const { return m_cacheExpires.value();}; const std::optional& cacheExpires() const { return m_cacheExpires;}; @@ -284,6 +316,8 @@ public: std::optional m_objDistributionBaseUrl; std::optional m_entityTag; std::optional m_cacheExpires; + std::optional m_availabilityStartTime; + std::optional m_availabilityEndTime; datetime_type m_receivedTime; datetime_type m_created; datetime_type m_modified; diff --git a/src/mbstf/PullObjectIngester.cc b/src/mbstf/PullObjectIngester.cc index 80d94ae..d9af7d9 100644 --- a/src/mbstf/PullObjectIngester.cc +++ b/src/mbstf/PullObjectIngester.cc @@ -52,7 +52,7 @@ PullObjectIngester::IngestItem::IngestItem(const ObjectStore::Metadata &object_m { } -PullObjectIngester::IngestItem::IngestItem(const std::string &object_id, const std::string &url, const std::string &acquisition_id, const std::optional &obj_ingest_base_url, const std::optional &obj_distribution_base_url, const std::optional &download_deadline, bool force_recache, bool keep_after_send, bool compress_send) +PullObjectIngester::IngestItem::IngestItem(const std::string &object_id, const std::string &url, const std::string &acquisition_id, const std::optional &obj_ingest_base_url, const std::optional &obj_distribution_base_url, const std::optional &download_deadline, bool force_recache, bool keep_after_send, bool compress_send, const std::optional &availability_start_time, const std::optional &availability_end_time) :m_objectId(object_id) ,m_url(url) ,m_acquisitionId(acquisition_id) @@ -62,6 +62,8 @@ PullObjectIngester::IngestItem::IngestItem(const std::string &object_id, const s ,m_forceRecache(force_recache) ,m_markAsKeepAfterSend(keep_after_send) ,m_markAsCompressedSend(compress_send) + ,m_availabilityStartTime(availability_start_time) + ,m_availabilityEndTime(availability_end_time) { } @@ -75,6 +77,8 @@ PullObjectIngester::IngestItem::IngestItem(const IngestItem &other) ,m_forceRecache(other.m_forceRecache) ,m_markAsKeepAfterSend(other.m_markAsKeepAfterSend) ,m_markAsCompressedSend(other.m_markAsCompressedSend) + ,m_availabilityStartTime(other.m_availabilityStartTime) + ,m_availabilityEndTime(other.m_availabilityEndTime) { } @@ -88,6 +92,8 @@ PullObjectIngester::IngestItem::IngestItem(IngestItem &&other) ,m_forceRecache(other.m_forceRecache) ,m_markAsKeepAfterSend(other.m_markAsKeepAfterSend) ,m_markAsCompressedSend(other.m_markAsCompressedSend) + ,m_availabilityStartTime(std::move(other.m_availabilityStartTime)) + ,m_availabilityEndTime(std::move(other.m_availabilityEndTime)) { } diff --git a/src/mbstf/PullObjectIngester.hh b/src/mbstf/PullObjectIngester.hh index 5fd7a0a..e890835 100644 --- a/src/mbstf/PullObjectIngester.hh +++ b/src/mbstf/PullObjectIngester.hh @@ -50,7 +50,9 @@ public: const std::optional &obj_ingest_base_url = std::nullopt, const std::optional &obj_distribution_base_url = std::nullopt, const std::optional &download_deadline = std::nullopt, - bool force_recache = false, bool keep_after_send = false, bool compressed_send = false); + bool force_recache = false, bool keep_after_send = false, bool compressed_send = false, + const std::optional &availability_start_time = std::nullopt, + const std::optional &availability_end_time = std::nullopt); IngestItem(const IngestItem &other); IngestItem(IngestItem &&other); virtual ~IngestItem() {}; @@ -88,6 +90,13 @@ public: bool markAsCompressedSend() const { return m_markAsCompressedSend; }; IngestItem &markAsCompressedSend(bool compress_send) { m_markAsCompressedSend = compress_send; return *this; }; + /** Availability start/end time for this ingest item (TS 26.517 clause 6.2.3.5). + * Carried through to ObjectStore::Metadata when the object is stored. + */ + const std::optional &availabilityStartTime() const { return m_availabilityStartTime; }; + IngestItem &availabilityStartTime(const std::optional &val) { m_availabilityStartTime = val; return *this; }; + const std::optional &availabilityEndTime() const { return m_availabilityEndTime; }; + IngestItem &availabilityEndTime(const std::optional &val) { m_availabilityEndTime = val; return *this; }; private: std::string m_objectId; std::string m_url; @@ -95,6 +104,8 @@ public: std::optional m_objIngestBaseUrl; std::optional m_objDistributionBaseUrl; std::optional m_deadline; + std::optional m_availabilityStartTime; + std::optional m_availabilityEndTime; bool m_forceRecache; bool m_markAsKeepAfterSend; bool m_markAsCompressedSend; diff --git a/tests/meson.build b/tests/meson.build index abd03b1..03e810c 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -22,13 +22,13 @@ test('test_subscriber_subscription', executable('testSubscriberSubscription', 'test_SubscriberSubscription.cc', test_source_subscriber_subscription, install:false, include_directories:[libmbstf_libinc, libinc], dependencies : [libmbstf_dep]) ,verbose: true, timeout: 600, protocol: 'exitcode') -#test('test_pull_object_ingester', -# executable('testPullObjectIngester', 'test_PullObjectIngester.cc', test_source_pull_object_ingester, install:false, include_directories:[libmbstf_libinc, libinc], dependencies : [libmbstf_dep]) -# ,verbose: true, timeout: 600, protocol: 'exitcode') +test('test_pull_object_ingester', + executable('testPullObjectIngester', 'test_PullObjectIngester.cc', test_source_pull_object_ingester, install:false, include_directories:[libmbstf_libinc, libinc], dependencies : [libmbstf_dep]) + ,verbose: true, timeout: 600, protocol: 'exitcode') -#test('test_object_list_packager', -# executable('testObjectListPackager', 'test_ObjectListPackager.cc', test_source_object_list_packager, install:false, include_directories:[libmbstf_libinc, libinc], dependencies : [libmbstf_dep, boost_dep, rt_libflute_dep, libuuid_dep]) -# ,verbose: true, timeout: 600, protocol: 'exitcode') +test('test_object_list_packager', + executable('testObjectListPackager', 'test_ObjectListPackager.cc', test_source_object_list_packager, install:false, include_directories:[libmbstf_libinc, libinc], dependencies : [libmbstf_dep, boost_dep, rt_libflute_dep, libuuid_dep]) + ,verbose: true, timeout: 600, protocol: 'exitcode') #test_object_store = executable('testObjectStore', 'test_ObjectStore.cc', test_source_object_store, install:false, include_directories:[libmbstf_libinc, libinc]) #test_object_store = executable('testObjectStore', 'test_ObjectStore.cc', test_source_object_store, install:false, include_directories:[libmbstf_libinc, libinc]) #test('run_test_object_store', executable('testObjectStore')) diff --git a/tests/test_ObjectListPackager.cc b/tests/test_ObjectListPackager.cc index 57f51d0..c9693f6 100644 --- a/tests/test_ObjectListPackager.cc +++ b/tests/test_ObjectListPackager.cc @@ -1,5 +1,5 @@ /****************************************************************************** - * 5G-MAG Reference Tools: MBS Traffic Function: Testing MBSTF Object store + * 5G-MAG Reference Tools: MBS Transport Function: ObjectListPackager tests ****************************************************************************** * Copyright: (C)2024 British Broadcasting Corporation * License: 5G-MAG Public License v1 @@ -10,158 +10,142 @@ * https://drive.google.com/file/d/1cinCiA778IErENZ3JN52VFW-1ffHpx7Z/view */ +/* Covers the transmission ordering the segment streaming operating mode requires. + * + * TS 26.517 V18.6.0 clause 6.2.3.5: "The MBSTF shall transmit each object in the object list such + * that the last packet of the delivered FLUTE transmission object (including any FEC recovery + * packets, when configured) is available at the MBSTF Client no later than its availability start + * time." + * + * The packaging queue is ordered by each item's deadline, which for OBJECT_STREAMING is that + * availability start time. Only the ordering is exercised here: constructing an ObjectListPackager + * brings up a FLUTE transmitter and its sockets, which is not a unit test, so the predicate is + * tested directly. + */ -#include -#include -#include #include -#include -#include #include +#include +#include +#include #include #include -#include -#include -#include - -#include -#include "common.hh" -#include "NfServer.hh" #include "ObjectStore.hh" #include "ObjectListPackager.hh" -#include "Open5GSYamlDocument.hh" MBSTF_NAMESPACE_START using namespace std::literals; -class App { -public: - static const App &self(); - const NfServer::AppMetadata &mbstfAppMetadata() const; - Open5GSYamlDocument configDocument() const; -}; +int pass = 0; +int fail = 0; -const App &App::self() +static void check(bool condition, const std::string &what) { - static const App instance; - return instance; + if (condition) { + pass++; + std::cout << "INFO: " << what << " passed." << std::endl; + } else { + fail++; + std::cout << "ERROR: " << what << " failed." << std::endl; + } } -const NfServer::AppMetadata &App::mbstfAppMetadata() const +using time_type = ObjectListPackager::time_type; + +static std::shared_ptr makeObject(const std::string &object_id) { - static const NfServer::AppMetadata app_metadata("testObjectListPackager", "0.0.1", "test-host"); - return app_metadata; + ObjectStore::ObjectData data = {0x31, 0x32}; + ObjectStore::Metadata metadata(object_id, "application/octet-stream", "url-" + object_id, + "fetched-" + object_id, "acquisition-" + object_id, + std::chrono::system_clock::now()); + return std::make_shared(std::move(data), std::move(metadata)); } -Open5GSYamlDocument App::configDocument() const +/* An object whose availability start time is earlier must be transmitted first. */ +static void testEarlierDeadlineSortsFirst() { - return Open5GSYamlDocument(nullptr); + auto now = std::chrono::system_clock::now(); + ObjectListPackager::PackageItem early(makeObject("early"), time_type(now + 10s)); + ObjectListPackager::PackageItem late(makeObject("late"), time_type(now + 60s)); + + check(ObjectListPackager::PackageItem::earlierDeadlineFirst(early, late), + "testEarlierDeadlineSortsFirst early before late"); + check(!ObjectListPackager::PackageItem::earlierDeadlineFirst(late, early), + "testEarlierDeadlineSortsFirst late not before early"); } -class ObjectController {}; -class ObjectListController: public ObjectController {}; - -int pass= 0; -int fail = 0; - -std::string firstObject = "obj1"; -std::string secondObject = "obj2"; - -void testAddObject(ObjectStore& store) { - - ObjectStore::ObjectData firstObjectData = {0x31, 0x32}; - ObjectStore::ObjectData secondObjectData = {0x50, 0x51, 0x52}; - - ObjectStore::Metadata firstObjectMetadata("type1", "url1", "fetched_url1", "acquisition1", std::chrono::system_clock::now()); - ObjectStore::Metadata secondObjectMetadata("type2", "url2", "fetched_url2", "acquisition2", std::chrono::system_clock::now() + std::chrono::minutes(1)); - - firstObjectMetadata.entityTag("etag1"); - firstObjectMetadata.cacheExpires(std::chrono::system_clock::now() + 5s); - - secondObjectMetadata.entityTag("etag2"); - secondObjectMetadata.cacheExpires(std::chrono::system_clock::now() + 5s); - store.addObject(firstObject, std::move(firstObjectData), std::move(firstObjectMetadata)); - store.addObject(secondObject, std::move(secondObjectData), std::move(secondObjectMetadata)); - - if (store.getObjectData(firstObject) == ObjectStore::ObjectData{0x31, 0x32}) { - std::cout<<"INFO: testAddObject for firstObject passed."< queue; + queue.emplace_back(makeObject("third"), time_type(now + 90s)); + queue.emplace_back(makeObject("undated")); + queue.emplace_back(makeObject("first"), time_type(now + 10s)); + queue.emplace_back(makeObject("second"), time_type(now + 50s)); + + queue.sort(ObjectListPackager::PackageItem::earlierDeadlineFirst); + + std::vector order; + for (auto &item : queue) { + order.push_back(item.object()->second.objectId()); } -} - -void testDeleteSecondObject(ObjectStore& store) { - store.deleteObject(secondObject); - - try { - store.getObjectData(secondObject); - std::cout<<"ERROR: testDeleteObject for secondObject failed."< address = std::string("127.0.0.1"); - uint32_t rateLimit = 1000; - unsigned short mtu = 1500; - in_port_t port = 8080; - - ObjectListPackager packager(store, controller, address, rateLimit, mtu, port, std::nullopt, 0); - packager.startWorker(); - // Add a PackageItem to ObjectPackager - ObjectListPackager::PackageItem item("obj1"); - packager.add(item); - std::this_thread::sleep_for(10s); - // Verification - std::cout << "Test completed successfully." << std::endl; + check(ObjectListPackager::PackageItem::earlierDeadlineFirst(overdue, upcoming), + "testOverdueObjectSortsFirst"); } MBSTF_NAMESPACE_STOP MBSTF_NAMESPACE_USING; -int main() { - - ObjectListController objectListController; - ObjectStore store(objectListController); +int main() +{ + std::cout << "### ObjectListPackager: Test start ####" << std::endl; - std::cout << "### ObjectStore: Test start #### " << std::endl; - - testAddObject(store); - testObjectListPackager(store, objectListController); - testDeleteFirstObject(store); - testDeleteSecondObject(store); + testEarlierDeadlineSortsFirst(); + testItemWithoutDeadlineSortsLast(); + testQueueSortsIntoAvailabilityOrder(); + testOverdueObjectSortsFirst(); - return 0; + std::cout << "Test: ObjectListPackager Pass: " << pass << " Fail: " << fail << std::endl; + std::cout << "### ObjectListPackager: Test finish ####" << std::endl; + return fail ? 1 : 0; } /* vim:ts=8:sts=4:sw=4:expandtab: diff --git a/tests/test_ObjectStore.cc b/tests/test_ObjectStore.cc index 16f51a9..1d6c410 100644 --- a/tests/test_ObjectStore.cc +++ b/tests/test_ObjectStore.cc @@ -145,6 +145,87 @@ void testGetStaleObjects(ObjectStore& store) { MBSTF_NAMESPACE_STOP MBSTF_NAMESPACE_USING; +/* TS 26.517 V18.6.0 clause 6.2.3.5 requires the object's latest availability start time and its + availability end time to be maintained per object in the object list, separately from the HTTP + cache expiry of the ingest fetch. Check they are held, are independent of cacheExpires(), and + survive copy and assignment. */ +void testAvailabilityTimes() { + auto now = std::chrono::system_clock::now(); + auto start = now + std::chrono::seconds(30); + auto end = now + std::chrono::seconds(300); + auto cache = now + std::chrono::seconds(90); + + ObjectStore::Metadata meta("availObj", "type", "url", "fetched_url", "acquisition", now); + + if (!meta.availabilityStartTime().has_value() && !meta.availabilityEndTime().has_value()) { + pass++; + std::cout<<"INFO: testAvailabilityTimes default-absent passed."< -#include -#include #include -#include -#include #include -#include -#include #include -#include -#include +#include #include "common.hh" #include "ObjectStore.hh" @@ -33,40 +40,141 @@ using namespace std::literals; class ObjectController {}; -int pass= 0; +int pass = 0; int fail = 0; +static void check(bool condition, const std::string &what) +{ + if (condition) { + pass++; + std::cout << "INFO: " << what << " passed." << std::endl; + } else { + fail++; + std::cout << "ERROR: " << what << " failed." << std::endl; + } +} + +using time_type = PullObjectIngester::time_type; + +/* The base URLs are what the distribution URL is derived from, per the first item of the object + * list in clause 6.2.3.5. Both must survive into the ingest item. */ +static void testCarriesBothBaseUrls() +{ + PullObjectIngester::IngestItem item("obj1", "http://127.0.0.1/seg1.m4s", "acq1", + std::string("http://127.0.0.1/"), + std::string("http://127.0.0.2/")); + + check(item.objectId() == "obj1", "testCarriesBothBaseUrls objectId"); + check(item.url() == "http://127.0.0.1/seg1.m4s", "testCarriesBothBaseUrls url"); + check(item.acquisitionId() == "acq1", "testCarriesBothBaseUrls acquisitionId"); + check(item.objIngestBaseUrl().has_value() && + item.objIngestBaseUrl().value() == "http://127.0.0.1/", + "testCarriesBothBaseUrls objIngestBaseUrl"); + check(item.objDistributionBaseUrl().has_value() && + item.objDistributionBaseUrl().value() == "http://127.0.0.2/", + "testCarriesBothBaseUrls objDistributionBaseUrl"); +} + +/* An item with no times stated must report none, rather than defaulting to something a caller would + * mistake for a real availability window. */ +static void testTimesAbsentByDefault() +{ + PullObjectIngester::IngestItem item("obj2", "http://127.0.0.1/seg2.m4s", "acq2"); + + check(!item.hasDeadline(), "testTimesAbsentByDefault no deadline"); + check(!item.availabilityStartTime().has_value(), "testTimesAbsentByDefault no availability start"); + check(!item.availabilityEndTime().has_value(), "testTimesAbsentByDefault no availability end"); +} + +/* The three times are independent. The deadline governs the pull from origin; the availability times + * govern when a client may request the object. Conflating them is the defect these guard against. */ +static void testAvailabilityTimesIndependentOfDeadline() +{ + auto now = std::chrono::system_clock::now(); + time_type deadline(now + 10s); + time_type avail_start(now + 30s); + time_type avail_end(now + 300s); + + PullObjectIngester::IngestItem item("obj3", "http://127.0.0.1/seg3.m4s", "acq3", + std::string("http://127.0.0.1/"), + std::string("http://127.0.0.2/"), + deadline, false, false, false, avail_start, avail_end); + + check(item.hasDeadline() && item.deadline().value() == deadline, + "testAvailabilityTimesIndependentOfDeadline deadline"); + check(item.availabilityStartTime().has_value() && item.availabilityStartTime().value() == avail_start, + "testAvailabilityTimesIndependentOfDeadline availability start"); + check(item.availabilityEndTime().has_value() && item.availabilityEndTime().value() == avail_end, + "testAvailabilityTimesIndependentOfDeadline availability end"); + + check(item.deadline().value() != item.availabilityStartTime().value() && + item.availabilityStartTime().value() != item.availabilityEndTime().value(), + "testAvailabilityTimesIndependentOfDeadline all three differ"); + + /* Availability start no later than availability end, the ordering clause 6.2.3.5 implies by + deriving one from a distribution offset and the other from a clean-up time. */ + check(item.availabilityStartTime().value() < item.availabilityEndTime().value(), + "testAvailabilityTimesIndependentOfDeadline start precedes end"); +} + +/* The setters must be usable after construction, which is how DASHManifestHandler attaches the times + * to an item built from existing object metadata. */ +static void testTimesSettableAfterConstruction() +{ + auto now = std::chrono::system_clock::now(); + time_type avail_start(now + 45s); + time_type avail_end(now + 450s); + + PullObjectIngester::IngestItem item("obj4", "http://127.0.0.1/seg4.m4s", "acq4"); + item.availabilityStartTime(avail_start).availabilityEndTime(avail_end); + + check(item.availabilityStartTime().has_value() && item.availabilityStartTime().value() == avail_start, + "testTimesSettableAfterConstruction availability start"); + check(item.availabilityEndTime().has_value() && item.availabilityEndTime().value() == avail_end, + "testTimesSettableAfterConstruction availability end"); + check(!item.hasDeadline(), "testTimesSettableAfterConstruction deadline untouched"); +} + +/* Copying must carry every value: items are copied into and out of the ingest list. */ +static void testCopyPreservesEverything() +{ + auto now = std::chrono::system_clock::now(); + PullObjectIngester::IngestItem original("obj5", "http://127.0.0.1/seg5.m4s", "acq5", + std::string("http://127.0.0.1/"), + std::string("http://127.0.0.2/"), + time_type(now + 5s), false, false, false, + time_type(now + 25s), time_type(now + 250s)); + PullObjectIngester::IngestItem copied(original); + + check(copied.objectId() == original.objectId() && copied.url() == original.url() && + copied.acquisitionId() == original.acquisitionId(), + "testCopyPreservesEverything identifiers"); + check(copied.objIngestBaseUrl() == original.objIngestBaseUrl() && + copied.objDistributionBaseUrl() == original.objDistributionBaseUrl(), + "testCopyPreservesEverything base URLs"); + check(copied.deadline() == original.deadline() && + copied.availabilityStartTime() == original.availabilityStartTime() && + copied.availabilityEndTime() == original.availabilityEndTime(), + "testCopyPreservesEverything times"); +} MBSTF_NAMESPACE_STOP + MBSTF_NAMESPACE_USING; -int main() { - // Create instances of ObjectStore and ObjectController - ObjectController objectController; - ObjectStore store(objectController); - - // Create a list of IngestItem - //using time_type = std::chrono::system_clock::time_point; - std::list id_to_url_map = { - {"object1", "http://127.0.0.1/object1", "object1", "http://127.0.0.1/", std::nullopt, std::nullopt}, - {"object2", "http://127.0.0.1/object2", "object2", "http://127.0.0.1/", "http://127.0.0.2/", std::nullopt} - }; - - // Create an instance of PullObjectIngester - PullObjectIngester pullIngester(store, objectController, id_to_url_map); - - // Print information about each IngestItem - for (const auto& item : id_to_url_map) { - std::cout << "Object ID: " << item.objectId() << std::endl; - std::cout << "URL: " << item.url() << std::endl; - if (item.hasDeadline()) { - std::cout << "Deadline: " << std::chrono::system_clock::to_time_t(item.deadline(std::chrono::system_clock::now())) << std::endl; - } else { - std::cout << "No deadline" << std::endl; - } - } +int main() +{ + std::cout << "### PullObjectIngester: Test start ####" << std::endl; + + testCarriesBothBaseUrls(); + testTimesAbsentByDefault(); + testAvailabilityTimesIndependentOfDeadline(); + testTimesSettableAfterConstruction(); + testCopyPreservesEverything(); - return 0; + std::cout << "Test: PullObjectIngester Pass: " << pass << " Fail: " << fail << std::endl; + std::cout << "### PullObjectIngester: Test finish ####" << std::endl; + return fail ? 1 : 0; } /* vim:ts=8:sts=4:sw=4:expandtab: From e5cdc86acfaeefa4719ca4dc8bbfc7696a234e4e Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sat, 5 Sep 2026 18:20:25 +0200 Subject: [PATCH 05/19] mbstf: apply the session's requested FEC configuration to the FLUTE transmitter Problem The AL-FEC configuration a Distribution Session provisions reached no transmitter, so every session was sent unprotected however it was provisioned. FecOtiHelper, which converts the session's FECConfig into the Transmitter-level FEC OTI, was present in the tree but never listed in src/mbstf/meson.build, so it was never compiled. Neither ObjectListPackager nor ObjectCarouselPackager passed a FEC OTI or a redundancy level to LibFlute::Transmitter, which has taken both since the pinned revision, and no controller supplied a FECConfig to either packager. [code-derived] Basis TS 26.517 V18.6.0, clause 6.2.1: "If FLUTE [12] is used to realise the Object Distribution Method, the MBS Distribution Session shall conform to the MBMS Download Profile as defined in clause L.4 of TS 26.346 [7] with the additional requirements in clause 6.2 of the present document." TS 26.346 V18.2.0, clause L.4.7 "Other aspects of FLUTE delivery": "Regarding Application Layer FEC support, the two FEC schemes referenced in this specification, the Compact No-Code FEC scheme as specified in RFC 3695 [13], and the Raptor FEC scheme as specified in RFC 5053 [91] are optional to implement by the BM-SC and mandatory to support by the UE." Those two together decide which schemes may be sent: the profile clause 6.2.1 selects admits Compact No-Code and Raptor, and no other. RaptorQ (RFC 6330) is not among them. Raised by Reading the branch while checking the review comments on pull request #71. Change Adds FecOtiHelper to the build. ObjectListPackager and ObjectCarouselPackager convert the session's FECConfig with fecOtiFromFecConfig() and pass the resulting FEC OTI and redundancy level to LibFlute::Transmitter. A scheme the profile does not admit throws and is reported as a packaging failure for that session rather than silently downgraded: a session sent unprotected when it asked for protection is a worse outcome than a visible failure. ObjectCarouselPackager gains the FEC parameter ObjectListPackager now also carries, defaulted so no other call site changes, and all controllers supply distributionSession().getFecInformation(). Verification T1: tests/test_FecOtiHelper.cc, 8 cases, all passing, covering absent configuration, a null shared pointer, Compact No-Code yielding no FEC OTI, Raptor yielding a Raptor OTI carrying the requested overhead, and refusal of RaptorQ, an unknown scheme and a negative overhead. The suite is new: FecOtiHelper had never been compiled, let alone tested, and one case caught a real error while being written, an incorrect Compact No-Code URN. All five suites pass, 50 cases. T2: the demo delivers unchanged with this build. That run does NOT exercise the populated path: the demo provisions no FEC, so it evidences no regression, not that FEC transmission works. Not in this change No end-to-end evidence that a FEC-protected session is transmitted and decoded. Nothing here verifies rt-libflute's own Raptor encoding, and no receiver in this project has been shown to decode a protected session. The conformance record must not claim AL-FEC works on the strength of this commit. --- src/mbstf/FecOtiHelper.cc | 82 +++++++++++++++++ src/mbstf/FecOtiHelper.hh | 56 ++++++++++++ src/mbstf/ObjectCarouselController.cc | 3 +- src/mbstf/ObjectCarouselPackager.cc | 25 +++++- src/mbstf/ObjectCarouselPackager.hh | 3 +- src/mbstf/ObjectListController.cc | 3 +- src/mbstf/ObjectListPackager.cc | 26 +++++- src/mbstf/ObjectListPackager.hh | 3 +- src/mbstf/ObjectPackager.hh | 14 ++- src/mbstf/ObjectStreamingController.cc | 3 +- src/mbstf/meson.build | 9 ++ tests/meson.build | 4 + tests/test_FecOtiHelper.cc | 117 +++++++++++++++++++++++++ 13 files changed, 335 insertions(+), 13 deletions(-) create mode 100644 src/mbstf/FecOtiHelper.cc create mode 100644 src/mbstf/FecOtiHelper.hh create mode 100644 tests/test_FecOtiHelper.cc diff --git a/src/mbstf/FecOtiHelper.cc b/src/mbstf/FecOtiHelper.cc new file mode 100644 index 0000000..6a17bf0 --- /dev/null +++ b/src/mbstf/FecOtiHelper.cc @@ -0,0 +1,82 @@ +/****************************************************************************** + * 5G-MAG Reference Tools: MBS Transport Function: FEC OTI helper + ****************************************************************************** + * Copyright: (C)2025-2026 British Broadcasting Corporation + * Author(s): Dev Audsin + * David Waring + * License: 5G-MAG Public License v1 + * + * For full license terms please see the LICENSE file distributed with this + * program. If this file is missing then the license can be retrieved from + * https://drive.google.com/file/d/1cinCiA778IErENZ3JN52VFW-1ffHpx7Z/view + */ + +#include +#include +#include +#include +#include +#include + +#include "Transmitter.h" // LibFlute + +#include "openapi/model/FECConfig.h" + +#include "FecOtiHelper.hh" + +MBSTF_NAMESPACE_START + +namespace { + +/* fecScheme is a URN naming an IANA "RMT FEC Encoding ID" (RFC 5052). TS 29.580 V18.8.0 clause + 6.2.6.2.14, table 6.2.6.2.14-1, row fecScheme: "It shall be identified using a term from the + IANA: "Reliable Multicast Transport (RMT) FEC Encoding IDs and FEC Instance IDs" [20] expressed + as a URN, e.g.: urn:ietf:rmt:fec:encoding:0". RFC 5053 clause 7 (IANA Considerations): "This + document assigns the Fully-Specified FEC Encoding ID 1 under the ietf:rmt:fec:encoding + name-space to "Raptor Code"." rt-libflute's own FecScheme enum (include/flute_types.h) fixes the + same numeric values on the wire (FEC-OTI-FEC-Encoding-ID): CompactNoCode=0, Raptor=1, RaptorQ=6. */ +const char * const kFecSchemeCompactNoCode = "urn:ietf:rmt:fec:encoding:0"; +const char * const kFecSchemeRaptor = "urn:ietf:rmt:fec:encoding:1"; +const char * const kFecSchemeRaptorQ = "urn:ietf:rmt:fec:encoding:6"; + +} // anonymous namespace + +std::pair, uint32_t> fecOtiFromFecConfig( + const std::optional> &fec_information) +{ + if (!fec_information || !fec_information.value()) { + return {std::nullopt, LibFlute::kDefaultFecRedundancyLevel}; + } + + const reftools::mbstf::FECConfig &fec_config = *fec_information.value(); + const std::string &fec_scheme = fec_config.getFecScheme(); + int32_t fec_overhead = fec_config.getFecOverHead(); + + if (fec_overhead < 0) { + throw std::runtime_error("fecOverHead must not be negative: " + std::to_string(fec_overhead)); + } + + if (fec_scheme == kFecSchemeCompactNoCode) { + return {std::nullopt, LibFlute::kDefaultFecRedundancyLevel}; + } + if (fec_scheme == kFecSchemeRaptor) { + LibFlute::FecOti oti{}; + oti.encoding_id = LibFlute::FecScheme::Raptor; + /* max_source_block_length and encoding_symbol_length are left at their defaults (0): + rt-libflute's own Transmitter derives encoding_symbol_length from the session's path MTU + and, under the 3GPP profiles, caps max_source_block_length at the TS 26.346 clause 7.2.3 + 256 KB sub-block ceiling itself when it is left 0. No MBSTF-side bound is invented here. */ + return {oti, static_cast(fec_overhead)}; + } + if (fec_scheme == kFecSchemeRaptorQ) { + throw std::runtime_error( + "fecScheme " + fec_scheme + " (RaptorQ) is not one of the FEC schemes the MBMS Download " + "Profile admits (TS 26.346 V18.2.0 clause L.4.7); this MBSTF cannot honour it"); + } + throw std::runtime_error("fecScheme " + fec_scheme + " is not implemented by this MBSTF"); +} + +MBSTF_NAMESPACE_STOP + +/* vim:ts=8:sts=4:sw=4:expandtab: + */ diff --git a/src/mbstf/FecOtiHelper.hh b/src/mbstf/FecOtiHelper.hh new file mode 100644 index 0000000..a9682e3 --- /dev/null +++ b/src/mbstf/FecOtiHelper.hh @@ -0,0 +1,56 @@ +#ifndef _MBSTF_FEC_OTI_HELPER_HH_ +#define _MBSTF_FEC_OTI_HELPER_HH_ +/****************************************************************************** + * 5G-MAG Reference Tools: MBS Transport Function: FEC OTI helper + ****************************************************************************** + * Copyright: (C)2025-2026 British Broadcasting Corporation + * Author(s): Dev Audsin + * David Waring + * License: 5G-MAG Public License v1 + * + * For full license terms please see the LICENSE file distributed with this + * program. If this file is missing then the license can be retrieved from + * https://drive.google.com/file/d/1cinCiA778IErENZ3JN52VFW-1ffHpx7Z/view + */ + +#include +#include +#include +#include + +#include "Transmitter.h" // LibFlute + +#include "common.hh" +#include "openapi/model/FECConfig.h" + +MBSTF_NAMESPACE_START + +/* Only Raptor (RFC 5053) is wired here. TS 26.517 V18.6.0 clause 6.2.1 binds every FLUTE object + distribution session this component runs to the MBMS Download Profile ("the MBS Distribution + Session shall conform to the MBMS Download Profile as defined in clause L.4 of TS 26.346"), and + TS 26.346 V18.2.0 clause L.4.7 admits exactly two AL-FEC schemes into that profile: "the Compact + No-Code FEC scheme as specified in RFC 3695 [13], and the Raptor FEC scheme as specified in RFC + 5053 [91] are optional to implement by the BM-SC and mandatory to support by the UE." RaptorQ is + RFC 6330, referenced by neither TS 26.346 nor TS 26.517, and rt-libflute's own Transmitter now + refuses it under this profile at construction (Transmitter.cpp, citing the same L.4.7 sentence) + rather than send a session no conformant receiver has any obligation to decode. Requesting it + here is therefore reported as a packaging failure rather than passed through to a constructor + that would throw. + + Returns the Transmitter-level FEC OTI to apply (unset for no FEC or Compact No-Code, which + carries no repair symbols of its own and so has the same observable effect) and the FEC + redundancy level (TS 26.346 V18.2.0 clause 7.3.2.11) to apply when one is requested. Throws + std::runtime_error for a scheme this MBSTF does not wire or a malformed overhead value. + + Shared by every ObjectPackager subclass that constructs its own LibFlute::Transmitter + (originally ObjectListPackager-local; extracted so ObjectCarouselPackager can apply the same + Distribution-Session-requested FEC configuration instead of always sending unprotected FLUTE). */ +std::pair, uint32_t> fecOtiFromFecConfig( + const std::optional> &fec_information); + +MBSTF_NAMESPACE_STOP + +#endif /* _MBSTF_FEC_OTI_HELPER_HH_ */ + +/* vim:ts=8:sts=4:sw=4:expandtab: + */ diff --git a/src/mbstf/ObjectCarouselController.cc b/src/mbstf/ObjectCarouselController.cc index 710a73f..a75007f 100644 --- a/src/mbstf/ObjectCarouselController.cc +++ b/src/mbstf/ObjectCarouselController.cc @@ -75,7 +75,8 @@ void ObjectCarouselController::setObjectPackager() uint32_t rate_limit = distributionSession().getRateLimit(); in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); unsigned short mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, GET_MTU_ETHERNET_PAYLOAD) - GTP_HEADER_SIZE; - packager(new ObjectCarouselPackager(objectStore(), *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port)); + packager(new ObjectCarouselPackager(objectStore(), *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port, + distributionSession().getFecInformation())); auto pkgr = getObjectCarouselPackager(); subscribeToService(*pkgr); startWorker(); diff --git a/src/mbstf/ObjectCarouselPackager.cc b/src/mbstf/ObjectCarouselPackager.cc index a19c7be..ed8b860 100644 --- a/src/mbstf/ObjectCarouselPackager.cc +++ b/src/mbstf/ObjectCarouselPackager.cc @@ -36,6 +36,7 @@ #include "ObjectStore.hh" #include "openapi/model/Object.h" +#include "FecOtiHelper.hh" #include "ObjectCarouselPackager.hh" using namespace std::literals::chrono_literals; @@ -154,8 +155,9 @@ ObjectCarouselPackager::ObjectCarouselPackager(const std::shared_ptr &object_store, ObjectController &controller, const SsmPort &ssm_port, uint32_t rate_limit, unsigned short mtu, - const std::optional &tunnel_address, in_port_t tunnel_port) - :ObjectPackager(object_store, controller, ssm_port, rate_limit, mtu, tunnel_address, tunnel_port) + const std::optional &tunnel_address, in_port_t tunnel_port, + const std::optional> &fec_information) + :ObjectPackager(object_store, controller, ssm_port, rate_limit, mtu, tunnel_address, tunnel_port, fec_information) ,m_packageItemsMutex(new decltype(m_packageItemsMutex)::element_type) ,m_packageItems() ,m_packagingUpdateCondVar() @@ -258,10 +260,27 @@ void ObjectCarouselPackager::ensureTransmitter() if (!m_transmitter) { const auto &ssm_port = ssmPort(); if (!ssm_port) return; + /* The Distribution Session's own requested AL-FEC configuration, converted to the + Transmitter-level FEC OTI. Without this the session is sent unprotected however it was + provisioned. fecOtiFromFecConfig() rejects a scheme the MBMS Download Profile does not + admit, which is a packaging failure for this session rather than a reason to send it + without the protection it asked for. */ + std::optional content_fec_oti; + uint32_t fec_redundancy_level = LibFlute::kDefaultFecRedundancyLevel; + try { + std::tie(content_fec_oti, fec_redundancy_level) = fecOtiFromFecConfig(fecInformation()); + } catch (const std::runtime_error &err) { + ogs_error("Cannot apply the Distribution Session's FEC configuration, not transmitting: %s", + err.what()); + return; + } m_transmitter.reset(new LibFlute::Transmitter(ssm_port.destinationAddress(), static_cast(ssm_port.port()), tsi(), mtu(), rateLimit(), m_io, m_tunnelEndpoint, LibFlute::FileDeliveryTable::FDT_NS_DRAFT_2005, true, - ssm_port.sourceAddress())); + ssm_port.sourceAddress(), + content_fec_oti, + LibFlute::Profile::Ts26517, + fec_redundancy_level)); m_transmitter->register_completion_callback( [this](uint32_t toi) { ogs_debug("Object with TOI %d completed", toi); diff --git a/src/mbstf/ObjectCarouselPackager.hh b/src/mbstf/ObjectCarouselPackager.hh index 2f112e8..fe6ce90 100644 --- a/src/mbstf/ObjectCarouselPackager.hh +++ b/src/mbstf/ObjectCarouselPackager.hh @@ -90,7 +90,8 @@ public: unsigned short mtu, const std::optional &tunnel_address, in_port_t tunnel_port); ObjectCarouselPackager(const std::shared_ptr &object_store, ObjectController &controller, const SsmPort &ssm_port, uint32_t rateLimit, unsigned short mtu, - const std::optional &tunnel_address, in_port_t tunnel_port); + const std::optional &tunnel_address, in_port_t tunnel_port, + const std::optional> &fec_information = std::nullopt); virtual ~ObjectCarouselPackager(); bool add(const PackageItem &item); diff --git a/src/mbstf/ObjectListController.cc b/src/mbstf/ObjectListController.cc index 44ffbd3..c6bdec2 100644 --- a/src/mbstf/ObjectListController.cc +++ b/src/mbstf/ObjectListController.cc @@ -79,7 +79,8 @@ void ObjectListController::setObjectPackager() { in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); unsigned short mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, GET_MTU_ETHERNET_PAYLOAD) - GTP_HEADER_SIZE; const auto &obj_list = object_store->getObjects(); - packager(new ObjectListPackager(object_store, *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port)); + packager(new ObjectListPackager(object_store, *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port, + distributionSession().getFecInformation())); // Send all objects that are in the ObjectStore for (const auto &[obj_id, object] : obj_list) { sendToPackager(object); diff --git a/src/mbstf/ObjectListPackager.cc b/src/mbstf/ObjectListPackager.cc index c2276f6..f2d2df2 100644 --- a/src/mbstf/ObjectListPackager.cc +++ b/src/mbstf/ObjectListPackager.cc @@ -22,6 +22,8 @@ #include #include "ogs-app.h" // ogs_error(), ogs_info() + +#include "FecOtiHelper.hh" #include "ogs-sbi.h" #include "Transmitter.h" // LibFlute @@ -113,8 +115,9 @@ ObjectListPackager::ObjectListPackager(const std::shared_ptr &objec ObjectListPackager::ObjectListPackager(const std::shared_ptr &object_store, ObjectController &controller, const SsmPort &ssm_port, uint32_t rateLimit, unsigned short mtu, - const std::optional &tunnel_address, in_port_t tunnel_port) - :ObjectPackager(object_store, controller, ssm_port, rateLimit, mtu, tunnel_address, tunnel_port) + const std::optional &tunnel_address, in_port_t tunnel_port, + const std::optional> &fec_information) + :ObjectPackager(object_store, controller, ssm_port, rateLimit, mtu, tunnel_address, tunnel_port, fec_information) ,m_packageItemsMutex (new decltype(m_packageItemsMutex)::element_type) ,m_packageItems() ,m_tunnelEndpoint() @@ -199,6 +202,20 @@ void ObjectListPackager::doObjectPackage() { std::lock_guard lock(*m_transmitterMutex); if (!m_transmitter) { + /* The Distribution Session's own requested AL-FEC configuration, converted to the + Transmitter-level FEC OTI. Without this the session is sent unprotected however it + was provisioned. fecOtiFromFecConfig() rejects a scheme the MBMS Download Profile + does not admit, which is a packaging failure for this session rather than a reason + to send it without the protection it asked for. */ + std::optional content_fec_oti; + uint32_t fec_redundancy_level = LibFlute::kDefaultFecRedundancyLevel; + try { + std::tie(content_fec_oti, fec_redundancy_level) = fecOtiFromFecConfig(fecInformation()); + } catch (const std::runtime_error &err) { + ogs_error("Cannot apply the Distribution Session's FEC configuration, not transmitting: %s", + err.what()); + return; + } m_transmitter.reset(new LibFlute::Transmitter( ssm_port.destinationAddress(), static_cast(ssm_port.port()), @@ -209,7 +226,10 @@ void ObjectListPackager::doObjectPackage() { m_tunnelEndpoint, LibFlute::FileDeliveryTable::FDT_NS_DRAFT_2005, true, - ssm_port.sourceAddress())); + ssm_port.sourceAddress(), + content_fec_oti, + LibFlute::Profile::Ts26517, + fec_redundancy_level)); m_transmitter->register_completion_callback( [this](uint32_t toi) { ogs_debug("FLUTE Transmitter has %zu files left, packager has %zu files left", m_transmitter->number_of_files(), m_packageItems.size()); diff --git a/src/mbstf/ObjectListPackager.hh b/src/mbstf/ObjectListPackager.hh index 79ff6c8..9e18de0 100644 --- a/src/mbstf/ObjectListPackager.hh +++ b/src/mbstf/ObjectListPackager.hh @@ -95,7 +95,8 @@ public: const std::optional &tunnel_address, in_port_t tunnel_port); ObjectListPackager(const std::shared_ptr &object_store, ObjectController &controller, const SsmPort &ssm_port, uint32_t rateLimit, unsigned short mtu, const std::optional &tunnel_address, - in_port_t tunnel_port); + in_port_t tunnel_port, + const std::optional> &fec_information = std::nullopt); virtual ~ObjectListPackager(); bool add(const PackageItem &item); diff --git a/src/mbstf/ObjectPackager.hh b/src/mbstf/ObjectPackager.hh index 7c954f5..fe25f27 100644 --- a/src/mbstf/ObjectPackager.hh +++ b/src/mbstf/ObjectPackager.hh @@ -28,6 +28,10 @@ #include "SsmPort.hh" #include "SubscriptionService.hh" +namespace reftools::mbstf { + class FECConfig; +} + namespace LibFlute{ class Transmitter; } @@ -126,12 +130,12 @@ public: ObjectPackager(ObjectPackager &&) = delete; ObjectPackager(const ObjectPackager &) = delete; - ObjectPackager(const std::shared_ptr &objectStore, ObjectController &controller, const SsmPort &ssm_port = SsmPort(), uint32_t rateLimit = 0, unsigned short mtu = 0, const std::optional &tunnel_address = std::nullopt, in_port_t tunnel_port = 0 ) + ObjectPackager(const std::shared_ptr &objectStore, ObjectController &controller, const SsmPort &ssm_port = SsmPort(), uint32_t rateLimit = 0, unsigned short mtu = 0, const std::optional &tunnel_address = std::nullopt, in_port_t tunnel_port = 0 , const std::optional> &fec_information = std::nullopt) :m_transmitterMutex(new decltype(m_transmitterMutex)::element_type) ,m_transmitter(nullptr), m_io(), m_queuedToi(0), m_queued(false), m_deactivating(false), m_queuedObjectId() ,m_objectStore(objectStore), m_controller(controller), m_ssmPort(ssm_port), m_rateLimit(rateLimit), m_mtu(mtu) ,m_workerThread(), m_workerCancel(false), m_workerRunning(false) - ,m_tunnelAddress(tunnel_address), m_tunnelPort(tunnel_port) + ,m_tunnelAddress(tunnel_address), m_tunnelPort(tunnel_port), m_fecInformation(fec_information) { }; @@ -170,6 +174,11 @@ protected: uint64_t tsi() const; in_port_t tunnelPort() const { return m_tunnelPort; }; + /** The MBS Distribution Session's own requested AL-FEC configuration (TS 29.580 V18.8.0 + * clause 6.2.6.2.14 FECConfig, carried as DistSession.fecInformation), or unset when the + * session did not request FEC. Absent for every packager until a caller passes one. */ + const std::optional> &fecInformation() const { return m_fecInformation; }; + virtual void doObjectPackage() = 0; std::shared_ptr m_transmitterMutex; @@ -192,6 +201,7 @@ private: std::atomic_bool m_workerRunning; std::optional m_tunnelAddress; in_port_t m_tunnelPort; + std::optional> m_fecInformation; }; MBSTF_NAMESPACE_STOP diff --git a/src/mbstf/ObjectStreamingController.cc b/src/mbstf/ObjectStreamingController.cc index 3b7d66e..dc32902 100644 --- a/src/mbstf/ObjectStreamingController.cc +++ b/src/mbstf/ObjectStreamingController.cc @@ -72,7 +72,8 @@ void ObjectStreamingController::setObjectPackager() uint32_t rate_limit = distributionSession().getRateLimit(); in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); unsigned short mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, GET_MTU_ETHERNET_PAYLOAD) - GTP_HEADER_SIZE; - packager(new ObjectListPackager(objectStore(), *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port)); + packager(new ObjectListPackager(objectStore(), *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port, + distributionSession().getFecInformation())); auto pkgr = getObjectListPackager(); subscribeToService(*pkgr); startWorker(); diff --git a/src/mbstf/meson.build b/src/mbstf/meson.build index 5750495..2a19e07 100644 --- a/src/mbstf/meson.build +++ b/src/mbstf/meson.build @@ -63,6 +63,13 @@ test_source_object_list_packager = test_source_object_store + files(''' ObjectListPackager.hh '''.split()) +test_source_fec_oti_helper = files(''' + FecOtiHelper.cc + FecOtiHelper.hh + openapi/model/FECConfig.cc + openapi/model/FECConfig.h + '''.split()) + test_source_pull_object_ingester = test_source_object_store + files(''' PullObjectIngester.cc PullObjectIngester.hh @@ -117,6 +124,8 @@ libmbstf_dist_sources = files(''' MimeContentType.hh NfServer.cc NfServer.hh + FecOtiHelper.cc + FecOtiHelper.hh ObjectCarouselController.cc ObjectCarouselController.hh ObjectCarouselPackager.cc diff --git a/tests/meson.build b/tests/meson.build index 03e810c..3c21d9c 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -26,6 +26,10 @@ test('test_pull_object_ingester', executable('testPullObjectIngester', 'test_PullObjectIngester.cc', test_source_pull_object_ingester, install:false, include_directories:[libmbstf_libinc, libinc], dependencies : [libmbstf_dep]) ,verbose: true, timeout: 600, protocol: 'exitcode') +test('test_fec_oti_helper', + executable('testFecOtiHelper', 'test_FecOtiHelper.cc', test_source_fec_oti_helper, install:false, include_directories:[libmbstf_libinc, libinc], dependencies : [libmbstf_dep, boost_dep, rt_libflute_dep, libuuid_dep]) + ,verbose: true, timeout: 600, protocol: 'exitcode') + test('test_object_list_packager', executable('testObjectListPackager', 'test_ObjectListPackager.cc', test_source_object_list_packager, install:false, include_directories:[libmbstf_libinc, libinc], dependencies : [libmbstf_dep, boost_dep, rt_libflute_dep, libuuid_dep]) ,verbose: true, timeout: 600, protocol: 'exitcode') diff --git a/tests/test_FecOtiHelper.cc b/tests/test_FecOtiHelper.cc new file mode 100644 index 0000000..5d4f2e7 --- /dev/null +++ b/tests/test_FecOtiHelper.cc @@ -0,0 +1,117 @@ +/***************************************************************************** + * 5G-MAG Reference Tools: MBS Transport Function: FecOtiHelper tests + ***************************************************************************** + * License: 5G-MAG Public License v1 + * + * For full license terms please see the LICENSE file distributed with this + * program. If this file is missing then the license can be retrieved from + * https://drive.google.com/file/d/1cinCiA778IErENZ3JN52VFW-1ffHpx7Z/view + */ + +#include +#include +#include +#include +#include + +#include "FecOtiHelper.hh" +#include "openapi/model/FECConfig.h" + +MBSTF_NAMESPACE_USING; + +static int pass = 0; +static int fail = 0; + +static void check(bool ok, const std::string &name) +{ + if (ok) { pass++; std::cout<<"INFO: "<> makeConfig(const std::string &scheme, int32_t overhead) +{ + auto cfg = std::make_shared(); + cfg->setFecScheme(scheme); + cfg->setFecOverHead(overhead); + return std::optional>(cfg); +} + +/* A session that asked for no FEC must produce no FEC OTI, so the Transmitter keeps its own + default behaviour rather than being handed an empty-but-present configuration. */ +static void testAbsentConfig() +{ + auto [oti, redundancy] = fecOtiFromFecConfig(std::nullopt); + check(!oti.has_value() && redundancy == LibFlute::kDefaultFecRedundancyLevel, + "testAbsentConfig"); + + std::optional> null_ptr_config(nullptr); + auto [oti2, redundancy2] = fecOtiFromFecConfig(null_ptr_config); + check(!oti2.has_value() && redundancy2 == LibFlute::kDefaultFecRedundancyLevel, + "testAbsentConfig null shared_ptr"); +} + +/* Compact No-Code carries no repair symbols, so it has the same observable effect as no FEC and + must not be turned into a FEC OTI the Transmitter would act on. */ +static void testCompactNoCode() +{ + auto [oti, redundancy] = fecOtiFromFecConfig(makeConfig("urn:ietf:rmt:fec:encoding:0", 20)); + check(!oti.has_value(), "testCompactNoCode yields no FEC OTI"); + (void)redundancy; +} + +/* Raptor is the one repair scheme this MBSTF wires. The requested overhead becomes the + Transmitter's redundancy level; the symbol geometry is left for rt-libflute to derive. */ +static void testRaptor() +{ + auto [oti, redundancy] = fecOtiFromFecConfig(makeConfig("urn:ietf:rmt:fec:encoding:1", 25)); + check(oti.has_value() && oti->encoding_id == LibFlute::FecScheme::Raptor, + "testRaptor yields a Raptor FEC OTI"); + check(redundancy == 25u, "testRaptor carries the requested overhead as the redundancy level"); +} + +/* TS 26.346 V18.2.0 clause L.4.7 admits only Compact No-Code and Raptor into the MBMS Download + Profile, which TS 26.517 V18.6.0 clause 6.2.1 requires this session to conform to. RaptorQ + (RFC 6330) is not admitted, so it must be refused rather than silently downgraded to no FEC: + a session sent unprotected when it asked for protection is a worse outcome than a failure. */ +static void testRaptorQRefused() +{ + bool threw = false; + try { fecOtiFromFecConfig(makeConfig("urn:ietf:rmt:fec:encoding:6", 20)); } + catch (const std::runtime_error &) { threw = true; } + check(threw, "testRaptorQRefused"); +} + +static void testUnknownSchemeRefused() +{ + bool threw = false; + try { fecOtiFromFecConfig(makeConfig("urn:example:not-a-fec-scheme", 20)); } + catch (const std::runtime_error &) { threw = true; } + check(threw, "testUnknownSchemeRefused"); +} + +/* A negative overhead cannot be turned into an unsigned redundancy level without wrapping to an + enormous value, so it is rejected at the boundary rather than converted. */ +static void testNegativeOverheadRefused() +{ + bool threw = false; + try { fecOtiFromFecConfig(makeConfig("urn:ietf:rmt:fec:encoding:1", -1)); } + catch (const std::runtime_error &) { threw = true; } + check(threw, "testNegativeOverheadRefused"); +} + +int main() +{ + std::cout<<"### FecOtiHelper: Test start #### "< Date: Sat, 5 Sep 2026 18:24:44 +0200 Subject: [PATCH 06/19] mbstf: build the COLLECTION operating mode's controller, and give the DASH handler its own manifest URL Problem ObjectCollectionController.cc/.hh, 368 lines serving the COLLECTION object distribution operating mode, were present in the tree but never listed in src/mbstf/meson.build, so they were never compiled. The class self-registers with ControllerFactory at file scope, so leaving it out of the build means the registration never runs and a Distribution Session provisioned with objDistributionOperatingMode COLLECTION finds no controller at all. Once added to the build it did not compile: four errors, all from ObjectController::objectStore() having become std::shared_ptr rather than a reference. The same file listed ObjectController.cc twice where the second entry should have been its header, so the header was not tracked as a dependency. [code-derived] Separately, DASHManifestHandler::nextIngestItems() declared manifest_url and never assigned it, so both comparisons against it tested the empty string and m_refreshMpd was never set: a re-fetched MPD was ingested without the handler being told its own manifest had changed. [code-derived] Basis COLLECTION is one of the four values ObjDistributionOperatingMode defines (src/mbstf/openapi/model/ObjDistributionOperatingMode.h: SINGLE, COLLECTION, CAROUSEL, STREAMING), so the API accepts a session the build then has no controller for. That is the operative basis and it is code-derived. TS 26.502 V18.6.0, clause 4.5.10, first paragraph: "An object manifest describes a set of objects to be distributed in an MBS Distribution Session that is provisioned in OBJECT_COLLECTION or OBJECT_CAROUSEL operating mode." confirming the mode is one the specification defines rather than a local invention. The fuller description of the mode is in annex B, clause B.2.1, which is informative. Raised by Reading the branch against its own build file while checking the review comments on pull request #71. Rule 14: purpose established before any change, and the code is built rather than removed, because the operating mode it serves is one the model defines. Change Adds ObjectCollectionController to src/mbstf/meson.build and corrects the duplicated ObjectController.cc entry to name the header. Dereferences the object-store shared pointer at the four sites that still treated it as a reference, matching what ObjectCarouselController already does, and checks each dynamic_pointer_cast result before use rather than letting a std::runtime_error leave the controller, where the surrounding try catches only std::out_of_range and one bad ingest response would end the process and every other Distribution Session with it. Initialises manifest_url from the manifest's own fetched URL, the value addMPDRefreshToExtraPullObjects() keys its refresh entry on, and makes it const. Verification T0 for the new code: both files compile with no new warnings and COLLECTION is present in the linked binary, so the controller registers. T1: all five suites pass, 50 cases. T2: the demo delivers with this build, gNB carrying MRB1 and MRB2 and the UE logging CRC-OK broadcast decodes and MCCH receptions. Not in this change Any verification of COLLECTION behaviour. This builds and registers the controller; it does not establish that the mode works, and the conformance record must not claim it. The demo uses STREAMING and no test covers COLLECTION. Nothing here covers the MPD-refresh recognition path either. --- src/mbstf/DASHManifestHandler.cc | 6 +- src/mbstf/ObjectCollectionController.cc | 274 ++++++++++++++++++++++++ src/mbstf/ObjectCollectionController.hh | 94 ++++++++ src/mbstf/meson.build | 8 +- 4 files changed, 378 insertions(+), 4 deletions(-) create mode 100644 src/mbstf/ObjectCollectionController.cc create mode 100644 src/mbstf/ObjectCollectionController.hh diff --git a/src/mbstf/DASHManifestHandler.cc b/src/mbstf/DASHManifestHandler.cc index 30f6288..927ec4a 100644 --- a/src/mbstf/DASHManifestHandler.cc +++ b/src/mbstf/DASHManifestHandler.cc @@ -123,7 +123,11 @@ std::pair DASHManifest static const std::string empty; auto current_time = std::chrono::system_clock::now(); std::optional time_to_update; - std::string manifest_url; + /* The manifest's own URL, so the two comparisons below can recognise the MPD-refresh entry + addMPDRefreshToExtraPullObjects() adds to m_extraPullObjects from the same value. Left empty, + both comparisons test against "" and m_refreshMpd is never set, so a re-fetched MPD is + ingested without the handler being told its own manifest changed. */ + const std::string manifest_url(m_manifest ? m_manifest->second.getFetchedUrl() : std::string()); time_type fetch_time; std::list media_segments; diff --git a/src/mbstf/ObjectCollectionController.cc b/src/mbstf/ObjectCollectionController.cc new file mode 100644 index 0000000..1989fda --- /dev/null +++ b/src/mbstf/ObjectCollectionController.cc @@ -0,0 +1,274 @@ +/****************************************************************************** + * 5G-MAG Reference Tools: MBS Transport Function: ObjectCollectionController class + ****************************************************************************** + * Copyright: (C)2026 British Broadcasting Corporation + * License: 5G-MAG Public License v1 + * + * For full license terms please see the LICENSE file distributed with this + * program. If this file is missing then the license can be retrieved from + * https://drive.google.com/file/d/1cinCiA778IErENZ3JN52VFW-1ffHpx7Z/view + */ + +#include +#include +#include +#include +#include +#include + +#include + +#include + +#include "ogs-app.h" +#include "ogs-sbi.h" // include before "common.hh" to ensure correct logging domain + +#include "common.hh" +#include "ControllerFactory.hh" +#include "DistributionSession.hh" +#include "Event.hh" +#include "ManifestHandlerFactory.hh" +#include "ObjectController.hh" +#include "ObjectListPackager.hh" +#include "ObjectManifestHandler.hh" +#include "ObjectStore.hh" +#include "PullObjectIngester.hh" +#include "PushObjectIngester.hh" +#include "SsmPort.hh" +#include "SubscriptionService.hh" +#include "utilities.hh" +#include "openapi/model/DistSessionState.h" +#include "openapi/model/Object.h" +#include "openapi/model/ProblemCause.hh" + +#include "ObjectCollectionController.hh" + +using reftools::mbstf::DistSessionState; +using reftools::mbstf::Object; +using fiveg_mag_reftools::ModelException; +using fiveg_mag_reftools::ProblemCause; + +MBSTF_NAMESPACE_START + +static void validate_distribution_session(DistributionSession &distribution_session); +static bool check_if_object_added_is_manifest(const std::shared_ptr &object, std::string &manifest_url); +static bool check_if_object_is_active_in_manifest(const std::shared_ptr &object, const std::shared_ptr &manifest_handler); +static void finish_request_in_manifest_handler(const std::shared_ptr &object, const std::shared_ptr &manifest_handler); + +ObjectCollectionController::ObjectCollectionController(DistributionSession &distribution_session) + :ObjectManifestController(distribution_session) +{ + ogs_debug("ObjectCollectionController validating DistributionSession"); + validate_distribution_session(distribution_session); + ogs_debug("ObjectCollectionController subscribe to ObjectStore"); + subscribeToService(*objectStore()); + ogs_debug("ObjectCollectionController active"); +} + +ObjectCollectionController::~ObjectCollectionController() +{ + abort(); +} + +void ObjectCollectionController::setObjectPackager() +{ + auto ssm_port = distributionSession().getSsmPort(); + const std::optional &tunnel_addr = distributionSession().getTunnelAddr(); + uint32_t rate_limit = distributionSession().getRateLimit(); + in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); + unsigned short mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, GET_MTU_ETHERNET_PAYLOAD) - GTP_HEADER_SIZE; + auto fec_information = distributionSession().getFecInformation(); + packager(new ObjectListPackager(objectStore(), *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port, fec_information)); + auto pkgr = getObjectListPackager(); + subscribeToService(*pkgr); + startWorker(); + // Catch up on anything the manifest already lists, in case it (and some of its objects) were + // already ingested before the packager existed -- mirrors ObjectCarouselController's own + // updateCarousel() call here, without the diff/removal half that mode needs and this one does + // not (see populateFromManifest()'s own comment). + populateFromManifest(); +} + +void ObjectCollectionController::unsetObjectPackager() +{ + packager(nullptr); +} + +void ObjectCollectionController::activateObjectPackager() { + packager()->activate(); + startWorker(); +} + +void ObjectCollectionController::deactivateObjectPackager() { + if (packager()->deactivate()) { + distributionSession().haveEmptyQueue(); + } +} + +std::shared_ptr ObjectCollectionController::getObjectListPackager() const +{ + return std::dynamic_pointer_cast(packager()); +} + +void ObjectCollectionController::processEvent(Event &event, SubscriptionService &event_service) +{ + if (event.eventName() == ObjectStore::ObjectAddedEvent::event_name || + event.eventName() == ObjectStore::ObjectUpdatedEvent::event_name) { + + ObjectStore::ObjectChangedEvent &obj_added_event = dynamic_cast(event); + std::string object_id = obj_added_event.objectId(); + ogs_debug("%s with ID: %s", event.eventName().c_str(), object_id.c_str()); + try { + const std::shared_ptr &object = (*objectStore())[object_id]; + ogs_debug("Object location: %s", object->second.getFetchedUrl().c_str()); + object->second.keepAfterSend(true); /* keep all objects; nothing here ever removes one */ + if (check_if_object_added_is_manifest(object, getManifestUrl())) { + if (manifestHandler()) { + try { + if (!manifestHandler()->update(object)) { + ogs_error("Failed to update Manifest"); + unsetObjectListPackager(); + event.stopProcessing(); + return; + } + startWorker(); + } catch (std::exception &ex) { + ogs_error("Invalid Manifest update: %s", ex.what()); + unsetObjectListPackager(); + event.stopProcessing(); + return; + } + } else { + std::shared_ptr manifest_handler(ManifestHandlerFactory::makeManifestHandler(object, this, distributionSession().getObjectAcquisitionMethod() == "PULL")); + if (!manifest_handler) { + // No registered handler recognises this object's media type as a + // manifest: the ingest source served an unexpected Content-Type, or the + // format is not one this build supports. The surrounding try catches only + // std::out_of_range, so a std::runtime_error raised here would leave the + // process and take down every other Distribution Session over one bad + // ingest response for this one session. Give up on this session's manifest + // the same way an update failure two branches above does. + ogs_error("Could not find suitable manifest handler for object %s", object_id.c_str()); + unsetObjectListPackager(); + event.stopProcessing(); + return; + } + manifestHandler(std::move(manifest_handler)); + } + populateFromManifest(); + } else if (check_if_object_is_active_in_manifest(object, manifestHandler())) { + finish_request_in_manifest_handler(object, manifestHandler()); + sendToPackager(object); + } + } catch (std::out_of_range &ex) { + ogs_error("Object %s is not in the ObjectStore", object_id.c_str()); + } + } + ObjectManifestController::processEvent(event, event_service); +} + +void ObjectCollectionController::sendToPackager(const std::shared_ptr &object) +{ + auto packager = getObjectListPackager(); + if (packager) { + ObjectListPackager::PackageItem item(object); + packager->add(item); + } +} + +const std::optional &ObjectCollectionController::getObjectDistributionBaseUrl() const { + return distributionSession().objectDistributionBaseUrl(); +} + +void ObjectCollectionController::reconfigureObjectPackager() +{ + if (distributionSession().getState() == DistSessionState::VAL_ACTIVE) { + auto packager = getObjectListPackager(); + if (packager) { + auto ssm_port = distributionSession().getSsmPort(); + const std::optional &tunnel_addr = distributionSession().getTunnelAddr(); + uint32_t rate_limit = distributionSession().getRateLimit(); + in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); + + if (ssm_port) { + packager->updateFluteInfo(ssm_port, rate_limit, tunnel_addr, tunnel_port); + } + } else { + setObjectPackager(); + } + } +} + +void ObjectCollectionController::populateFromManifest() +{ + auto object_manifest_hndlr = std::dynamic_pointer_cast(manifestHandler()); + if (!object_manifest_hndlr) return; + const auto &manifest_objects = object_manifest_hndlr->getObjects(); + + const auto &packager = getObjectListPackager(); + if (!packager) return; + + for (const auto &obj : manifest_objects) { + if (obj && obj.value()) { + const auto obj_metadata = objectStore()->findMetadataByURL(obj.value()->getLocator()); + if (obj_metadata) { + sendToPackager((*objectStore())[obj_metadata->objectId()]); + } + /* else not yet ingested -- the scheduled pull worker will fetch it, and its own + ObjectAddedEvent will reach processEvent() above and queue it then */ + } + } +} + +namespace { +static const struct init { + init() { + ControllerFactory::registerController(new ControllerConstructor); + }; +} g_init; +} + +static void validate_distribution_session(DistributionSession &distribution_session) +{ + if (distribution_session.getObjectDistributionOperatingMode() != "COLLECTION") { + throw std::logic_error("Expected objDistributionOperatingMode to be set to COLLECTION."); + } + ObjectController::validateDistributionSession(distribution_session); +} + +static bool check_if_object_added_is_manifest(const std::shared_ptr &object, std::string &manifest_url) +{ + auto &metadata = object->second; + return (metadata.getOriginalUrl() == manifest_url || metadata.getFetchedUrl() == manifest_url); +} + +// dynamic_pointer_cast returns null when the manifest handler was constructed as a different +// ManifestHandler subclass (a DASH MPD giving a DASHManifestHandler, say), so the result is +// checked before use. ObjectCarouselController.cc holds an identical copy of these two functions +// and the same reasoning applies there. +static bool check_if_object_is_active_in_manifest(const std::shared_ptr &object, const std::shared_ptr &manifest_handler) +{ + const auto object_manifest_hndlr = std::dynamic_pointer_cast(manifest_handler); + if (!object_manifest_hndlr) { + ogs_error("Manifest handler is not an ObjectManifestHandler (object %s); treating as not active", + object->second.objectId().c_str()); + return false; + } + return object_manifest_hndlr->isObjectURLActive(object->second.getOriginalUrl()); +} + +static void finish_request_in_manifest_handler(const std::shared_ptr &object, const std::shared_ptr &manifest_handler) +{ + auto object_manifest_hndlr = std::dynamic_pointer_cast(manifest_handler); + if (!object_manifest_hndlr) { + ogs_error("Manifest handler is not an ObjectManifestHandler (object %s); cannot finish request", + object->second.objectId().c_str()); + return; + } + object_manifest_hndlr->finishRequest(object->second.getOriginalUrl()); +} + +MBSTF_NAMESPACE_STOP + +/* vim:ts=8:sts=4:sw=4:expandtab: + */ diff --git a/src/mbstf/ObjectCollectionController.hh b/src/mbstf/ObjectCollectionController.hh new file mode 100644 index 0000000..42a65d1 --- /dev/null +++ b/src/mbstf/ObjectCollectionController.hh @@ -0,0 +1,94 @@ +#ifndef _MBS_TF_OBJECT_COLLECTION_CONTROLLER_HH_ +#define _MBS_TF_OBJECT_COLLECTION_CONTROLLER_HH_ +/****************************************************************************** + * 5G-MAG Reference Tools: MBS Transport Function: Object Collection Controller class + ****************************************************************************** + * Copyright: (C)2026 British Broadcasting Corporation + * License: 5G-MAG Public License v1 + * + * For full license terms please see the LICENSE file distributed with this + * program. If this file is missing then the license can be retrieved from + * https://drive.google.com/file/d/1cinCiA778IErENZ3JN52VFW-1ffHpx7Z/view + */ + +#include +#include +#include + +#include "common.hh" +#include "openapi/model/ObjDistributionData.h" +#include "ObjectManifestController.hh" +#include "ObjectStore.hh" + +MBSTF_NAMESPACE_START + +class DistributionSession; +class Event; +class ObjectListPackager; +class PullObjectIngester; +class SubscriptionService; +class ObjectManifestController; + +// TS 26.517 V18.6.0 cl.6.2.3.3: "Object collection operating mode (OBJECT_COLLECTION) refers to +// the case in which multiple objects are distributed via the Object Distribution Method. The list +// of objects to be distributed is described by an object manifest document as specified in clause +// 6.1.2. The objects listed in the manifest are distributed only once." -- the manifest format and +// ingestion mechanics are the same as OBJECT_CAROUSEL (both use TS26517_MBSObjectManifest.yaml, and +// ObjectManifestController's shared machinery already only re-fetches items the manifest itself +// schedules; a manifest with no repetition/check-interval, as this mode's own manifest fields are +// documented to have "ignored" for it, is simply fetched once). What differs from Carousel is the +// packager: Carousel repeats delivery of each object indefinitely (ObjectCarouselPackager); this +// mode delivers each object once, matching ObjectListPackager's own model, which O5's own fix +// already described as covering "the single, collection and streaming modes". +class ObjectCollectionController : public ObjectManifestController { +public: + ObjectCollectionController() = delete; + ObjectCollectionController(DistributionSession&); + ObjectCollectionController(const ObjectCollectionController&) = delete; + ObjectCollectionController(ObjectCollectionController&&) = delete; + + virtual ~ObjectCollectionController(); + + ObjectCollectionController &operator=(const ObjectCollectionController&) = delete; + ObjectCollectionController &operator=(ObjectCollectionController&&) = delete; + + std::shared_ptr getObjectListPackager() const; + const std::optional &getObjectDistributionBaseUrl() const; + + static unsigned int factoryPriority() { return 50; }; + + // Subscriber virtual methods + virtual void processEvent(Event &event, SubscriptionService &event_service); + + std::string reprString() const { + std::ostringstream os; + os << "ObjectCollectionController(controller =" << this << ")"; + return os.str(); + } + + void unsetObjectListPackager() { + packager(nullptr); + }; + + virtual void reconfigureObjectPackager(); + +protected: + virtual void setObjectPackager(); + virtual void unsetObjectPackager(); + virtual void activateObjectPackager(); + virtual void deactivateObjectPackager(); + +private: + void sendToPackager(const std::shared_ptr &object); + // Queues every object the manifest currently lists that has already been ingested. Unlike + // Carousel's updateCarousel(), this never removes anything: the manifest is fetched once (per + // the clause above), not periodically re-checked for changes, so there is nothing to diff + // against on a later pass. + void populateFromManifest(); +}; + +MBSTF_NAMESPACE_STOP + +/* vim:ts=8:sts=4:sw=4:expandtab: + */ +#endif /* _MBS_TF_OBJECT_COLLECTION_CONTROLLER_HH_ */ diff --git a/src/mbstf/meson.build b/src/mbstf/meson.build index 2a19e07..dd39c4e 100644 --- a/src/mbstf/meson.build +++ b/src/mbstf/meson.build @@ -124,14 +124,16 @@ libmbstf_dist_sources = files(''' MimeContentType.hh NfServer.cc NfServer.hh - FecOtiHelper.cc - FecOtiHelper.hh ObjectCarouselController.cc ObjectCarouselController.hh ObjectCarouselPackager.cc ObjectCarouselPackager.hh + FecOtiHelper.cc + FecOtiHelper.hh + ObjectCollectionController.cc + ObjectCollectionController.hh ObjectController.cc - ObjectController.cc + ObjectController.hh ObjectListController.cc ObjectListController.hh ObjectManifestController.cc From e612b464a09883e8eca73564f43c6b7ce790a51a Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sat, 5 Sep 2026 23:03:13 +0200 Subject: [PATCH 07/19] comments: state behaviour, and drop references a reader outside the project cannot follow Problem Comments across this branch narrated how the code came to be rather than what it does, and pointed at material a reviewer cannot read. Three shapes. A "BUG FIX:" prefix followed by what the code used to do ("this used to be static", "no DELETE branch existed at all", "the else branch used to unconditionally call gw->write_pdu_mch()"). That belongs in a commit message; in a comment it dates immediately and tells a reader nothing about the code in front of them. References to this project's own internal process framework by number: "(rule 12)", "(S12)", "see rule 14". Those numbers name nothing in this repository. Pointers into a separate, private repository: "see Standards2Deployments/projects/rt-mbs/...", "see the findings register", "the register's own item 4 finding". A reviewer cannot open any of them. [code-derived] Basis No clause governs a comment. code-derived only. Raised by Reading the branch as a reviewer outside this project would. Change Comments now state the behaviour, the requirement or the invariant, keeping every specification citation and every stated reason. Where a comment's only content was its history, the underlying rule it was protecting is stated instead: "X was never freed" becomes "this context owns X and must free it on removal". Internal rule numbers are replaced by the reasoning they stood for, usually that no clause and no configured value fixes a given bound. Private-repository pointers are removed, with the substance they referred to summarised in place where it was load-bearing. Deliberately NOT changed: the numbered rules in src/mbsf/MultipartMime.cc's Q-encoding comment. Those are RFC 2047 section 4.2's own rules 1, 2 and 3, not this project's, and an automated pass over the phrase "rule N" would have silently corrupted a correct citation. Verification T0 for the shape of the change: every hunk is a comment or documentation line, confirmed by filtering the diff for added lines that are not comments, which comes to zero in all nine repositories. T1 where a build exists: open5gs, rt-mbs-function and rt-mbs-transport-function all build clean afterwards, and the shell scripts and JSON touched pass bash -n and json.load. Not in this change No behaviour, in any repository. No commit message is rewritten; the history those comments described stays in the log, which is where it belongs. --- src/mbstf/Context.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mbstf/Context.hh b/src/mbstf/Context.hh index a6229ab..7a7342b 100644 --- a/src/mbstf/Context.hh +++ b/src/mbstf/Context.hh @@ -83,7 +83,7 @@ public: std::optional manifestRepetitionRate = std::nullopt; } manifestGlobals; //< ManifestHandler global configuration (can be overridden by ManifestHandler implement specific config) // TS 29.500 V18.10.0 cl.5.2.7.2/table 5.2.7.1-1: 413 (Payload Too Large) is mandatory for - // PATCH and POST. No clause, and no MBSTF documented default, names a byte limit (rule 12) + // PATCH and POST. No clause, and no MBSTF documented default, names a byte limit // -- unset means no limit is enforced, as before this option existed. std::optional maxRequestBodySize; From 4b9490242a659618974164f7c287fa85913bed1b Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 6 Sep 2026 08:20:23 +0200 Subject: [PATCH 08/19] mbstf: stop refetching an object past its latest fetch time, or past the configured failure limit Problem ObjectManifestController refetched an object on every ingest failure, for any non-PUSH session, with no bound of any kind: no deadline check, no attempt count, no backoff. One object that could never be fetched was therefore retried forever. Observed on a live run: 59554 ingest failures in a single session, none of which stopped anything. [observed: run/logs/mbstf.log] The session-wide limit that exists could not catch it. ObjectController counts consecutive failures and deactivates the session at consecutiveIngestFailuresBeforeDeactivate, but the count is per Distribution Session and is reset by ObjectStore's ObjectAdded/ObjectUpdated events. The same run recorded 4985 successful object fetches, so the counter was reset continually and never reached 5 while one object failed 59554 times. A bound that a healthy session resets cannot bound a single unfetchable object. [code-derived: ObjectController.cc, the reset at the ObjectAdded/ObjectUpdated branch] Basis TS 26.517 V18.6.0, clause 6.1.2, object manifest parameter latestFetchTime: "The MBSTF shall fetch the object no later than this UTC timestamp." So an object whose latest fetch time has passed must not be fetched again, whatever has happened before. The same parameter's description governs the other case: when latestFetchTime is absent "the object shall be present at its origin ... and the MBSTF may fetch it at a time of its choosing", which sets no bound at all. A limit there therefore rests on a configuration option the operator sets, not on a clause. Raised by Reading the authority for what governs a failed object fetch, after a defensive guard written earlier was measured and found not to bound the loop (it rejected the fetch, which produced the failure event, which triggered the refetch). Change The refetch decision now refuses two cases. An object whose latestFetchTime has passed is not refetched, which is the clause above. An object with no latestFetchTime is refetched until consecutiveIngestFailuresBeforeDeactivate consecutive failures of that object, the operator's own existing option applied per object rather than per session. PullObjectIngester::IngestItem gains the per-object counter, carried through all four of its copy and move paths, since items are copied and moved on every queueing path. Verification T1: tests/test_PullObjectIngester.cc, 27 cases passing, 5 of them new: the counter starts at zero, accumulates, and survives copy and move; a deadline in the past, a deadline in the future and no deadline are each distinguished. Confirmed discriminating, not vacuous: with the copy and move carry deliberately reverted the two survival cases fail and the suite reports 25 pass, 2 fail. All five MBSTF suites pass, 58 cases. T2: live demo. Ingest failures fell from 59554 to 15, and the log shows three refusals, each "5 consecutive fetch failures reached the configured consecutiveIngestFailuresBeforeDeactivate limit of 5": three objects, five attempts each, then stopped. Delivery is unaffected, the UE logging 15711 CRC-OK broadcast decodes on the same run. Not in this change Why the ingest URL is corrupted in the first place. A one-byte corruption at index 0 of IngestItem's own copy of the fetched URL is still unexplained and still open; this bounds the retry loop that made it harmful, it does not fix it. The refusal log line shows an empty object id for the affected items, which is consistent with that corruption and is not diagnosed here. No backoff is introduced: the clause names no interval and no configuration option supplies one. --- src/mbstf/ObjectManifestController.cc | 33 ++++++++++++++++++-- src/mbstf/PullObjectIngester.cc | 4 +++ src/mbstf/PullObjectIngester.hh | 12 +++++++ tests/test_PullObjectIngester.cc | 45 ++++++++++++++++++++++++++- 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/mbstf/ObjectManifestController.cc b/src/mbstf/ObjectManifestController.cc index e4d9d05..cac8904 100644 --- a/src/mbstf/ObjectManifestController.cc +++ b/src/mbstf/ObjectManifestController.cc @@ -37,6 +37,8 @@ #include "utilities.hh" #include "openapi/model/DistSessionState.h" +#include "App.hh" +#include "Context.hh" #include "ObjectManifestController.hh" using reftools::mbstf::DistSessionState; @@ -140,8 +142,35 @@ void ObjectManifestController::processEvent(Event &event, SubscriptionService &e if (!ingesters.empty()) { auto &ingester = ingesters.front(); auto &item = pull_ingest_failed_event.item(); - item.forceRecache(true); // Force refetch on error - ingester->fetch(item); + + // TS 26.517 V18.6.0 clause 6.1.2, object manifest parameter latestFetchTime: + // "The MBSTF shall fetch the object no later than this UTC timestamp." Once that + // time has passed the object must not be fetched again, however many attempts have + // been made, so a retry past it is refused rather than issued and failed. + const bool past_latest_fetch_time = + item.hasDeadline() && std::chrono::system_clock::now() > item.getDeadline(); + + // An object with no latestFetchTime may, by the same clause, be fetched "at a time + // of its choosing", so no clause bounds its retries and the operator's own + // consecutiveIngestFailuresBeforeDeactivate is applied per object instead. The + // session-wide counter in ObjectController cannot serve here: any other object's + // successful fetch resets it, so one permanently unfetchable object would be + // retried without limit while the rest of the session proceeds normally. + const int max_failures = App::self().context()->consecutiveIngestFailuresBeforeDeactivate; + const unsigned failures = item.recordFetchFailure(); + const bool out_of_tries = max_failures != 0 && failures >= static_cast(max_failures); + + if (past_latest_fetch_time) { + ogs_info("Not refetching %s: its latest fetch time has passed after %u attempt(s)", + item.objectId().c_str(), failures); + } else if (out_of_tries) { + ogs_warn("Not refetching %s: %u consecutive fetch failures reached the configured " + "consecutiveIngestFailuresBeforeDeactivate limit of %d", + item.objectId().c_str(), failures, max_failures); + } else { + item.forceRecache(true); // Force refetch on error + ingester->fetch(item); + } } } catch (std::bad_cast &ex) { // Should never happen, but just incase diff --git a/src/mbstf/PullObjectIngester.cc b/src/mbstf/PullObjectIngester.cc index d9af7d9..b2a74e3 100644 --- a/src/mbstf/PullObjectIngester.cc +++ b/src/mbstf/PullObjectIngester.cc @@ -49,6 +49,7 @@ PullObjectIngester::IngestItem::IngestItem(const ObjectStore::Metadata &object_m ,m_forceRecache(force_recache) ,m_markAsKeepAfterSend(keep_after_send) ,m_markAsCompressedSend(compress_send) + ,m_fetchFailures(0) { } @@ -62,6 +63,7 @@ PullObjectIngester::IngestItem::IngestItem(const std::string &object_id, const s ,m_forceRecache(force_recache) ,m_markAsKeepAfterSend(keep_after_send) ,m_markAsCompressedSend(compress_send) + ,m_fetchFailures(0) ,m_availabilityStartTime(availability_start_time) ,m_availabilityEndTime(availability_end_time) { @@ -77,6 +79,7 @@ PullObjectIngester::IngestItem::IngestItem(const IngestItem &other) ,m_forceRecache(other.m_forceRecache) ,m_markAsKeepAfterSend(other.m_markAsKeepAfterSend) ,m_markAsCompressedSend(other.m_markAsCompressedSend) + ,m_fetchFailures(other.m_fetchFailures) ,m_availabilityStartTime(other.m_availabilityStartTime) ,m_availabilityEndTime(other.m_availabilityEndTime) { @@ -92,6 +95,7 @@ PullObjectIngester::IngestItem::IngestItem(IngestItem &&other) ,m_forceRecache(other.m_forceRecache) ,m_markAsKeepAfterSend(other.m_markAsKeepAfterSend) ,m_markAsCompressedSend(other.m_markAsCompressedSend) + ,m_fetchFailures(other.m_fetchFailures) ,m_availabilityStartTime(std::move(other.m_availabilityStartTime)) ,m_availabilityEndTime(std::move(other.m_availabilityEndTime)) { diff --git a/src/mbstf/PullObjectIngester.hh b/src/mbstf/PullObjectIngester.hh index e890835..a06bff2 100644 --- a/src/mbstf/PullObjectIngester.hh +++ b/src/mbstf/PullObjectIngester.hh @@ -97,6 +97,17 @@ public: IngestItem &availabilityStartTime(const std::optional &val) { m_availabilityStartTime = val; return *this; }; const std::optional &availabilityEndTime() const { return m_availabilityEndTime; }; IngestItem &availabilityEndTime(const std::optional &val) { m_availabilityEndTime = val; return *this; }; + + /** Consecutive failed fetch attempts for this object. + * + * Held per item, not per Distribution Session: a session that is ingesting many objects sees + * a successful fetch of any one of them, which is what resets the session-wide counter in + * ObjectController. A single object that can never be fetched therefore never accumulates a + * session-wide run of failures, and would be retried without limit. + */ + unsigned fetchFailures() const { return m_fetchFailures; }; + IngestItem &fetchFailures(unsigned n) { m_fetchFailures = n; return *this; }; + unsigned recordFetchFailure() { return ++m_fetchFailures; }; private: std::string m_objectId; std::string m_url; @@ -106,6 +117,7 @@ public: std::optional m_deadline; std::optional m_availabilityStartTime; std::optional m_availabilityEndTime; + unsigned m_fetchFailures; bool m_forceRecache; bool m_markAsKeepAfterSend; bool m_markAsCompressedSend; diff --git a/tests/test_PullObjectIngester.cc b/tests/test_PullObjectIngester.cc index 92ec2ac..cbf5e09 100644 --- a/tests/test_PullObjectIngester.cc +++ b/tests/test_PullObjectIngester.cc @@ -158,8 +158,49 @@ static void testCopyPreservesEverything() "testCopyPreservesEverything times"); } -MBSTF_NAMESPACE_STOP +/* The failure count decides whether an object is retried, so it has to survive being copied and + moved: items are copied into and out of the ingest list on every queueing path. */ +static void testFetchFailureCountSurvivesCopyAndMove() +{ + PullObjectIngester::IngestItem original("obj6", "http://127.0.0.1/seg6.m4s", "acq6"); + + check(original.fetchFailures() == 0, "testFetchFailureCount starts at zero"); + check(original.recordFetchFailure() == 1, "testFetchFailureCount first failure returns 1"); + original.recordFetchFailure(); + check(original.fetchFailures() == 2, "testFetchFailureCount accumulates"); + + PullObjectIngester::IngestItem copied(original); + check(copied.fetchFailures() == 2, "testFetchFailureCount survives copy"); + + PullObjectIngester::IngestItem moved(std::move(copied)); + check(moved.fetchFailures() == 2, "testFetchFailureCount survives move"); +} + +/* TS 26.517 V18.6.0 clause 6.1.2, latestFetchTime: "The MBSTF shall fetch the object no later than + this UTC timestamp." The refetch decision reads hasDeadline()/getDeadline() to enforce that, so an + item whose deadline has passed must report one, and report it as being in the past. */ +static void testDeadlineDistinguishesPastFromFuture() +{ + auto now = std::chrono::system_clock::now(); + + PullObjectIngester::IngestItem expired("obj7", "http://127.0.0.1/seg7.m4s", "acq7", + std::nullopt, std::nullopt, time_type(now - 60s)); + check(expired.hasDeadline() && expired.getDeadline() < now, + "testDeadline expired item is past its latest fetch time"); + + PullObjectIngester::IngestItem live("obj8", "http://127.0.0.1/seg8.m4s", "acq8", + std::nullopt, std::nullopt, time_type(now + 60s)); + check(live.hasDeadline() && live.getDeadline() > now, + "testDeadline live item is not past its latest fetch time"); + + /* No latestFetchTime: the same clause then lets the MBSTF fetch "at a time of its choosing", so + no deadline bounds the retries and the configured per-object limit is what applies. */ + PullObjectIngester::IngestItem undated("obj9", "http://127.0.0.1/seg9.m4s", "acq9"); + check(!undated.hasDeadline(), "testDeadline absent when no latest fetch time was given"); +} + +MBSTF_NAMESPACE_STOP MBSTF_NAMESPACE_USING; int main() @@ -171,6 +212,8 @@ int main() testAvailabilityTimesIndependentOfDeadline(); testTimesSettableAfterConstruction(); testCopyPreservesEverything(); + testFetchFailureCountSurvivesCopyAndMove(); + testDeadlineDistinguishesPastFromFuture(); std::cout << "Test: PullObjectIngester Pass: " << pass << " Fail: " << fail << std::endl; std::cout << "### PullObjectIngester: Test finish ####" << std::endl; From b1cf3e1238703fdfd6d0a1f2cf3d1bba0a915366 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 6 Sep 2026 11:49:24 +0200 Subject: [PATCH 09/19] mbstf: copy an object's metadata under the store's lock, not through a released reference Problem ObjectStore::getMetadata() takes the store mutex, returns a reference into the store, and releases the mutex as it returns. Everything the caller then reads through that reference is unsynchronised. Metadata holds std::strings and ObjectStore::updateMetadata() move-assigns them, so a caller copying a string while its data pointer and length are being reassigned builds a string from two different states of the same object. This is the cause of a corruption that had been open and unexplained: an ingest URL arriving 55 bytes long with byte 0 zeroed and bytes 1 onward intact, and an object id arriving empty. The store's own copy was correct at that moment, which is what made it look impossible. Confirmed by ThreadSanitizer, both sides named, holding different mutexes: Write of size 8 by thread T4 (mutexes: write M0) std::string::_M_data(char*) ObjectStore::Metadata::operator=(Metadata&&) ObjectStore::updateMetadata(...) PullObjectIngester::doObjectIngest() Previous read of size 8 by thread T6 (mutexes: write M1, write M2) std::string::_M_data() const std::string::basic_string(const std::string&) PullObjectIngester::IngestItem::IngestItem(ObjectStore::Metadata const&, ...) PullObjectIngester::fetch(...) ObjectManifestController::workerLoop(...) The same call site also chained keepAfterSend() and compressedSend() onto that reference, mutating the live store entry with no lock held. [observed, then code-derived] Basis No clause governs a component's internal locking. code-derived and observed only. Raised by Running the component under ThreadSanitizer, after three earlier passes narrowed the corruption to the interval between the copy and the end of IngestItem's constructor without identifying any writing instruction. Reading the code had already shown the reference outlives the lock; the sanitiser is what turned that from a hypothesis into the cause. Change Adds ObjectStore::takeMetadataForIngest(), which takes the lock, applies the two marks and returns the metadata by value before releasing it, so the copy the ingest list stores is made while the entry cannot be mutated. PullObjectIngester::fetch() uses it instead of copying through getMetadata()'s reference. Verification T2: live demo, MBSTF built with -Db_sanitize=thread, same scenario before and after. Before: 162 data race reports, 12 of them naming IngestItem's constructor. After: 64 data race reports, ZERO naming IngestItem's constructor. Delivery unaffected on the same run: gNB carrying MRB1 and MRB2, 2036 CRC-OK broadcast decodes and 118 MCCH receptions at the UE, and no ingest failures at all. T1: all five MBSTF suites pass, 58 cases. Not in this change The other 64 races ThreadSanitizer still reports. They are real and are recorded, but each needs its own diagnosis and none is this defect. getMetadata() itself is left in place: its remaining callers read under conditions this commit has not examined, and changing its contract is a wider refactor than the defect requires. --- src/mbstf/ObjectStore.cc | 8 ++++++++ src/mbstf/ObjectStore.hh | 15 +++++++++++++++ src/mbstf/PullObjectIngester.cc | 4 +++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/mbstf/ObjectStore.cc b/src/mbstf/ObjectStore.cc index 6977dcb..adb630b 100644 --- a/src/mbstf/ObjectStore.cc +++ b/src/mbstf/ObjectStore.cc @@ -267,6 +267,14 @@ ObjectStore::ObjectData& ObjectStore::getObjectData(const std::string& object_id return m_store.at(object_id)->first; } +ObjectStore::Metadata ObjectStore::takeMetadataForIngest(const std::string& object_id, bool keep_after_send, + bool compress_send) { + std::lock_guard lock(m_mutex); + Metadata &metadata = m_store.at(object_id)->second; + metadata.keepAfterSend(keep_after_send).compressedSend(compress_send); + return metadata; // copied while the lock is still held +} + const ObjectStore::Metadata& ObjectStore::getMetadata(const std::string& object_id) const { std::lock_guard lock(m_mutex); return m_store.at(object_id)->second; diff --git a/src/mbstf/ObjectStore.hh b/src/mbstf/ObjectStore.hh index a6925c0..92ba803 100644 --- a/src/mbstf/ObjectStore.hh +++ b/src/mbstf/ObjectStore.hh @@ -341,6 +341,21 @@ public: void updateError(const std::string& object_id, int response_code, const std::string &url, bool synchronous_event = false); const ObjectData& getObjectData(const std::string& object_id) const; ObjectData& getObjectData(const std::string& object_id); + /** Take a copy of an object's metadata, and mark it, without releasing the store lock in between. + * + * getMetadata() returns a reference and drops the lock as it returns, so anything the caller then + * reads through that reference races with any thread updating the same entry. Metadata holds + * std::strings, and ObjectStore::updateMetadata() move-assigns them: a reader copying a string while + * its data pointer and length are being reassigned gets a string built from two different states of + * the same object. ThreadSanitizer reports exactly that between updateMetadata() and + * PullObjectIngester::IngestItem's constructor. + * + * The keep-after-send and compressed-send marks are applied here rather than by the caller for the + * same reason: chaining setters onto a reference returned by getMetadata() mutates the live entry + * with no lock held. + */ + Metadata takeMetadataForIngest(const std::string& object_id, bool keep_after_send, bool compress_send); + const Metadata& getMetadata(const std::string& object_id) const; Metadata& getMetadata(const std::string& object_id); void deleteObject(const std::string& object_id); diff --git a/src/mbstf/PullObjectIngester.cc b/src/mbstf/PullObjectIngester.cc index b2a74e3..17e7dac 100644 --- a/src/mbstf/PullObjectIngester.cc +++ b/src/mbstf/PullObjectIngester.cc @@ -134,7 +134,9 @@ bool PullObjectIngester::fetch(const std::string &object_id, const std::optional // otherwise we need a new fetch based on the ObjectStore entry if (it == m_fetchList.end()) { - m_fetchList.emplace_back(objectStore()->getMetadata(object_id).keepAfterSend(keep_after_send).compressedSend(compress_send), download_deadline, force_recache); + // Copied under the store's own lock: see ObjectStore::takeMetadataForIngest(). + m_fetchList.emplace_back(objectStore()->takeMetadataForIngest(object_id, keep_after_send, compress_send), + download_deadline, force_recache); } sortListByPolicy(); From d22e4305b3f08e148ed7a212823eb3600d17f021 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 6 Sep 2026 15:31:32 +0200 Subject: [PATCH 10/19] comments: make every specification citation verifiable against its document Problem Running tools/verify-citations.py over this branch's own changed files reported citations it could not confirm. Each was a comment whose quoted sentence differed from the document it named. [code-derived] Basis No clause governs how a comment is written. The defect is that a quotation did not match its source, which is checkable without any specification claim. code-derived only. Raised by Running the citation checker across this branch for the first time. [rule 13] Change Quotations now reproduce contiguous source text. Where an earlier comment joined two sentences with an elision, each is quoted separately. Where it inserted an editorial gloss inside the quotation marks, the gloss moved outside them. Where a specification writes a value inside its own quotation marks (a status code, a state name, a file extension), that fragment is named without quotation marks rather than nested, and the comment says why, so nobody restores them. Two citations that sat inside runtime strings moved into comments beside them: a full document identifier in a log or exception message is read as a citation by the checker, which then matches the next string literal in the file. No behaviour changes. Comments only. Verification T1: tools/verify-citations.py reports no unconfirmed citation across this branch's changed files. T0: builds clean. Not in this change Citations naming a document the local corpus does not hold. Those remain unchecked and are listed in the project's own specification index. --- src/mbstf/FecOtiHelper.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/mbstf/FecOtiHelper.cc b/src/mbstf/FecOtiHelper.cc index 6a17bf0..d974070 100644 --- a/src/mbstf/FecOtiHelper.cc +++ b/src/mbstf/FecOtiHelper.cc @@ -64,14 +64,18 @@ std::pair, uint32_t> fecOtiFromFecConfig( oti.encoding_id = LibFlute::FecScheme::Raptor; /* max_source_block_length and encoding_symbol_length are left at their defaults (0): rt-libflute's own Transmitter derives encoding_symbol_length from the session's path MTU - and, under the 3GPP profiles, caps max_source_block_length at the TS 26.346 clause 7.2.3 - 256 KB sub-block ceiling itself when it is left 0. No MBSTF-side bound is invented here. */ + and, under the 3GPP profiles, caps max_source_block_length at the 256 KB sub-block + ceiling of TS 26.346 V18.2.0 clause 7.2.3 itself when it is left 0. No MBSTF-side bound is invented here. */ return {oti, static_cast(fec_overhead)}; } if (fec_scheme == kFecSchemeRaptorQ) { + /* The FEC schemes the MBMS Download Profile admits are listed in TS 26.346 V18.2.0 + clause L.4.7, and RaptorQ is not among them. The runtime message below names the clause + without its version: a full identifier inside a string literal is read as a citation by + the citation checker, which then matches the next string literal in the file. */ throw std::runtime_error( "fecScheme " + fec_scheme + " (RaptorQ) is not one of the FEC schemes the MBMS Download " - "Profile admits (TS 26.346 V18.2.0 clause L.4.7); this MBSTF cannot honour it"); + "Profile admits (TS 26.346 clause L.4.7); this MBSTF cannot honour it"); } throw std::runtime_error("fecScheme " + fec_scheme + " is not implemented by this MBSTF"); } From 2da960c6028ec3bd9680872a2013a0ba4046c724 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 6 Sep 2026 20:52:43 +0200 Subject: [PATCH 11/19] mbstf: size FLUTE symbols for the path, not for a route that never leaves the host Problem FLUTE encoding symbols are sized from getsockopt(IP_MTU) on a socket to the distribution session's ingress. Where the MBSTF and the ingress point are co-located that destination is one of the host's own addresses, so the kernel routes it over loopback and answers 65535. Symbols were sized at 65441 bytes and every object larger than one of them left as a datagram nothing downstream carries. Measured on the rt-mbs-examples broadcast demo, OBJECT_STREAMING session: the FDT advertised FEC-OTI-Encoding-Symbol-Length="65441"; the client was told about 663 distinct TOIs and received data packets for three of them, TOI 0 (the FDT itself), 2 and 4. Every media segment TOI got its FDT Instance and then no data. [observed, code-derived] Basis RFC 5651 section 6.1: "However, network efficiency considerations recommend that the sender uses an as large as possible packet payload size, but in such a way that packets do not exceed the network's maximum transmission unit size (MTU), or when fragmentation coupled with packet loss might introduce severe inefficiency in the transmission." RFC 5651 is the LCT building block TS 26.346 V18.2.0 lists as reference [119]. No clause fixes a number. mbstf.pathMtu is the operator's setting and is documented in mbstf.yaml with a default, which is what rule 12 requires of a bound. Raised by Counting, per TOI, the data packets a client received against the TOIs its FDTs advertised, while chasing why no media segment reached a player. The shape of the fix comes from review by David Waring on pull request #71, who pointed out that clamping a discovered MTU would prevent the jumbo frames his lab runs between the MB-UPF and the gNodeB. [rule 13] Change A discovered MTU is used as it stands whenever the destination is not one of this host's own addresses. A deployment configuring jumbo frames on its interfaces gets them; nothing here caps what the operator set up. get_path_mtu() reports, through via_loopback, whether the destination is a local address. It tests the destination against getifaddrs() rather than for the 127/8 prefix, because a co-located MB-UPF is commonly reached on the address of a real interface -- in the demo, a veth -- which loops back without looking like a loopback address. Only in that case, where there is no path to measure, is Context::pathMtu used, and the substitution is logged with both numbers. The four controllers sequence the discovery before reading the flag rather than nesting the calls: the order function arguments are evaluated in is unspecified. Verification T1: meson test, the five rt-mbs-transport-function suites pass. The other failures in that run are open5gs and libmpdpp subproject tests, untouched by this change. T2: run scripts/mbs-broadcast-demo/start-all.sh in rt-mbs-examples. mbstf.log records "The route to this session's ingress is loopback, so its 65535 byte MTU is not the path to a receiver; sizing FLUTE symbols for the configured 1500 byte path MTU instead", the FDT then advertises FEC-OTI-Encoding-Symbol-Length="1406", and the client receives the announcement bundle, both initialisation segments, the manifest and media segments. Before this change it received data for three TOIs out of 663. Not in this change GTP_HEADER_SIZE in common.hh, which is 2. Whether that is the right allowance needs TS 29.281, which is not held; halted under rule 2. --- src/mbstf/Context.cc | 13 +++++ src/mbstf/Context.hh | 31 +++++++++++ src/mbstf/ObjectCarouselController.cc | 7 ++- src/mbstf/ObjectCollectionController.cc | 7 ++- src/mbstf/ObjectListController.cc | 7 ++- src/mbstf/ObjectStreamingController.cc | 7 ++- src/mbstf/mbstf.yaml.in | 11 ++++ src/mbstf/utilities.cc | 68 +++++++++++++++++++++++-- src/mbstf/utilities.hh | 17 ++++++- 9 files changed, 158 insertions(+), 10 deletions(-) diff --git a/src/mbstf/Context.cc b/src/mbstf/Context.cc index bf06f8c..a11221e 100644 --- a/src/mbstf/Context.cc +++ b/src/mbstf/Context.cc @@ -48,6 +48,7 @@ Context::Context() ,servers() ,cacheControl({60, 60}) ,totalMaxBitRateSoftLimit(100) + ,pathMtu(kDefaultPathMtu) ,consecutiveIngestFailuresBeforeDeactivate(5) ,packetModeSchedulingQueueSize(128*1024) // 128KB queue for rate smoothing ,manifestGlobals() @@ -119,6 +120,18 @@ bool Context::parseConfig() } else { throw std::out_of_range("Bad configuration node at mbstf.totalMaxBitRateSoftLimit"); } + } else if (mbstf_key == "pathMtu") { + Open5GSYamlIter mtu_iter(mbstf_iter); + if (mtu_iter.type() == YAML_SCALAR_NODE) { + std::string num_val(mtu_iter.value()); + size_t idx = 0; + pathMtu = std::stoi(num_val, &idx); + if (idx != num_val.size() || pathMtu <= 0) { + throw std::out_of_range("Bad configuration value at mbstf.pathMtu"); + } + } else { + throw std::out_of_range("Bad configuration node at mbstf.pathMtu"); + } } else if (mbstf_key == "consecutiveIngestFailuresBeforeDeactivate") { Open5GSYamlIter failures_iter(mbstf_iter); if (failures_iter.type() == YAML_SCALAR_NODE) { diff --git a/src/mbstf/Context.hh b/src/mbstf/Context.hh index 7a7342b..b54d46e 100644 --- a/src/mbstf/Context.hh +++ b/src/mbstf/Context.hh @@ -73,6 +73,37 @@ public: unsigned int defaultObjectMaxAge; // Use if not given by push/pull resource Cache-Control. } cacheControl; int totalMaxBitRateSoftLimit; //< total maximum bit rate this MBSTF ought to asked to handle + /**< MTU, in bytes, of the path a distribution session's packets travel to the receiver. + * + * FLUTE encoding symbols are sized from this, so a value larger than the path can carry puts + * every multi-symbol object into datagrams that do not arrive. RFC 5651 section 6.1: "However, + * network efficiency considerations recommend that the sender uses an as large as possible + * packet payload size, but in such a way that packets do not exceed the network's maximum + * transmission unit size (MTU), or when fragmentation coupled with packet loss might introduce + * severe inefficiency in the transmission." + * + * getsockopt(IP_MTU) on a socket to the session's tunnel address, or to its SSM destination + * when there is no tunnel, measures the first hop only. Where the MBSTF and the ingress point + * are co-located, which is every single-host deployment, that hop is the loopback interface + * and the answer is the loopback MTU, 65536 on Linux; and where a tunnel is configured the + * datagram is re-encapsulated and forwarded over a path the MBSTF cannot measure at all. The + * measurement is therefore a ceiling on the first hop, never a description of the whole path. + * + * kDefaultPathMtu is the default and the operator overrides it with mbstf.pathMtu. The + * discovered value is used in place of it only when it is smaller, since a first hop narrower + * than the stated path MTU is a real constraint while a wider one says nothing about the rest + * of the path. + */ + int pathMtu; + + /**< The path MTU assumed when the operator does not state one, in bytes. + * + * The conventional Ethernet MTU. No clause fixes it: it is a documented default, and a + * deployment whose path differs sets mbstf.pathMtu. Sizing symbols below the path MTU costs + * efficiency; sizing them above it costs delivery, so the default is the safe side of that. + */ + static constexpr int kDefaultPathMtu = 1500; + int consecutiveIngestFailuresBeforeDeactivate; //< The number of consecutive ingest failures allowed before the session aborts size_t packetModeSchedulingQueueSize; //< The maximum queue size for packet mode scheduling per DistSession struct { diff --git a/src/mbstf/ObjectCarouselController.cc b/src/mbstf/ObjectCarouselController.cc index a75007f..ee20a30 100644 --- a/src/mbstf/ObjectCarouselController.cc +++ b/src/mbstf/ObjectCarouselController.cc @@ -74,7 +74,12 @@ void ObjectCarouselController::setObjectPackager() const std::optional &tunnel_addr = distributionSession().getTunnelAddr(); uint32_t rate_limit = distributionSession().getRateLimit(); in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); - unsigned short mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, GET_MTU_ETHERNET_PAYLOAD) - GTP_HEADER_SIZE; + bool mtu_via_loopback = false; + /* Sequenced, not nested: the order arguments are evaluated in is unspecified, so reading + mtu_via_loopback in the same call that fills it would read it before it is set. */ + const int discovered_mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, + GET_MTU_ETHERNET_PAYLOAD, &mtu_via_loopback); + unsigned short mtu = flute_path_mtu(discovered_mtu, mtu_via_loopback) - GTP_HEADER_SIZE; packager(new ObjectCarouselPackager(objectStore(), *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port, distributionSession().getFecInformation())); auto pkgr = getObjectCarouselPackager(); diff --git a/src/mbstf/ObjectCollectionController.cc b/src/mbstf/ObjectCollectionController.cc index 1989fda..e3ffd4e 100644 --- a/src/mbstf/ObjectCollectionController.cc +++ b/src/mbstf/ObjectCollectionController.cc @@ -76,7 +76,12 @@ void ObjectCollectionController::setObjectPackager() const std::optional &tunnel_addr = distributionSession().getTunnelAddr(); uint32_t rate_limit = distributionSession().getRateLimit(); in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); - unsigned short mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, GET_MTU_ETHERNET_PAYLOAD) - GTP_HEADER_SIZE; + bool mtu_via_loopback = false; + /* Sequenced, not nested: the order arguments are evaluated in is unspecified, so reading + mtu_via_loopback in the same call that fills it would read it before it is set. */ + const int discovered_mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, + GET_MTU_ETHERNET_PAYLOAD, &mtu_via_loopback); + unsigned short mtu = flute_path_mtu(discovered_mtu, mtu_via_loopback) - GTP_HEADER_SIZE; auto fec_information = distributionSession().getFecInformation(); packager(new ObjectListPackager(objectStore(), *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port, fec_information)); auto pkgr = getObjectListPackager(); diff --git a/src/mbstf/ObjectListController.cc b/src/mbstf/ObjectListController.cc index c6bdec2..87e2ff5 100644 --- a/src/mbstf/ObjectListController.cc +++ b/src/mbstf/ObjectListController.cc @@ -77,7 +77,12 @@ void ObjectListController::setObjectPackager() { std::optional tunnel_addr = distributionSession().getTunnelAddr(); uint32_t rate_limit = distributionSession().getRateLimit(); in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); - unsigned short mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, GET_MTU_ETHERNET_PAYLOAD) - GTP_HEADER_SIZE; + bool mtu_via_loopback = false; + /* Sequenced, not nested: the order arguments are evaluated in is unspecified, so reading + mtu_via_loopback in the same call that fills it would read it before it is set. */ + const int discovered_mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, + GET_MTU_ETHERNET_PAYLOAD, &mtu_via_loopback); + unsigned short mtu = flute_path_mtu(discovered_mtu, mtu_via_loopback) - GTP_HEADER_SIZE; const auto &obj_list = object_store->getObjects(); packager(new ObjectListPackager(object_store, *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port, distributionSession().getFecInformation())); diff --git a/src/mbstf/ObjectStreamingController.cc b/src/mbstf/ObjectStreamingController.cc index dc32902..22a80cd 100644 --- a/src/mbstf/ObjectStreamingController.cc +++ b/src/mbstf/ObjectStreamingController.cc @@ -71,7 +71,12 @@ void ObjectStreamingController::setObjectPackager() const std::optional &tunnel_addr = distributionSession().getTunnelAddr(); uint32_t rate_limit = distributionSession().getRateLimit(); in_port_t tunnel_port = distributionSession().getTunnelPortNumber(); - unsigned short mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, GET_MTU_ETHERNET_PAYLOAD) - GTP_HEADER_SIZE; + bool mtu_via_loopback = false; + /* Sequenced, not nested: the order arguments are evaluated in is unspecified, so reading + mtu_via_loopback in the same call that fills it would read it before it is set. */ + const int discovered_mtu = get_tunnelled_path_mtu(ssm_port, tunnel_addr, tunnel_port, + GET_MTU_ETHERNET_PAYLOAD, &mtu_via_loopback); + unsigned short mtu = flute_path_mtu(discovered_mtu, mtu_via_loopback) - GTP_HEADER_SIZE; packager(new ObjectListPackager(objectStore(), *this, ssm_port, rate_limit, mtu, tunnel_addr, tunnel_port, distributionSession().getFecInformation())); auto pkgr = getObjectListPackager(); diff --git a/src/mbstf/mbstf.yaml.in b/src/mbstf/mbstf.yaml.in index b7e4036..c9491cc 100644 --- a/src/mbstf/mbstf.yaml.in +++ b/src/mbstf/mbstf.yaml.in @@ -212,6 +212,16 @@ sbi: # update period is greater than the repetition period then the MPD will be repeated at the given interval until the update # period elapses, at which point the MPD will be refetched and sent. # +# +# pathMtu: 1500 +# +# o The MTU, in bytes, of the path a distribution session's packets travel to the receiver. FLUTE encoding symbols are sized +# from it, so a value larger than the path carries puts every multi-symbol object into datagrams that never arrive. +# +# o The MBSTF can only measure the first hop, and where it is co-located with the ingress point that hop is loopback and +# measures 65536. The default of 1500, the conventional Ethernet MTU, is used instead unless the first hop measures less. +# Set this to the real figure for a path that carries more, or less, than an Ethernet frame. +# mbstf: sbi: - addr: 127.0.0.59 @@ -226,6 +236,7 @@ mbstf: - addr: 127.0.0.61 port: 0 # ephemeral totalMaxBitRateSoftLimit: 1000 # 1Gbps + pathMtu: 1500 consecutiveIngestFailuresBeforeDeactivate: 5 packetModeSchedulingQueueSize: 131072 # 128KB queue per packet mode DistSession serverResponseCacheControl: diff --git a/src/mbstf/utilities.cc b/src/mbstf/utilities.cc index 9c00ab9..e057156 100644 --- a/src/mbstf/utilities.cc +++ b/src/mbstf/utilities.cc @@ -34,6 +34,10 @@ #include "common.hh" #include "SsmPort.hh" +#include "App.hh" +#include "Context.hh" +#include + #include "utilities.hh" MBSTF_NAMESPACE_START @@ -91,8 +95,33 @@ std::chrono::system_clock::time_point http_datetime_str_to_time_point(const std: return retval; } -int get_path_mtu(const ogs_sockaddr_t &sock_addr, int minus_level_hdrs) +/** Whether an address belongs to an interface on this host. + * + * Traffic to such an address does not reach a network: the kernel routes it over the loopback + * interface, whatever the address looks like. + */ +static bool address_is_local(const ogs_sockaddr_t &sock_addr) +{ + struct ifaddrs *ifaddr = nullptr; + if (getifaddrs(&ifaddr) != 0) return false; + bool found = false; + for (const struct ifaddrs *ifa = ifaddr; ifa != nullptr && !found; ifa = ifa->ifa_next) { + if (ifa->ifa_addr == nullptr || ifa->ifa_addr->sa_family != sock_addr.ogs_sa_family) continue; + if (sock_addr.ogs_sa_family == AF_INET) { + const auto *a = reinterpret_cast(ifa->ifa_addr); + found = (a->sin_addr.s_addr == sock_addr.sin.sin_addr.s_addr); + } else if (sock_addr.ogs_sa_family == AF_INET6) { + const auto *a = reinterpret_cast(ifa->ifa_addr); + found = (memcmp(&a->sin6_addr, &sock_addr.sin6.sin6_addr, sizeof(struct in6_addr)) == 0); + } + } + freeifaddrs(ifaddr); + return found; +} + +int get_path_mtu(const ogs_sockaddr_t &sock_addr, int minus_level_hdrs, bool *via_loopback) { + if (via_loopback) *via_loopback = false; ogs_sock_t *sock = ogs_sock_socket(sock_addr.ogs_sa_family, SOCK_DGRAM, 0); ogs_sock_connect(sock, const_cast(&sock_addr)); int mtu = 1500; @@ -104,25 +133,56 @@ int get_path_mtu(const ogs_sockaddr_t &sock_addr, int minus_level_hdrs) getsockopt(sock->fd, IPPROTO_IPV6, IPV6_MTU, &mtu, &mtu_size); if (minus_level_hdrs >= GET_MTU_IP_PAYLOAD) mtu -= sizeof(ip6_hdr); } + /* Whether this destination is one of the host's own addresses decides whether the datagram + leaves the host at all, and so whether the MTU just read describes a path to a receiver. + Where it is, the kernel routes it over the loopback interface and answers with the loopback + MTU however the destination address is written. + Testing the destination against the host's own addresses rather than for the 127/8 prefix: + a co-located MB-UPF is commonly reached on the address of a real interface (a veth, say), + which loops back without ever looking like a loopback address. */ + if (via_loopback) *via_loopback = address_is_local(sock_addr); ogs_sock_destroy(sock); return mtu; } -int get_tunnelled_path_mtu(const SsmPort &ssm_port, const std::optional &tunnel_ip, in_port_t tunnel_port, int minus_level_hdrs) +int flute_path_mtu(int discovered_mtu, bool discovered_via_loopback) +{ + /* A measurement of a route that leaves the host is a real bound and is used as it stands, + including where an operator has raised it: a deployment running jumbo frames between the + MB-UPF and the gNB is configured through the interface MTUs, and second-guessing that here + would cap it for no reason. + + A loopback route is not a path. Where the MBSTF and the ingress point are co-located the + kernel answers with the loopback MTU, 65536, and symbols sized for that leave as datagrams + nothing downstream carries. There is nothing to measure in that case, so the configured + path MTU is used, which mbstf.pathMtu sets and which is documented in mbstf.yaml. */ + if (!discovered_via_loopback && discovered_mtu > 0) return discovered_mtu; + + const int path_mtu = App::self().context()->pathMtu; + if (discovered_via_loopback) { + ogs_info("The route to this session's ingress is loopback, so its %d byte MTU is not the " + "path to a receiver; sizing FLUTE symbols for the configured %d byte path MTU " + "instead (mbstf.pathMtu)", discovered_mtu, path_mtu); + } + return path_mtu; +} + +int get_tunnelled_path_mtu(const SsmPort &ssm_port, const std::optional &tunnel_ip, in_port_t tunnel_port, int minus_level_hdrs, bool *via_loopback) { int mtu = 1500; // default to 1500 if no MTU can be found. + if (via_loopback) *via_loopback = false; if (tunnel_ip) { // Use MTU of tunnel if provided ogs_sockaddr_t *sa = nullptr; if (ogs_addaddrinfo(&sa, AF_UNSPEC, tunnel_ip.value().c_str(), tunnel_port, AI_NUMERICSERV) == OGS_OK) { - mtu = get_path_mtu(*sa, minus_level_hdrs); + mtu = get_path_mtu(*sa, minus_level_hdrs, via_loopback); ogs_freeaddrinfo(sa); } // else error already reported } else { // No tunnel provided so try MTU of direct destination if (ssm_port) { ogs_sockaddr_t *sa = nullptr; if (ogs_addaddrinfo(&sa, AF_UNSPEC, ssm_port.destinationAddress().c_str(), ssm_port.port(), AI_NUMERICSERV) == OGS_OK) { - mtu = get_path_mtu(*sa, minus_level_hdrs); + mtu = get_path_mtu(*sa, minus_level_hdrs, via_loopback); ogs_freeaddrinfo(sa); } } diff --git a/src/mbstf/utilities.hh b/src/mbstf/utilities.hh index a48745e..78da7f2 100644 --- a/src/mbstf/utilities.hh +++ b/src/mbstf/utilities.hh @@ -44,9 +44,22 @@ enum GetMTULevels { GET_MTU_IP_PAYLOAD = 1 }; -int get_path_mtu(const ogs_sockaddr_t &sock_addr, int minus_level_hdrs = GET_MTU_ETHERNET_PAYLOAD); -int get_tunnelled_path_mtu(const SsmPort &ssm_port, const std::optional &tunnel_ip, in_port_t tunnel_port, int minus_level_hdrs = GET_MTU_ETHERNET_PAYLOAD); +int get_path_mtu(const ogs_sockaddr_t &sock_addr, int minus_level_hdrs = GET_MTU_ETHERNET_PAYLOAD, + bool *via_loopback = nullptr); +int get_tunnelled_path_mtu(const SsmPort &ssm_port, const std::optional &tunnel_ip, in_port_t tunnel_port, + int minus_level_hdrs = GET_MTU_ETHERNET_PAYLOAD, bool *via_loopback = nullptr); + +/** The MTU to size a distribution session's FLUTE symbols with. + * + * A discovered MTU for a route that leaves the host is used as it stands, so a deployment running + * jumbo frames between the MB-UPF and the gNB gets the frames it configured its interfaces for. + * + * Where the route is loopback, which is what get_path_mtu() reports through via_loopback, there is + * no path to measure: the MBSTF and the ingress point are co-located and the kernel answers with + * the loopback MTU. Context::pathMtu is used instead, set by mbstf.pathMtu. + */ +int flute_path_mtu(int discovered_mtu, bool discovered_via_loopback); std::shared_ptr make_shared_sockaddr(int family_hint, const std::string &hostname, in_port_t port); MBSTF_NAMESPACE_STOP From 2958c1eb33c99f92702ee2202f5740918d0cf087 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Sun, 6 Sep 2026 22:50:24 +0200 Subject: [PATCH 12/19] mbstf: send a DASH media segment once, not once per manifest pass Problem In OBJECT_STREAMING every media segment is transmitted several times, each time under a different TOI. Counting distinct TOIs against distinct Content-Locations in the FDTs a receiver was given: 4.0 TOIs per object on average, and up to 15. 15 x chunk-stream0-00047.m4s 11 x manifest.mpd 10 x chunk-stream0-00051.m4s This is not redundancy. A receiver cannot combine symbols across TOIs, so each copy is a separate object that is independently incomplete, and the copies consume the bearer the first copy needed. Measured live on the rt-mbs-examples broadcast demo, the effect is that no media segment ever completes at the client: symbol IDs arrive with gaps throughout, for example SBN 1 ID 7, 11, 12, 16, 19, 20, 22, 26, 27, 30, 32, 33, 42, while the radio itself delivered 33382 of 33455 grants with zero CRC failures. [observed] DASHManifestHandler::nextIngestItems() takes every segment the MPD currently advertises and clamps any whose availability start has passed to the present, so a segment stays in the candidate set for the whole of its availability window. Nothing records that it has already been sent. The ObjectStore cannot answer for it either: ObjectStreamingController leaves Metadata::keepAfterSend() at false, so ObjectController deletes the object as soon as it is sent, findMetadataByURL() then misses, and a second object is created for the same URL, which the packager sends under a second TOI. [code-derived] Basis RFC 3926 clause 3.1: "Note that each object is associated with a unique TOI within the scope of a session." Sending one file under several TOIs therefore presents it as several objects, and a receiver has no basis on which to combine their symbols. An MPD advertising a segment states that a client may still fetch it, not that it still needs transmitting; no clause requires a segment to be sent more than once. Raised by Counting TOIs per Content-Location while establishing why no object completed at a receiver, after the path-MTU defect was fixed and the segments started arriving. [rule 13] Change DASHManifestHandler remembers the media segment URLs it has already handed to the ingester and skips them on later passes. The set is pruned against the current manifest on every pass, so it holds at most one entry per segment the MPD still advertises. Only segments the MPD itself advertises are suppressed. The MPD refresh and the initialisation segments, which come from m_extraPullObjects, are meant to repeat and are deliberately not recorded. keepAfterSend() is untouched. Removing it from ObjectManifestController was correct: an OBJECT_STREAMING object genuinely should not be retained once sent, and the carousel sets it for itself. What was missing is a record of what has been sent, which is added here rather than by retaining objects that are no longer needed. Verification T0: builds clean. Not in this change Anything in the carousel path, which retains its objects and repeats them under one TOI. The gNB-side per-slot MBS scheduling limits, which are a separate matter in srsRAN_Project_mbs. --- src/mbstf/DASHManifestHandler.cc | 53 ++++++++++++++++++++++++++++++++ src/mbstf/DASHManifestHandler.hh | 20 ++++++++++++ 2 files changed, 73 insertions(+) diff --git a/src/mbstf/DASHManifestHandler.cc b/src/mbstf/DASHManifestHandler.cc index 927ec4a..1b840ec 100644 --- a/src/mbstf/DASHManifestHandler.cc +++ b/src/mbstf/DASHManifestHandler.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -135,6 +136,34 @@ std::pair DASHManifest std::lock_guard guard(m_mpdMutex); media_segments = std::move(augmentSegmentAvailabilityList(m_mpd.selectedSegmentAvailability(), false, false, false)); } + /* A live MPD lists a segment for the whole of its availability window, which tells a unicast client + it may still fetch that segment. It does not mean the segment still needs transmitting. Without + this filter the same segment is emitted on every pass until its window closes, and because + OBJECT_STREAMING removes an object from the ObjectStore once sent, findMetadataByURL() below + misses on each pass and builds a fresh object, which the packager then sends under a fresh TOI. + A receiver cannot combine symbols across TOIs, so those copies are not redundancy: each is a + separate incomplete object, and together they crowd out the bandwidth the first copy needed. + Measured before this filter: 4.0 TOIs per object on average and up to 15. */ + pruneSentSegments(media_segments); + media_segments.remove_if([this](const SegmentEntry &seg) { + try { + return m_sentSegmentUrls.find(seg.segmentURL()) != m_sentSegmentUrls.end(); + } catch (std::domain_error&) { + return false; /* leave a malformed URL to the existing handling further down */ + } + }); + + /* Only segments the MPD itself advertises are suppressed on a later pass. The entries appended + from m_extraPullObjects below -- the MPD refresh and the initialisation segments -- are meant to + repeat, and are deliberately not recorded. */ + std::set media_segment_urls; + for (const auto &seg : media_segments) { + try { + media_segment_urls.insert(seg.segmentURL()); + } catch (std::domain_error&) { + } + } + media_segments.insert(media_segments.end(), m_extraPullObjects.begin(), m_extraPullObjects.end()); for (auto &ms: media_segments) { if(ms.availabilityStartTime() < current_time) @@ -175,6 +204,7 @@ std::pair DASHManifest obj_dist_base_url, first_media_segment.availabilityEndTime(), first_media_segment.forceRecache(), first_media_segment.keepAfterSend(), first_media_segment.compressEntry()); } removeExtraPullObjectsEntry(first_media_segment); + if (media_segment_urls.find(segment_url) != media_segment_urls.end()) m_sentSegmentUrls.insert(segment_url); try { if (first_media_segment.segmentURL() == manifest_url) m_refreshMpd = true; @@ -189,6 +219,7 @@ std::pair DASHManifest segment_url = it->segmentURL(); existing_obj = object_store->findMetadataByURL(segment_url); removeExtraPullObjectsEntry(*it); + if (media_segment_urls.find(segment_url) != media_segment_urls.end()) m_sentSegmentUrls.insert(segment_url); if (existing_obj) { ingest_items.emplace_back(*existing_obj, it->availabilityEndTime(), it->forceRecache(), it->keepAfterSend(), it->compressEntry()); } else { @@ -202,6 +233,28 @@ std::pair DASHManifest return std::make_pair(fetch_time, ingest_items); } +void DASHManifestHandler::pruneSentSegments(const std::list ¤t_segments) +{ + /* Bounds m_sentSegmentUrls by the manifest itself: a segment the MPD no longer advertises can no + longer be re-emitted by the filter above, so remembering it serves nothing. Without this the set + grows for the lifetime of a live session. */ + if (m_sentSegmentUrls.empty()) return; + std::set still_listed; + for (const auto &seg : current_segments) { + try { + still_listed.insert(seg.segmentURL()); + } catch (std::domain_error&) { + } + } + for (auto it = m_sentSegmentUrls.begin(); it != m_sentSegmentUrls.end();) { + if (still_listed.find(*it) == still_listed.end()) { + it = m_sentSegmentUrls.erase(it); + } else { + ++it; + } + } +} + void DASHManifestHandler::addMPDRefreshToExtraPullObjects() { std::lock_guard guard(m_mpdMutex); diff --git a/src/mbstf/DASHManifestHandler.hh b/src/mbstf/DASHManifestHandler.hh index 7a7610a..0df82d4 100644 --- a/src/mbstf/DASHManifestHandler.hh +++ b/src/mbstf/DASHManifestHandler.hh @@ -12,6 +12,7 @@ * https://drive.google.com/file/d/1cinCiA778IErENZ3JN52VFW-1ffHpx7Z/view */ #include +#include #include #include #include @@ -95,6 +96,25 @@ private: bool m_refreshMpd; ManifestHandler::time_type m_mpdReceivedTime; std::list m_extraPullObjects; + /** Media segment URLs already handed to the ingester in this session. + * + * A live MPD keeps a segment listed for the whole of its availability window, which is what tells + * a unicast client it may still fetch it. It does not mean the segment still needs sending. In + * OBJECT_STREAMING an object is removed from the ObjectStore once it has been sent + * (ObjectStreamingController leaves Metadata::keepAfterSend() at false, unlike the carousel), so + * the store cannot answer "has this been sent already?" either: findMetadataByURL() misses, a + * second ObjectStore object is created for the same URL, and it goes out under a second TOI. + * + * A receiver cannot combine symbols across TOIs. RFC 3926 clause 3.1: "Note that each object is + * associated with a unique TOI within the scope of a session." Each copy is therefore a separate, + * independently incomplete object rather than redundancy, and the copies consume the bearer that + * the first copy needed. + * + * Bounded by the MPD's own availability window: pruneSentSegments() drops every entry the current + * manifest no longer lists, so this holds at most one string per segment currently advertised. + */ + std::set m_sentSegmentUrls; + void pruneSentSegments(const std::list ¤t_segments); }; MBSTF_NAMESPACE_STOP From 4cebe62b54a175e0708d47f6fef62e64bb9f782d Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 15 Sep 2026 18:05:53 +0200 Subject: [PATCH 13/19] mbstf: name the generated OpenAPI test sources after the generator has run Problem meson setup failed on any checkout that had not been built before: src/mbstf/meson.build:66:29: ERROR: File openapi/model/FECConfig.cc does not exist. FECConfig.cc is generated, not tracked: src/mbstf/.gitignore excludes the whole openapi directory and the generator writes its 153 files at configure time. meson's files() checks existence when it is evaluated, and the test source list naming those two generated files sat above the generator invocation, so a fresh checkout failed and only a tree left over from an earlier build succeeded. [observed: clean clone of this branch; code-derived: meson.build ordering] Basis No clause governs this; it is this repository's own build definition. [code-derived] Raised by Building this branch from a clean clone, after a user report that third-party developers must be able to build it themselves. Change test_source_fec_oti_helper is defined after the generator has run and written .openapi.srcs. Its only consumer is tests/meson.build, which the top-level meson.build enters after src, so nothing else moves. Verification T2: a clean clone of this branch builds through to open5gs-mbstfd. Not in this change The generator's own failure reporting, which is fixed in rt-common-shared. --- src/mbstf/meson.build | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/mbstf/meson.build b/src/mbstf/meson.build index dd39c4e..c9a7f6a 100644 --- a/src/mbstf/meson.build +++ b/src/mbstf/meson.build @@ -63,13 +63,6 @@ test_source_object_list_packager = test_source_object_store + files(''' ObjectListPackager.hh '''.split()) -test_source_fec_oti_helper = files(''' - FecOtiHelper.cc - FecOtiHelper.hh - openapi/model/FECConfig.cc - openapi/model/FECConfig.h - '''.split()) - test_source_pull_object_ingester = test_source_object_store + files(''' PullObjectIngester.cc PullObjectIngester.hh @@ -217,6 +210,17 @@ message('Generating OpenAPI bindings for version '+api_tag+' of the 5G APIs...') openapi_gen_result = run_command([gen_5gmbstf_sh,'-c','"$MESON_SOURCE_ROOT/$MESON_SUBDIR/generator-mbstf" -M "'+openapi_dep_file+'" -b '+api_tag], capture: true, check: true) libmbstf_openapi_gen_sources = files(fs.read(openapi_dep_file).split()) +# Defined here, after the generator has run, and not up with the other test source lists: two of +# these files are generated by it. meson's files() checks existence at configure time, so naming +# them earlier fails a fresh checkout with "File openapi/model/FECConfig.cc does not exist" and +# succeeds only where a previous build already left the generated tree behind. +test_source_fec_oti_helper = files(''' + FecOtiHelper.cc + FecOtiHelper.hh + openapi/model/FECConfig.cc + openapi/model/FECConfig.h + '''.split()) + version_conf = configuration_data() version_conf.set_quoted('MBSTF_NAME', meson.project_name()) version_conf.set_quoted('MBSTF_VERSION', meson.project_version()) From 1a93f5559807bb554a40e5917f73c28d0c0008f7 Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 15 Sep 2026 18:05:53 +0200 Subject: [PATCH 14/19] mbstf: pin rt-common-shared to the MBS branch, not the diverged release branch Problem The submodule pointed at a commit that exists only on a local 3GPP-Rel18, while rt-mbs-function pinned the same submodule to feature/mbs-compliance-fixes. The two network functions were therefore built against different versions of a shared library, and this one missed the MBS work and the generator's failure-reporting fix. Because that commit was never pushed, the pin also resolved only on the machine it was made on. [code-derived: the two gitlinks] Basis No clause governs this; it is this repository's own dependency pin. [code-derived] Raised by User question asking why this repository was not pinned to what it needs. Change The submodule advances to the MBS branch's tip, which carries the ACCESS_TOKEN_CLAIM_MISSING change this repository was pinned to 3GPP-Rel18 for, and adds the generator fix and the HTTP server work. Both network functions now build against the same rt-common-shared. Verification T2: a clean clone of this branch builds through to open5gs-mbstfd against that tip. Not in this change Publishing the rt-common-shared branch this points at. --- subprojects/rt-common-shared | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/rt-common-shared b/subprojects/rt-common-shared index f83f1bd..0bab3d1 160000 --- a/subprojects/rt-common-shared +++ b/subprojects/rt-common-shared @@ -1 +1 @@ -Subproject commit f83f1bdedf6f16275011760ce98958ce928301f5 +Subproject commit 0bab3d165085287320ab7742941f6ba1902e5afe From eb080ec4abe7886bb8c8a467512eabc4e198b1aa Mon Sep 17 00:00:00 2001 From: Jordi Joan Gimenez Date: Tue, 15 Sep 2026 18:05:53 +0200 Subject: [PATCH 15/19] docs: bring the README up to the reference-tool baseline Problem The README opened with a bare heading and two badges, had no At a glance table, and its clone command named no branch and cloned into a path under $HOME. It did not say that the build fetches the 3GPP 5G APIs over the network, nor what the two 5G-MAG libraries it pulls in supply. [code-derived] Basis No clause governs a repository's own README. The structure is the house baseline used by rt-cmmf-encoder. [code-derived] Raised by User report that each repository must state what to install, clone, build, install and run. Change The baseline's header and sections, including Specification with the versions this is built against, and Dependencies naming what rt-common-shared and rt-libflute supply. A section on the 5G APIs fetch: that it needs network access and Java, that forge.3gpp.org serves an incomplete certificate chain, and how to install the missing intermediate rather than disable verification. Verification T2: a clean clone built through to open5gs-mbstfd following these instructions, including the certificate step, which is what makes the generation succeed. Not in this change The generator and the build definition, fixed separately. --- .github/banner.svg | 25 ++++++++++ README.md | 117 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 .github/banner.svg diff --git a/.github/banner.svg b/.github/banner.svg new file mode 100644 index 0000000..db1d2cf --- /dev/null +++ b/.github/banner.svg @@ -0,0 +1,25 @@ + + Reference Tools · 5G Multicast Broadcast Services: MBS Transport Function (MBSTF) + + + + + + + + + + + + + + + + + + + + + + + diff --git a/README.md b/README.md index 6e790c7..bf9229f 100644 --- a/README.md +++ b/README.md @@ -1,15 +1,58 @@ -

5G MBS User Services: MBS Transport Function

- Version - Under Development - License + 5G-MAG Reference Tools, 5G Multicast Broadcast Services: MBS Transport Function (MBSTF)

+

+ The MBS Transport Function (MBSTF), the user-plane function that delivers MBS content as + FLUTE object streams, per 3GPP TS 26.502 and TS 29.581. +

+ +

+ Status: under development + Version + 5G-MAG Public License v1.0 +

+ +

+ Project page  ·  + Issues  ·  + Contributing +

+ +--- + +## At a glance + +| | | +|---|---| +| **Implements** | 3GPP TS 26.502, *5G multicast-broadcast services; User Service architecture*, and TS 29.581, *Nmb8 service API* | +| **Role** | MBSTF: object ingest, FLUTE packaging, MBS delivery over the user plane | +| **Built with** | C++ and meson, on top of Open5GS | +| **Works with** | [rt-mbs-function](https://github.com/5G-MAG/rt-mbs-function) (MBSF), which drives it over Nmb8, and [rt-mbs-client](https://github.com/5G-MAG/rt-mbs-client) at the receiving end | +| **Part of** | [5G Multicast Broadcast Services](https://www.5g-mag.com/reference-tools/5g-multicast-broadcast-services) | + ## Introduction -This repository provides a 5G MBS Transport Function which forms part of the MBS User Services. This NF provides the interfaces designated as Nmb2, Nmb8 and Nmb9 in the [3GPP TS 29.581 V18.5.0](https://www.3gpp.org/DynaReport/29581.htm) specification. +This repository provides the MBS Transport Function. It takes content by PULL or PUSH, packages it into +FLUTE object streams, and transmits them on the MBS session the MBSF has established, whether that +session is broadcast or multicast. + +It is built on the [Open5GS](https://open5gs.org/) framework and registers with an NRF like any +other network function. + +## Specification + +Built against these versions, named rather than referred to by release: + +- **3GPP TS 26.502 V18.6.0**, *5G multicast-broadcast services; User Service architecture* +- **3GPP TS 29.581 V18.6.0**, *Nmb8 service API* +- **3GPP TS 26.346 V18.2.0**, *MBMS protocols and codecs*, for the FLUTE and FDT profiling -Additional information can be found at: https://5g-mag.github.io/Getting-Started/pages/5g-multicast-broadcast-services/ +Clause-by-clause coverage, and what is still absent, is recorded on the project page rather than +here: ## Install dependencies @@ -24,6 +67,26 @@ sudo sh -c 'for i in cpp g++ gcc gcc-ar gcc-nm gcc-ranlib gcov gcov-dump gcov-to sudo python3 -m pip install --break-system-packages --upgrade meson ``` +### The build fetches the 5G APIs + +The OpenAPI bindings are generated at configure time from the 3GPP 5G APIs, which the build clones +from `forge.3gpp.org`. The build therefore needs network access to that host, and Java, which is +why `default-jdk` is in the list above. + +That host currently serves an **incomplete certificate chain**: it sends its own certificate but +not the Sectigo intermediate that signs it. A browser fetches the missing intermediate by itself, +but `git` and `curl` do not, so the clone fails with: + +``` +fatal: unable to access 'https://forge.3gpp.org/rep/all/5G_APIs.git/': + SSL certificate verification failed: certificate signer not trusted +``` + +If you see that, install the missing intermediate rather than disabling verification. On Debian +and Ubuntu, fetch *Sectigo Public Server Authentication CA OV R36* from +, put the PEM in `/usr/local/share/ca-certificates/` with a `.crt` +extension, and run `sudo update-ca-certificates`. + ## Downloading Release tar files can be downloaded from . @@ -33,10 +96,25 @@ The source can be obtained by cloning the github repository. For example to download the latest release you can use: ```bash -cd ~ git clone --recurse-submodules https://github.com/5G-MAG/rt-mbs-transport-function.git +cd rt-mbs-transport-function ``` +`--recurse-submodules` is not optional: this repository carries `rt-common-shared` as a submodule +and the build fails without it. If you have already cloned without it, run +`git submodule update --init --recursive`. + +## Dependencies + +Two 5G-MAG libraries are pulled in by the build and fetched automatically. They are listed here +because a version mismatch surfaces as a compile or link error rather than as a missing +dependency. + +| Dependency | How | What it supplies | +|---|---|---| +| `rt-common-shared` | git submodule | the HTTP server and the shared Open5GS tooling, including the OpenAPI generator this build runs | +| `rt-libflute` | meson wrap | the FLUTE transmitter, the TS 26.346 annex L.6 profiled FDT schema, the scheme-specific FEC OTI and the RFC 5053 Raptor scheme | + ## Building The build process requires a working Internet connection as the API files are retrieved at build time. @@ -44,19 +122,17 @@ The build process requires a working Internet connection as the API files are re To build the 5G Data Collection Application Function from the source: ```bash -cd ~/rt-mbs-transport-function -meson build +meson setup build ninja -C build ``` -**Note:** Errors during the `meson build` command are often caused by missing dependencies or a network issue while trying to retrieve the API files and `openapi-generator` JAR file. See the `~/rt-mbs-transport-function/build/meson-logs/meson-log.txt` log file for the errors in greater detail. Search for `generator-libspdc` to find the start of the API fetch sequence. +**Note:** Errors during the `meson build` command are often caused by missing dependencies or a network issue while trying to retrieve the API files and `openapi-generator` JAR file. See the `build/meson-logs/meson-log.txt` log file for the errors in greater detail. Search for `generator-libspdc` to find the start of the API fetch sequence. ## Unit tests (optional) There are some unit tests that can be run using: ```bash -cd ~/rt-mbs-transport-function meson test -C build --suite rt-mbs-transport-function ``` @@ -67,8 +143,7 @@ This will build the MBSTF (if not already built) and then will run the unit test To install the built MBS Transport Function as a system process: ```bash -cd ~/rt-mbs-transport-function/build -sudo meson install --no-rebuild +sudo meson install -C build --no-rebuild ``` ## Running @@ -85,6 +160,13 @@ Make sure the IP address and port details of the NRF you are running are configu sudo /usr/local/bin/open5gs-mbstfd & ``` +## Configuration + +Configuration is a YAML file in the Open5GS style, installed as +`/usr/local/etc/open5gs/mbstf.yaml` and passed with `-c` when running from a build tree. The +sections that matter are `nrf`, which must point at a reachable NRF, and the MBSTF's own SBI +address and the local address it sends FLUTE from. + ## Development This project follows @@ -92,3 +174,12 @@ the [Gitflow workflow](https://www.atlassian.com/git/tutorials/comparing-workflo `development` branch of this project serves as an integration branch for new features. Consequently, please make sure to switch to the `development` branch before starting the implementation of a new feature. +## Contributing + +Contributions are welcome. How to raise an issue, fork the repository and open a pull request, and +the Contributor License Agreement required before code can be merged, are described at +. + +## License + +See [LICENSE](LICENSE). From e99a3ceec2b692cf10cd91f3f0775840b5dfd0cd Mon Sep 17 00:00:00 2001 From: "Jordi J. Gimenez" Date: Tue, 15 Sep 2026 18:49:00 +0200 Subject: [PATCH 16/19] build: follow rt-common-shared to its published commit Problem The submodule pinned a commit that no longer exists on rt-common-shared's MBS branch. The branch was reauthored before being published, which changed every commit hash on it, and this pin still named the pre-rewrite one. A clean clone therefore failed at submodule init: fatal: remote error: upload-pack: not our ref 0bab3d165085287320ab7742941f6ba1902e5afe [observed: clean clone of this branch after rt-common-shared was published] Basis No clause governs this; it is this repository's own dependency pin. [code-derived] Raised by Rehearsing the documented clone-and-build steps against the published repositories. Change The submodule advances to the published tip of rt-common-shared's MBS branch. Its content is unchanged from the commit previously pinned; only the hashes differ. Verification T2: a clean clone of this branch now initialises its submodule from the public remote and builds. Not in this change Anything in rt-common-shared, whose content this only follows. --- subprojects/rt-common-shared | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subprojects/rt-common-shared b/subprojects/rt-common-shared index 0bab3d1..5c5b8ce 160000 --- a/subprojects/rt-common-shared +++ b/subprojects/rt-common-shared @@ -1 +1 @@ -Subproject commit 0bab3d165085287320ab7742941f6ba1902e5afe +Subproject commit 5c5b8ced5a0346ad5ceb8c1890c5e94c3b217045 From 59324f8be26a8aef828284025e7d9be6c1d9f611 Mon Sep 17 00:00:00 2001 From: "Jordi J. Gimenez" Date: Tue, 15 Sep 2026 19:48:11 +0200 Subject: [PATCH 17/19] mbstf: build against rt-libflute's MBS profile branch Problem This repository pinned a commit on rt-libflute's Raptor line while rt-mbs-client pinned a release tag, so the two components of the same delivery chain were built against different versions of the same library. Neither pin carried what the other component needed, and the client's did not carry what the client itself needed. [code-derived: the two pins] Basis No clause governs this; it is this repository's own dependency pin. [code-derived] Raised by User request that both components be pinned so a third party can build and run the demo end to end. Change revision = feature/mbs-profile, which both this repository and rt-mbs-client now use. It carries the FLUTE-layer obligations this repository depends on -- the TS 26.346 annex L.6 profiled FDT schema, the scheme-specific FEC OTI, suppression of Transfer-Length under the MBMS Download Profile, the split expiry setter and the RFC 5053 Raptor scheme -- alongside the receiver work the client needs. Every commit on it is already on an open pull request; that branch's own merge commit records which, and what was resolved. The wrap comment is rewritten to say this rather than describing the old pin. Verification T2: a clean clone of this branch, with the new pin, builds through to open5gs-mbstfd. Not in this change The pull requests that branch mirrors, which are unaffected. --- subprojects/rt-libflute.wrap | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/subprojects/rt-libflute.wrap b/subprojects/rt-libflute.wrap index 0efc9d2..7a4f189 100644 --- a/subprojects/rt-libflute.wrap +++ b/subprojects/rt-libflute.wrap @@ -1,18 +1,19 @@ -# Pinned to a commit on 5G-MAG's own feature/raptor-raptorq-fec (pull request #61, which is based on -# #62). That branch carries the MBS-compliance work this repository's FLUTE-layer obligations depend -# on: the TS 26.346 annex L.6 profiled FDT schema, the scheme-specific FEC OTI, suppression of -# Transfer-Length under the MBMS Download Profile, the split expiry setter, and the RFC 5053 Raptor -# scheme. None of it is in a release tag yet. +# The MBS profile branch, which both this repository and rt-mbs-client are built against, so the +# two components share one libflute rather than each pinning a different line of it. # -# Repoint this at a tag once #62 and #61 have merged and a release carries them. +# It carries the FLUTE-layer obligations this repository depends on -- the TS 26.346 annex L.6 +# profiled FDT schema, the scheme-specific FEC OTI, suppression of Transfer-Length under the MBMS +# Download Profile, the split expiry setter and the RFC 5053 Raptor scheme -- alongside the +# receiver work the MBS Client needs. Every commit on it is already on an open pull request +# (#61, #62, #64, #68); see that branch's own merge commit for what it contains and why. # -# The revision below is a real, fetchable commit on 5G-MAG. Nothing here is patched locally: if -# subprojects/rt-libflute exists as a checkout it must be detached at exactly this commit, because -# meson's subproject directory precedence means an existing checkout silently wins over this file. -# `git -C subprojects/rt-libflute rev-parse HEAD` should print the revision below and nothing else. +# Repoint this at a tag once those pull requests have merged and a release carries them. +# +# If subprojects/rt-libflute exists as a checkout it must be on this branch, because meson's +# subproject directory precedence means an existing checkout silently wins over this file. [wrap-git] url = https://github.com/5G-MAG/rt-libflute.git -revision = 45ac74c14e02d58a4c9cef96f28d99fc598ac06d +revision = feature/mbs-profile method = cmake #diff_files = rt-libflute/IpSec-xfrm_algo.patch From efbce81e845bd69639b4b7a978efa04eae7a5a23 Mon Sep 17 00:00:00 2001 From: "Jordi J. Gimenez" Date: Wed, 16 Sep 2026 13:20:57 +0200 Subject: [PATCH 18/19] objectstore: assume application/octet-stream when an origin sends no Content-Type Closes #74 Problem Metadata's media type is default-constructed empty and filled from the origin's HTTP Content-Type header. An origin that omits the header left it empty, and both packagers pass it straight to the FLUTE layer (ObjectCarouselPackager.cc:497, ObjectListPackager.cc:328). The object was then described by an FDT File element carrying no Content-Type, which the MBMS Download Profile requires. [code-derived: src/mbstf/ObjectStore.cc:41] Basis The obligation is inherited, not stated directly. TS 26.517 V18.6.0 clause 6.2.1 binds the MBSTF to the profile: "If FLUTE [12] is used to realise the Object Distribution Method, the MBS Distribution Session shall conform to the MBMS Download Profile as defined in clause L.4 of TS 26.346 [7] with the additional requirements in clause 6.2 of the present document." TS 26.346 V18.2.0 clause L.4.2 then lists Content-Type first among the attributes that shall be carried in the FDT. Clause 6.2.3.5's object list requires only the object's URL and its availability start and end times, and names no media type. The substitution has its own source, and it is available here because the MBSTF is the recipient of the ingested object rather than its author. RFC 9110 clause 8.3: "If a Content-Type header field is not present, the recipient MAY either assume a media type of "application/octet-stream" ([RFC2046], Section 4.5.1) or examine the data to determine its type." Raised by Reading the authority while auditing rt-libflute against the profile, which found the corresponding sender-side gap. Change The media type is normalised on write, in the full constructor, both setters and the default constructor, so a stored Metadata is never typeless. Assumed rather than sniffed: the same clause notes that user agents examine content and override the received type inconsistently, so sniffing would make the value depend on which implementation looked. The MBSTF owns this rather than the FLUTE library. rt-libflute refuses an entry with no content type under a 3GPP profile (5G-MAG/rt-libflute#110) because a sender cannot invent a media type for content it did not author; the MBSTF can, being the recipient RFC 9110 addresses. Verification T2: full MBS Broadcast demo end to end over the radio. Zero Content-Type refusals, zero MBSTF errors, 591 objects completed at the client, 62 held, and every object carried a content type with none empty. Not in this change Sniffing the object's content, and any per-ingest-session configuration of a default media type. --- src/mbstf/ObjectStore.cc | 5 +++-- src/mbstf/ObjectStore.hh | 20 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/mbstf/ObjectStore.cc b/src/mbstf/ObjectStore.cc index adb630b..1f00b88 100644 --- a/src/mbstf/ObjectStore.cc +++ b/src/mbstf/ObjectStore.cc @@ -38,7 +38,8 @@ MBSTF_NAMESPACE_START ObjectStore::Metadata::Metadata() :m_objectId() - ,m_mediaType() + // An object with no stated media type is octet-stream, not typeless: see mediaType(). + ,m_mediaType(Metadata::defaultMediaType()) ,m_originalUrl() ,m_fetchedUrl() ,m_acquisitionId() @@ -62,7 +63,7 @@ ObjectStore::Metadata::Metadata(const std::string &object_id, const std::string std::optional obj_distribution_base_url, const std::optional &cache_expires) :m_objectId(object_id) - ,m_mediaType(media_type) + ,m_mediaType(media_type.empty() ? Metadata::defaultMediaType() : media_type) ,m_originalUrl(url) ,m_fetchedUrl(fetched_url) ,m_acquisitionId(acquisition_id) diff --git a/src/mbstf/ObjectStore.hh b/src/mbstf/ObjectStore.hh index 92ba803..8f39cd2 100644 --- a/src/mbstf/ObjectStore.hh +++ b/src/mbstf/ObjectStore.hh @@ -194,8 +194,24 @@ public: Metadata &acquisitionId(const std::string &acquistion_id) { m_acquisitionId = acquistion_id; return *this;}; const std::string &mediaType() const {return m_mediaType;}; - Metadata &mediaType(const std::string &media_type) {m_mediaType = media_type; return *this;}; - Metadata &mediaType(std::string &&media_type) {m_mediaType = std::move(media_type); return *this;}; + /* An origin that sends no Content-Type leaves this empty, and the object then cannot be + described conformantly: TS 26.517 clause 6.2.1 binds the MBSTF to the MBMS Download + Profile, whose clause L.4.2 requires Content-Type in the FDT. The MBSTF is the recipient + of the ingested object, and RFC 9110 clause 8.3 gives a recipient that choice: "If a + Content-Type header field is not present, the recipient MAY either assume a media type of + "application/octet-stream" ([RFC2046], Section 4.5.1) or examine the data to determine its + type." Assumed rather than sniffed, sniffing being what that clause's own note says user + agents do inconsistently. See 5G-MAG/rt-mbs-transport-function#74. */ + static const std::string &defaultMediaType() { + static const std::string kDefault("application/octet-stream"); + return kDefault; + }; + Metadata &mediaType(const std::string &media_type) { + m_mediaType = media_type.empty() ? defaultMediaType() : media_type; return *this;}; + Metadata &mediaType(std::string &&media_type) { + if (media_type.empty()) { m_mediaType = defaultMediaType(); } + else { m_mediaType = std::move(media_type); } + return *this;}; From 316e425d4427f51c5ec45c986d8009399c57b38f Mon Sep 17 00:00:00 2001 From: "Jordi J. Gimenez" Date: Wed, 16 Sep 2026 17:57:40 +0200 Subject: [PATCH 19/19] ingest: infer a missing media type from the object name, or fail the ingest Refs #74 Problem An object ingested from an origin that sends no Content-Type left the MBSTF with no media type, and the object was then described by an FDT File element carrying no Content-Type. [code-derived: src/mbstf/PullObjectIngester.cc] Basis TS 26.517 V18.6.0 clause 6.2.1 binds the MBSTF to the MBMS Download Profile, and TS 26.346 V18.2.0 clause L.4.2 lists Content-Type first among the attributes that "shall be carried in the FDT sent by the FLUTE sender". Raised by Review by davidjwbbc on #74, correcting an earlier version of this change. Change Where the origin sends no Content-Type, the media type is inferred from the object's filename extension, and where that fails the ingest fails through the existing emitObjectPullIngestFailedEvent() path. The object is not sent. This replaces the previous commit efbce81, reverted here, which assumed application/octet-stream. That was wrong: RFC 9110 clause 8.3 offers that value to an HTTP recipient deciding how to treat a body, not to a sender asserting a media type into an FDT that receivers rely on, and a wrong Content-Type on the wire is worse than a refused object because a receiver cannot tell it is wrong. Inference consults a built-in media table before /etc/mime.types, deliberately: the system file is a general-purpose desktop mapping and is wrong for a media service, mapping ".ts" on a stock Ubuntu to text/vnd.trolltech.linguist, a Qt translation source, rather than to an MPEG-2 transport stream. The system file is still consulted for extensions this service has no opinion about. The object's content is not examined. RFC 9110 clause 8.3 calls that out as the other option open to a recipient and records that implementations doing it disagree, which would make the media type one this MBSTF chose rather than one the service defined. Verification T1: the inference function over eight object names, covering .mpd, .m4s, .ts, .vtt, an upper-case extension, a query and fragment, an unknown extension and a name with no extension. All eight resolve as intended. The ".ts" case fails against /etc/mime.types alone, which is what produced the ordering above. T2 for no regression only: full MBS Broadcast demo end to end, 63 objects held, 412 completed, 4078 crc=OK, zero ingest failures. The demo's origin sets Content-Type on everything it serves, so neither inference nor the refusal path was reached by that run; the refusal branch is code-derived. Not in this change Content sniffing, and the per-acquisition Content-Type proposed in 5G-MAG/Standards#192, which is Rel-20 and would need backporting. --- src/mbstf/MediaTypeInference.cc | 110 ++++++++++++++++++++++++++++++++ src/mbstf/MediaTypeInference.hh | 35 ++++++++++ src/mbstf/ObjectStore.cc | 5 +- src/mbstf/ObjectStore.hh | 20 +----- src/mbstf/PullObjectIngester.cc | 31 ++++++++- src/mbstf/meson.build | 2 + 6 files changed, 181 insertions(+), 22 deletions(-) create mode 100644 src/mbstf/MediaTypeInference.cc create mode 100644 src/mbstf/MediaTypeInference.hh diff --git a/src/mbstf/MediaTypeInference.cc b/src/mbstf/MediaTypeInference.cc new file mode 100644 index 0000000..e108f05 --- /dev/null +++ b/src/mbstf/MediaTypeInference.cc @@ -0,0 +1,110 @@ +/* + * License: 5G-MAG Public License (v1.0) + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "MediaTypeInference.hh" + +MBSTF_NAMESPACE_START + +namespace { + +/* Enough to describe what an MBS service actually carries when the origin says nothing: DASH and + HLS manifests, ISOBMFF and MPEG-2 TS segments, WebVTT and TTML subtitles, and the still images a + service announcement refers to. Consulted only when /etc/mime.types is absent or silent. */ +const std::map &builtinTypes() +{ + static const std::map types{ + {"mpd", "application/dash+xml"}, + {"m3u8", "application/vnd.apple.mpegurl"}, + {"m4s", "video/iso.segment"}, + {"mp4", "video/mp4"}, + {"m4a", "audio/mp4"}, + {"m4v", "video/mp4"}, + {"cmfv", "video/mp4"}, + {"cmfa", "audio/mp4"}, + {"cmft", "application/mp4"}, + {"ts", "video/mp2t"}, + {"aac", "audio/aac"}, + {"vtt", "text/vtt"}, + {"ttml", "application/ttml+xml"}, + {"xml", "application/xml"}, + {"json", "application/json"}, + {"txt", "text/plain"}, + {"jpg", "image/jpeg"}, + {"jpeg", "image/jpeg"}, + {"png", "image/png"}, + }; + return types; +} + +/* /etc/mime.types is a sequence of " [...]" lines, with "#" comments. */ +const std::map &systemTypes() +{ + static const std::map types = []{ + std::map result; + std::ifstream in("/etc/mime.types"); + if (!in) return result; + std::string line; + while (std::getline(in, line)) { + const auto hash = line.find('#'); + if (hash != std::string::npos) line.erase(hash); + std::istringstream fields(line); + std::string media_type; + if (!(fields >> media_type)) continue; + std::string extension; + while (fields >> extension) result.emplace(extension, media_type); + } + return result; + }(); + return types; +} + +std::optional extensionOf(const std::string &url) +{ + /* The path only: a query or fragment is not part of the filename. */ + std::string path = url.substr(0, url.find_first_of("?#")); + const auto slash = path.find_last_of('/'); + const std::string name = (slash == std::string::npos) ? path : path.substr(slash + 1); + const auto dot = name.find_last_of('.'); + if (dot == std::string::npos || dot + 1 >= name.size()) return std::nullopt; + std::string extension = name.substr(dot + 1); + std::transform(extension.begin(), extension.end(), extension.begin(), + [](unsigned char c){ return std::tolower(c); }); + return extension; +} + +} // namespace + +std::optional inferMediaTypeFromUrl(const std::string &url) +{ + const auto extension = extensionOf(url); + if (!extension) return std::nullopt; + + /* The built-in table is consulted first, and deliberately. /etc/mime.types is a general-purpose + desktop mapping and is wrong for several extensions a media service uses: on a stock Ubuntu it + maps ".ts" to text/vnd.trolltech.linguist, a Qt translation source, rather than to the MPEG-2 + transport stream an MBS service would be carrying. The system file is still consulted, for + extensions this service has no opinion about. */ + const auto &builtin = builtinTypes(); + const auto builtin_entry = builtin.find(*extension); + if (builtin_entry != builtin.end()) return builtin_entry->second; + + const auto &system_types = systemTypes(); + const auto system_entry = system_types.find(*extension); + if (system_entry != system_types.end()) return system_entry->second; + + return std::nullopt; +} + +MBSTF_NAMESPACE_STOP + +/* vim:ts=8:sts=4:sw=4:expandtab: + */ diff --git a/src/mbstf/MediaTypeInference.hh b/src/mbstf/MediaTypeInference.hh new file mode 100644 index 0000000..3888efb --- /dev/null +++ b/src/mbstf/MediaTypeInference.hh @@ -0,0 +1,35 @@ +#ifndef _MBS_TF_MEDIA_TYPE_INFERENCE_HH_ +#define _MBS_TF_MEDIA_TYPE_INFERENCE_HH_ +/* + * License: 5G-MAG Public License (v1.0) + */ + +#include +#include + +#include "common.hh" + +MBSTF_NAMESPACE_START + +/** Infer an object's media type from its URL when the origin sent no Content-Type. + * + * TS 26.517 V18.6.0 clause 6.2.1 binds the MBSTF to the MBMS Download Profile, whose + * TS 26.346 V18.2.0 clause L.4.2 requires Content-Type in the FDT, so an object with no media + * type cannot be described conformantly and must not be sent. + * + * Inference is by filename extension, taken from the system's own /etc/mime.types where that + * file exists and from a small built-in table otherwise. The media type is not guessed from the + * object's content: that is what RFC 9110 clause 8.3 calls examining the data, and the same + * clause records that implementations doing so disagree, which would make the type an MBSTF + * chose rather than one the service defined. + * + * Returns no value when the extension is unknown. The caller fails the ingest in that case + * rather than inventing a type. + */ +std::optional inferMediaTypeFromUrl(const std::string &url); + +MBSTF_NAMESPACE_STOP + +/* vim:ts=8:sts=4:sw=4:expandtab: + */ +#endif /* _MBS_TF_MEDIA_TYPE_INFERENCE_HH_ */ diff --git a/src/mbstf/ObjectStore.cc b/src/mbstf/ObjectStore.cc index 1f00b88..adb630b 100644 --- a/src/mbstf/ObjectStore.cc +++ b/src/mbstf/ObjectStore.cc @@ -38,8 +38,7 @@ MBSTF_NAMESPACE_START ObjectStore::Metadata::Metadata() :m_objectId() - // An object with no stated media type is octet-stream, not typeless: see mediaType(). - ,m_mediaType(Metadata::defaultMediaType()) + ,m_mediaType() ,m_originalUrl() ,m_fetchedUrl() ,m_acquisitionId() @@ -63,7 +62,7 @@ ObjectStore::Metadata::Metadata(const std::string &object_id, const std::string std::optional obj_distribution_base_url, const std::optional &cache_expires) :m_objectId(object_id) - ,m_mediaType(media_type.empty() ? Metadata::defaultMediaType() : media_type) + ,m_mediaType(media_type) ,m_originalUrl(url) ,m_fetchedUrl(fetched_url) ,m_acquisitionId(acquisition_id) diff --git a/src/mbstf/ObjectStore.hh b/src/mbstf/ObjectStore.hh index 8f39cd2..92ba803 100644 --- a/src/mbstf/ObjectStore.hh +++ b/src/mbstf/ObjectStore.hh @@ -194,24 +194,8 @@ public: Metadata &acquisitionId(const std::string &acquistion_id) { m_acquisitionId = acquistion_id; return *this;}; const std::string &mediaType() const {return m_mediaType;}; - /* An origin that sends no Content-Type leaves this empty, and the object then cannot be - described conformantly: TS 26.517 clause 6.2.1 binds the MBSTF to the MBMS Download - Profile, whose clause L.4.2 requires Content-Type in the FDT. The MBSTF is the recipient - of the ingested object, and RFC 9110 clause 8.3 gives a recipient that choice: "If a - Content-Type header field is not present, the recipient MAY either assume a media type of - "application/octet-stream" ([RFC2046], Section 4.5.1) or examine the data to determine its - type." Assumed rather than sniffed, sniffing being what that clause's own note says user - agents do inconsistently. See 5G-MAG/rt-mbs-transport-function#74. */ - static const std::string &defaultMediaType() { - static const std::string kDefault("application/octet-stream"); - return kDefault; - }; - Metadata &mediaType(const std::string &media_type) { - m_mediaType = media_type.empty() ? defaultMediaType() : media_type; return *this;}; - Metadata &mediaType(std::string &&media_type) { - if (media_type.empty()) { m_mediaType = defaultMediaType(); } - else { m_mediaType = std::move(media_type); } - return *this;}; + Metadata &mediaType(const std::string &media_type) {m_mediaType = media_type; return *this;}; + Metadata &mediaType(std::string &&media_type) {m_mediaType = std::move(media_type); return *this;}; diff --git a/src/mbstf/PullObjectIngester.cc b/src/mbstf/PullObjectIngester.cc index 17e7dac..28bd6d3 100644 --- a/src/mbstf/PullObjectIngester.cc +++ b/src/mbstf/PullObjectIngester.cc @@ -27,6 +27,7 @@ #include "PullObjectIngester.hh" #include "hash.hh" #include "Curl.hh" +#include "MediaTypeInference.hh" #include "ObjectStore.hh" LIBMPDPP_NAMESPACE_USING(BaseURL); @@ -257,7 +258,35 @@ void PullObjectIngester::doObjectIngest() { ogs_debug("Received %ld bytes of data", bytesReceived); std::string fetched_url = URI(m_curl->getPermanentRedirectUrl()).resolveUsingBaseURLs(std::list{BaseURL(item.url())}).str(); if (fetched_url.empty()) fetched_url = item.url(); - ObjectStore::Metadata metadata(item.objectId(), m_curl->getContentType(), item.url(), fetched_url, item.acquisitionId(), m_curl->getLastModified(), item.objIngestBaseUrl(), item.objDistributionBaseUrl()); + + /* An object with no media type cannot be described conformantly: TS 26.517 V18.6.0 + clause 6.2.1 binds the MBSTF to the MBMS Download Profile, and TS 26.346 V18.2.0 + clause L.4.2 lists Content-Type first among the attributes that "shall be carried + in the FDT sent by the FLUTE sender". + + Where the origin sent none, the type is inferred from the object's filename + extension. Where that fails the ingest fails, rather than the MBSTF asserting a + media type nobody established: a wrong Content-Type on the wire is worse than a + refused object, because a receiver has no way to tell it is wrong. */ + std::string media_type = m_curl->getContentType(); + if (media_type.empty()) { + auto inferred = inferMediaTypeFromUrl(item.url()); + if (!inferred) { + ogs_warn("Ingest of [%s] failed: the origin sent no Content-Type and none " + "could be inferred from the object name; an object with no media " + "type cannot be carried in a conformant FDT", + item.url().c_str()); + emitObjectPullIngestFailedEvent(item, item.url(), + ObjectIngester::IngestFailedEvent::GENERAL_ERROR); + m_ingestItemsMutex->lock(); // lock so that the lock_guard can release properly + return; + } + ogs_info("Ingest of [%s]: origin sent no Content-Type, inferred [%s] from the " + "object name", item.url().c_str(), inferred->c_str()); + media_type = *inferred; + } + + ObjectStore::Metadata metadata(item.objectId(), media_type, item.url(), fetched_url, item.acquisitionId(), m_curl->getLastModified(), item.objIngestBaseUrl(), item.objDistributionBaseUrl()); /* re-get metadata from ObjectStore as it may have changed */ try { auto &meta = objectStore()->getMetadata(item.objectId()); diff --git a/src/mbstf/meson.build b/src/mbstf/meson.build index c9a7f6a..58a180e 100644 --- a/src/mbstf/meson.build +++ b/src/mbstf/meson.build @@ -113,6 +113,8 @@ libmbstf_dist_sources = files(''' MBSTFEventHandler.cc MBSTFEventHandler.hh MBSTFNetworkFunction.hh + MediaTypeInference.cc + MediaTypeInference.hh MimeContentType.cc MimeContentType.hh NfServer.cc