-
Notifications
You must be signed in to change notification settings - Fork 0
test(nodes): fill RTMP protocol test coverage gaps #503
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
Changes from all commits
09e0f87
32eca86
56c0d05
a95ff38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1494,4 +1494,68 @@ mod tests { | |
| "video_data should be empty for SPS/PPS-only access units" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn stamp_with_no_metadata_defaults_to_zero() { | ||
| let mut state = RtmpTimestampState::new(); | ||
| let pkt = Packet::Binary { | ||
| data: bytes::Bytes::from_static(&[0]), | ||
| metadata: None, | ||
| content_type: None, | ||
| }; | ||
| let ts = state.stamp(&pkt, Track::Video, "test"); | ||
| assert_eq!(ts, 0); | ||
| } | ||
|
|
||
| #[test] | ||
| fn config_default_sample_rate() { | ||
| assert_eq!(default_sample_rate(), 48_000); | ||
| } | ||
|
|
||
| #[test] | ||
| fn config_default_channels() { | ||
| assert_eq!(default_channels(), 2); | ||
| } | ||
|
|
||
| #[test] | ||
| fn convert_annexb_sei_and_aud_in_video_data() { | ||
| let mut annexb = Vec::new(); | ||
| // SEI NAL (type 6) | ||
| annexb.extend_from_slice(&[0x00, 0x00, 0x00, 0x01]); | ||
| let sei = [0x06, 0x05, 0x04, 0x03]; | ||
| annexb.extend_from_slice(&sei); | ||
| // AUD NAL (type 9) | ||
| annexb.extend_from_slice(&[0x00, 0x00, 0x00, 0x01]); | ||
| let aud = [0x09, 0x10]; | ||
| annexb.extend_from_slice(&aud); | ||
| // IDR slice (type 5) | ||
| annexb.extend_from_slice(&[0x00, 0x00, 0x00, 0x01]); | ||
| let idr = [0x65, 0x88, 0x84]; | ||
| annexb.extend_from_slice(&idr); | ||
|
|
||
| let result = convert_annexb_to_avcc(&annexb); | ||
|
|
||
| assert!(result.sps_list.is_empty()); | ||
| assert!(result.pps_list.is_empty()); | ||
|
|
||
| let avcc = &result.video_data; | ||
| let mut nal_types = Vec::new(); | ||
| let mut offset = 0; | ||
| while offset + 4 <= avcc.len() { | ||
| let len = u32::from_be_bytes([ | ||
| avcc[offset], | ||
| avcc[offset + 1], | ||
| avcc[offset + 2], | ||
| avcc[offset + 3], | ||
| ]) as usize; | ||
| offset += 4; | ||
| assert!(offset + len <= avcc.len(), "AVCC data truncated"); | ||
| nal_types.push(avcc[offset] & H264_NAL_TYPE_MASK); | ||
| offset += len; | ||
| } | ||
|
|
||
| assert!(nal_types.contains(&6), "SEI should be in video_data"); | ||
| assert!(nal_types.contains(&9), "AUD should be in video_data"); | ||
| assert!(nal_types.contains(&5), "IDR should be in video_data"); | ||
|
Comment on lines
+1520
to
+1559
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: SEI/AUD retention test intentionally matches current AVCC conversion semantics The new Was this helpful? React with 👍 or 👎 to provide feedback. Debug |
||
| } | ||
| } | ||
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.
📝 Info: New no-metadata timestamp test documents an existing fallback, not a new runtime path
The added
stamp_with_no_metadata_defaults_to_zerotest covers the currentRtmpTimestampState::stampfallback where missingPacketMetadata::timestamp_usbecomespkt_ms = 0(crates/nodes/src/transport/rtmp.rs:682-693). I did not flag this as a bug because encoded media packets normally carry metadata through the video/audio encoders, and the test is documenting a defensive fallback rather than changing behavior. If reviewers expect RTMP publishing to reject untimestamped media instead of collapsing timestamps to zero/monotonic increments, that would be a product-contract decision outside this test-only PR.Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
Playground