-
Notifications
You must be signed in to change notification settings - Fork 100
frontend: Add support to video decoder using CDR #3407
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
frontend: Add support to video decoder using CDR #3407
Conversation
Reviewer's GuideIntegrates Foxglove ROS message parsing into the Zenoh inspector to support CDR-encoded CompressedVideo frames: extends the payload model to ZBytes with encoding/scheme metadata, fetches and parses the .msg definition at runtime, decodes video payloads via MessageReader into raw frame data, adjusts message formatting logic based on encoding, and updates build assets and dependencies to include .msg definitions. Sequence diagram for CDR-encoded CompressedVideo message decodingsequenceDiagram
participant Zenoh as ZenohInspector.vue
participant Server as Backend Server
participant Foxglove as MessageReader
participant User as actor
User->>Zenoh: Selects a CompressedVideo message
Zenoh->>Server: GET /public/msgs/CompressedVideo.msg
Server-->>Zenoh: .msg definition
Zenoh->>Foxglove: parseMessageDefinition(definition)
Zenoh->>Foxglove: reader.readMessage(payload)
Foxglove-->>Zenoh: { data: Uint8Array }
Zenoh->>User: Display raw video frame
ER diagram for CompressedVideo.msg structureerDiagram
COMPRESSED_VIDEO {
int sec
uint nanosec
string frame_id
uint8[] data
string format
}
Class diagram for updated ZenohMessage and message handlingclassDiagram
class ZenohMessage {
+string topic
+ZBytes payload
+string encoding
+string scheme
+Date timestamp
}
class Sample {
+keyexpr()
+payload()
+encoding()
}
class MessageReader {
+readMessage(bytes): object
}
ZenohMessage <.. Sample : created from
ZenohMessage o-- ZBytes
ZenohMessage o-- Date
ZenohInspector ..> MessageReader : uses
ZenohInspector ..> ZenohMessage : manages
ZenohInspector ..> Sample : subscribes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @patrickelectric - I've reviewed your changes - here's some feedback:
- Move the parsing of CompressedVideo.msg out of the computed getter—parse it once (e.g. on mount) and reuse the resulting definition/MessageReader to avoid repeated work on every access.
- Instead of fetching the .msg file at runtime with axios, consider bundling or importing the definition at build time (or lazy-loading it once) to eliminate the HTTP dependency and improve startup reliability.
- Add error handling around the axios fetch and message parsing steps to gracefully handle missing or malformed .msg files instead of assuming they’ll always load successfully.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Move the parsing of CompressedVideo.msg out of the computed getter—parse it once (e.g. on mount) and reuse the resulting definition/MessageReader to avoid repeated work on every access.
- Instead of fetching the .msg file at runtime with axios, consider bundling or importing the definition at build time (or lazy-loading it once) to eliminate the HTTP dependency and improve startup reliability.
- Add error handling around the axios fetch and message parsing steps to gracefully handle missing or malformed .msg files instead of assuming they’ll always load successfully.
## Individual Comments
### Comment 1
<location> `core/frontend/src/components/zenoh-inspector/ZenohInspector.vue:134` </location>
<code_context>
import RawVideoPlayer from './RawVideoPlayer.vue'
+const CompressedVideo = await axios.get('/public/msgs/CompressedVideo.msg').then((response) => response.data as string)
+
interface ZenohMessage {
</code_context>
<issue_to_address>
Top-level await may cause issues in module scope.
Consider moving this logic into an async lifecycle hook or verify that your build system supports top-level await to avoid runtime errors.
</issue_to_address>
### Comment 2
<location> `core/frontend/src/components/zenoh-inspector/ZenohInspector.vue:184` </location>
<code_context>
return null
}
- return new Uint8Array(JSON.parse(this.current_message.payload)?.data)
+ const definition = parseMessageDefinition(CompressedVideo)
+ const reader = new MessageReader(definition)
+ const msg: { data: Uint8Array } = reader.readMessage(this.current_message.payload.to_bytes())
+ return msg.data
},
},
</code_context>
<issue_to_address>
Repeated parsing of message definition and reader instantiation may impact performance.
Consider caching the parsed definition and MessageReader instance to avoid repeated instantiation if the message type remains constant.
Suggested implementation:
```
if (!this._compressedVideoDefinition) {
this._compressedVideoDefinition = parseMessageDefinition(CompressedVideo)
}
if (!this._compressedVideoReader) {
this._compressedVideoReader = new MessageReader(this._compressedVideoDefinition)
}
const msg: { data: Uint8Array } = this._compressedVideoReader.readMessage(this.current_message.payload.to_bytes())
return msg.data
```
```
},
// Cache for message definition and reader
_compressedVideoDefinition: null,
_compressedVideoReader: null,
```
</issue_to_address>
### Comment 3
<location> `core/frontend/src/components/zenoh-inspector/ZenohInspector.vue:209` </location>
<code_context>
topic_type: this.topic_types[message.topic] || 'Unknown',
message_type: this.topic_message_types[message.topic] || 'Unknown',
- payload: message.payload,
+ payload: message.payload.toString(),
}
</code_context>
<issue_to_address>
Converting payload to string may not be appropriate for all encodings.
Explicitly handle different payload encodings to prevent issues with binary or non-UTF8 data.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
payload: message.payload.toString(),
=======
payload: (() => {
// Try to detect encoding, default to 'utf-8'
const encoding = message.encoding || 'utf-8';
try {
if (encoding === 'utf-8' || encoding === 'ascii') {
// Try to decode as text
if (typeof TextDecoder !== "undefined") {
return new TextDecoder(encoding).decode(message.payload);
} else {
// Fallback for environments without TextDecoder
return message.payload.toString();
}
} else if (encoding === 'base64') {
// If already base64, just convert
return btoa(String.fromCharCode.apply(null, message.payload));
} else {
// Unknown or binary encoding, show as hex
return Array.from(message.payload)
.map((b: number) => b.toString(16).padStart(2, '0'))
.join(' ');
}
} catch (e) {
// Fallback: show as hex if decoding fails
return Array.from(message.payload)
.map((b: number) => b.toString(16).padStart(2, '0'))
.join(' ');
}
})(),
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `core/frontend/src/components/zenoh-inspector/ZenohInspector.vue:237` </location>
<code_context>
handler: async (sample: Sample) => {
const topic = sample.keyexpr().toString()
const payload = sample.payload()
+ const [encoding, scheme] = sample.encoding().toString().split(';')
+
const message: ZenohMessage = {
</code_context>
<issue_to_address>
Splitting encoding string by ';' assumes a specific format.
This approach may fail if the format changes or extra semicolons are present. Use a more reliable parsing or add validation.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
const [encoding, scheme] = sample.encoding().toString().split(';')
=======
// Robustly parse encoding string, handling extra semicolons and missing parts
const encodingStr = sample.encoding().toString()
let encoding = ''
let scheme = ''
if (encodingStr) {
const parts = encodingStr.split(';').map(part => part.trim()).filter(Boolean)
encoding = parts[0] || ''
scheme = parts[1] || ''
}
>>>>>>> REPLACE
</suggested_fix>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
import Vue from 'vue' | ||
|
||
import RawVideoPlayer from './RawVideoPlayer.vue' | ||
|
||
const CompressedVideo = await axios.get('/public/msgs/CompressedVideo.msg').then((response) => response.data as string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Top-level await may cause issues in module scope.
Consider moving this logic into an async lifecycle hook or verify that your build system supports top-level await to avoid runtime errors.
core/frontend/src/components/zenoh-inspector/ZenohInspector.vue
Outdated
Show resolved
Hide resolved
core/frontend/src/components/zenoh-inspector/ZenohInspector.vue
Outdated
Show resolved
Hide resolved
4122ad3
to
c238733
Compare
@joaoantoniocardoso check now |
payload: string | ||
payload: ZBytes | ||
encoding: string | ||
scheme: string | undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scheme: string | undefined | |
schema: string | undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small typo there. Then we are good to go
Signed-off-by: Patrick José Pereira <[email protected]>
…DR video messages Signed-off-by: Patrick José Pereira <[email protected]>
Signed-off-by: Patrick José Pereira <[email protected]>
Signed-off-by: Patrick José Pereira <[email protected]>
Signed-off-by: Patrick José Pereira <[email protected]>
c238733
to
569e4c5
Compare
Summary by Sourcery
Add support for compressed video message decoding in the Zenoh inspector using CDR serialization
New Features:
Enhancements:
Build:
Chores: