Skip to content

Commit 521a448

Browse files
committed
interpret Annex B sequences
Servers are not supposed to use Annex B sequences in RTP payloads, but V380 cameras do. Retina promises that it "works around brokenness in cheap closed-source cameras", so let's try to make this work. As noted by @Curid in #108, there's prior art: gortsp does Annex B decoding. ffmpeg also appears to support this, perhaps by accident: the output from its RTP layer is in Annex B format, and it doesn't error on Annex B sequences in RTP, so callers implicitly get the same result with proper or improper framing. Along the way, notice NALs that illegally contain the sequences `00 00 00` or `00 00 02` and ones that illegally end with `00`. We'll see if this happens in practice, but I'd prefer to explicitly consider how to work around it if so, rather than passing along bad data unknowingly. Throughput on my laptop falls from 3.4 GiB/s to 2.9 GiB/s on the `depacketize/h264_aac_discard` test, which uses realistic data. I think that's acceptable. The code avoids unnecessary copies (or refcounts even) and uses `memchr::memchr` to optimize the case of looking for the next zero byte. From skimming a profile, most of the added time is in `memchr`, so I don't think we're going to do much better than that. Fixes #68 Fixes #108
1 parent 5ce3e3a commit 521a448

File tree

9 files changed

+482
-57
lines changed

9 files changed

+482
-57
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
## unreleased
22

33
* improve some error messages on bad H.264 `sprop-parameter-sets`
4+
* interoperate with V380 cameras by interpreting Annex B sequences in contexts
5+
where single NALs are otherwise expected.
6+
(Fixes [#68](https://github.com/scottlamb/retina/issues/68),
7+
[[#108](https://github.com/scottlamb/retina/issues/108)]).
48

59
## `v0.4.10` (2024-08-19)
610

Cargo.lock

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ h264-reader = "0.7.0"
3030
hex = "0.4.3"
3131
http-auth = "0.1.2"
3232
log = "0.4.8"
33+
memchr = "2.7.4"
3334
once_cell = "1.7.2"
3435
pin-project = "1.0.7"
3536
pretty-hex = "0.4.0"

benches/client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ fn make_test_data(max_payload_size: u16) -> Bytes {
7777
3575, 3144, 3431, 3317, 3154, 3387, 3630, 3232, 3574, 3254, 4198, 4235, 4898, 4890, 4854,
7878
4854, 4863, 4652, 4227, 4101, 3867, 3870, 3716, 3074, 3253, 3267, 3192, 3995, 3904, 3781,
7979
];
80-
let mut dummy_frame = vec![0; 1048576];
80+
let mut dummy_frame = vec![1; 1048576];
8181
dummy_frame[4] = h264_reader::nal::UnitType::SliceLayerWithoutPartitioningIdr.id();
8282
let mut p =
8383
retina::codec::h264::Packetizer::new(max_payload_size, 0, 24104, 96, 0x4cacc3d1).unwrap();

clippy.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
allow-print-in-tests = true

fuzz/Cargo.lock

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

fuzz/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ cargo-fuzz = true
1212
[dependencies]
1313
bytes = "1"
1414
libfuzzer-sys = "0.4"
15+
memchr = "2.7.4"
1516

1617
[dependencies.retina]
1718
path = ".."

fuzz/fuzz_targets/roundtrip_h264.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use libfuzzer_sys::fuzz_target;
1212
use std::num::NonZeroU32;
1313

1414
fuzz_target!(|data: &[u8]| {
15-
if data.len() < 2 {
15+
if data.len() < 3 {
1616
return;
1717
}
1818
let conn_ctx = retina::ConnectionContext::dummy();
@@ -31,6 +31,15 @@ fuzz_target!(|data: &[u8]| {
3131
)
3232
.unwrap();
3333
let timestamp = retina::Timestamp::new(0, NonZeroU32::new(90_000).unwrap(), 0).unwrap();
34+
35+
// data[2] is the NAL header.
36+
// data[3..] should not contain Annex B separators; the depacketizer will split them, and the
37+
// result won't be the same as expected below.
38+
let finder = memchr::memmem::Finder::new(&[0, 0, 1]);
39+
if finder.find(&data[3..]).is_some() {
40+
return;
41+
}
42+
3443
if p.push(timestamp, Bytes::copy_from_slice(&data[2..]))
3544
.is_err()
3645
{

0 commit comments

Comments
 (0)