Skip to content

AMWA NMOS IS-05 1.2-dev / MXL Support#483

Open
jonathan-r-thorpe wants to merge 44 commits into
sony:masterfrom
jonathan-r-thorpe:nmos-mxl-test
Open

AMWA NMOS IS-05 1.2-dev / MXL Support#483
jonathan-r-thorpe wants to merge 44 commits into
sony:masterfrom
jonathan-r-thorpe:nmos-mxl-test

Conversation

@jonathan-r-thorpe
Copy link
Copy Markdown
Contributor

  • Prototype used to demonstrate MXL integration with NMOS

Copy link
Copy Markdown
Contributor

@garethsb garethsb left a comment

Choose a reason for hiding this comment

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

Forgot to submit this last week, sorry...

Comment thread Development/nmos-cpp-node/config.json Outdated
Comment thread Development/nmos-cpp-node/config.json Outdated
Comment thread Development/nmos-cpp-node/config.json Outdated
Comment thread Development/nmos/connection_api.cpp Outdated
Comment thread Development/nmos/is05_schemas/is05_schemas.h Outdated
Comment thread Development/nmos-cpp-node/node_implementation.cpp Outdated
Comment thread Development/nmos-cpp-node/node_implementation.cpp Outdated
Comment on lines +969 to +970
flow = nmos::make_raw_audio_flow(flow_id, source_id, device_id, 48000, 32, model.settings);
flow.data[nmos::fields::media_type] = value::string(U("audio/float32"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⬇️ Maybe we should use make_audio_flow or have another overload of make_raw_audio_flow, so we don't have to rewrite the media_type?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Use nmos::make_coded_audio_flow perhaps?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah... except we want bit_depth, I think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch - will update

Comment thread Development/nmos-cpp-node/node_implementation.cpp Outdated
Comment thread Development/nmos-cpp-node/node_implementation.cpp Outdated
jonathan-r-thorpe and others added 7 commits May 18, 2026 16:54
Co-authored-by: Gareth Sylvester-Bradley <31761158+garethsb@users.noreply.github.com>
Get-NetIPConfiguration fails when multiple adapters map to one interface; use the default route and Get-NetIPAddress instead, and only disable vEthernet (nat) when present.
Comment thread Development/nmos-cpp-node/node_implementation.cpp
Comment thread Sandbox/run_nmos_testing.sh Outdated
@garethsb
Copy link
Copy Markdown
Contributor

@jonathan-r-thorpe @lo-simon If this isn't going to get merged imminently (although that would be nice!) would you consider extracting d499aac into a separate PR so we can have running CI on Windows again?

…and update mxl_video_type to use a string literal. Implement error handling for transport_file usage in node implementation for MXL receivers.
…_flow to accept media_type, updating existing calls in node implementation to use this new overload.
Comment thread Development/nmos/node_resources.cpp Outdated
Comment thread Development/nmos/connection_api.cpp Outdated
Comment thread Development/nmos-cpp-node/node_implementation.cpp Outdated
…plementing UUID generation directly in node initialization. Update auto resolution logic for transport parameters to streamline MXL sender and receiver processing.
…rement to v1.2 for compatibility with Connection API.
…" as valid values. Update related functions and constraints to accommodate the new schema structure.
Comment thread Development/nmos/connection_api.cpp Outdated
Comment thread Development/nmos/node_resources.cpp
Comment thread Development/nmos/node_resources.h Outdated
Copy link
Copy Markdown
Contributor

@garethsb garethsb left a comment

Choose a reason for hiding this comment

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

Looking really good now. Two tiny review comments.

Comment thread Development/nmos-cpp-node/node_implementation.cpp Outdated
Comment thread Development/nmos/connection_api.cpp Outdated
@jonathan-r-thorpe jonathan-r-thorpe marked this pull request as ready for review May 28, 2026 14:26
Copy link
Copy Markdown
Collaborator

@lo-simon lo-simon left a comment

Choose a reason for hiding this comment

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

Shouldn't we validate mxl_domian_id against the UUID pattern? I have marked it in 3 places. The rest of the PR looks good to me.

Comment on lines +420 to +422
const auto mxl_domain_id = impl::fields::mxl_domain_id(model.settings).empty()
? nmos::make_repeatable_id(seed_id, U("/x-nmos/mxl/domain"))
: impl::fields::mxl_domain_id(model.settings);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As mxl_domain_id can be overridden by the user, shouldn't we validate it against the UUID pattern? Maybe it's better to validate it in nmos::make_connection_mxl_sender and nmos::make_connection_mxl_receiver, since those functions can be called directly by the user?

Copy link
Copy Markdown
Contributor

@garethsb garethsb May 29, 2026

Choose a reason for hiding this comment

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

I don't think we usually validate stuff that's come from the config.json file / nmos::settings? if we want to, I suggest doing that as a one-shot validate_settings(const nmos::settings& settings) function declared in nmos/settings.h and then if applications like nmos-cpp-node define their own application-specific settings, they have their own validate_settings function that calls the above one.

Copy link
Copy Markdown
Collaborator

@lo-simon lo-simon May 29, 2026

Choose a reason for hiding this comment

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

I am pointing this out because we could end up with an invalid constraint in the sender and receiver constraints endpoints. I think we should validate it in nmos::make_connection_mxl_sender and nmos::make_connection_mxl_receiver rather than in the settings.

/receivers/receiver_id/constraints
[ { "mxl_domain_id": { "enum": [ "bad_mxl_flow_id" ] }, "mxl_flow_id": {} } ]

/senders/sender_id/constraints
[ { "mxl_domain_id": { "enum": [ "bad_mxl_flow_id" ] }, "mxl_flow_id": { "enum": [ "43bb8367-fc6d-5402-b9e3-77202edf996d" ] } } ]

Copy link
Copy Markdown
Contributor

@garethsb garethsb May 29, 2026

Choose a reason for hiding this comment

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

I get your point that it's "user input" but actually it's only user input from the point of view of the example application, not the library, so I think it's better provided as an opt-in validation function so that you don't pay for what you don't need (e.g. you've done your app level validation of user level input and don't want nmos-cpp to validate it again, or you defined the value programmatically so you know it doesn't need further validation).

From the point of view of the nmos::make_connection_mxl_sender function, it's just another nmos::id parameter. And if we need to validate that, we should validate nmos::id parameters in many functions (after all, it's just a typedef for a utility::string_t...).

Update: I've had a go at providing settings validation: jonathan-r-thorpe#2

}
}

nmos::resource make_connection_mxl_sender(const nmos::id& id, const nmos::id& mxl_domain_id, const nmos::id& mxl_flow_id)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As mxl_domain_id can be overridden by the user, shouldn't we validate it against the UUID pattern? Maybe it's better to validate it in nmos::make_connection_mxl_sender and nmos::make_connection_mxl_receiver, since those functions can be called directly by the user?

return{ is05_versions::v1_2, types::sender, std::move(data), false };
}

nmos::resource make_connection_mxl_receiver(const nmos::id& id, const nmos::id& mxl_domain_id)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As mxl_domain_id can be overridden by the user, shouldn't we validate it against the UUID pattern? Maybe it's better to validate it in nmos::make_connection_mxl_sender and nmos::make_connection_mxl_receiver, since those functions can be called directly by the user?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants