cvd: Support media devices in multi-instance fleets - #3136
Open
changyeon-jo wants to merge 1 commit into
Open
changyeon-jo wants to merge 1 commit into
changyeon-jo wants to merge 1 commit into
Conversation
changyeon-jo
force-pushed
the
cvd_multi_instance_media
branch
from
September 3, 2026 21:56
d943d6e to
2e384b6
Compare
changyeon-jo
changed the title
changyeon-jo
force-pushed
the
cvd_multi_instance_media
branch
from
September 3, 2026 22:01
2e384b6 to
5262fa5
Compare
bridadan
reviewed
Sep 9, 2026
bridadan
left a comment
bridadan
left a comment
Collaborator
There was a problem hiding this comment.
Other than my one question everything LGTM
| flag += ":lens_facing=" + device.lens_facing(); | ||
| } | ||
| if (cfg.instances().size() > 1) { | ||
| flag += ":instance=" + std::to_string(i); |
Collaborator
There was a problem hiding this comment.
For existing devices or configuration options, is this how they handle addressing which instance the value applies to when specified using CLI flags? Is there a precedent for this? @ser-io
Author
There was a problem hiding this comment.
Good question. I didn't find any precedent for addressing individual instances inside a repeatable flag. I'd like to move this PR to the pattern displays already use:
- Add an
InstancesMediaproto tolaunch_cvd.proto(per-instance lists of the sameMediaDeviceshape asload_config.proto), with--media_binproto/--media_textprotoflags, the same way--displays_binproto/--displays_textprotowork. - The JSON parser emits
--media_binproto, which lifts the single-instance restriction. The JSON schema doesn't change. - Plain
--mediareturns an error whennum_instances > 1. Today it copies the same devices to every instance, so two instances read from one FIFO or share one/dev/videoN. It's also an error to combine it with--media_*proto. :instance=<i>goes away.
One open question for @ser-io @bridadan: do you know of any users or CI that launch multi-instance fleets with --media today? The new error would break them. If there are some, the fallback is to keep duplicating emulated cameras and only reject FIFO- and device-backed types.
- Add 'instance' property to --media flags and MediaConfig, allowing media devices to target a specific instance index in multi-device fleets. - Update cf_media_configs.cpp to allow multi-instance fleets where a single instance has media configured (has_media_count <= 1), and serialize the target instance index (:instance=<i>) when launching fleets. - Update media.cpp to validate that input_path and input_fps are not empty. - Update assemble_cvd/flags.cc to assign media configs only to the targeted instance matching config.instance_index. - Add unit tests in flags_parser_test.cc verifying multi-instance single-media fleet JSON parsing (targeting instance 1 and instance 0) and rejecting multiple media instances in a fleet. Bug: b/520098369
changyeon-jo
force-pushed
the
cvd_multi_instance_media
branch
from
September 26, 2026 18:08
5262fa5 to
af18290
Compare
changyeon-jo
changed the title
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for media devices in multi-instance fleets launched via
cvd create --config_file=.... Previously, anymediablock in a multi-instance config was rejected (TODO(b/520098369)).cf_media_configs.cppnow allows multi-instance fleets where at most one instance has media configured (has_media_count <= 1), and serializes the target instance index (:instance=<i>) into the--mediaflag when the fleet has more than one instance.instanceproperty to--media/MediaConfig.assemble_cvd/flags.ccassigns media configs only to the instance matchingconfig.instance_index.host/libs/config/media.cppenforces non-emptyinput_pathandinput_fpsviaCF_EXPECT(review feedback from vhost_user_media: Worker POLLHUP handling and shmem_unmap fix #3121).flags_parser_test.cc):ParseMediaMultiInstanceSingleMedia: media on instance 1.ParseMediaMultiInstanceFirstInstanceMedia: media on instance 0.ParseMediaMultiInstanceMultipleMediaFails: more than one instance with media is rejected.Follow-up to / spun off from: #3121
Bug: b/520098369
Testing
bazel test //cuttlefish/host/commands/cvd/cli/parser:flags_parser_test(Passed)bazel build //cuttlefish/host/commands/assemble_cvd:assemble_cvd //cuttlefish/host/commands/run_cvd:run_cvd(Passed)