From 1ee6d95cc31a75a4230462171411deea90dbbd91 Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Tue, 5 Apr 2022 13:09:05 -0700 Subject: [PATCH] tweak handling of data on unassigned channels * Call `note_stale_live555_data` when this happens in playing state. This makes it more consistent with what happens while waiting for a response in the described state, or in playing state on an assigned channel. * Print a hex dump of some initial bytes, enough to analyze it to see if it looks like RTP or RTCP, what the ssrc is, etc. And do the same for other places we log (parts of) packets. --- src/client/mod.rs | 18 +++++++++++++----- src/client/rtp.rs | 5 ++--- src/codec/g723.rs | 3 +-- src/codec/mod.rs | 18 +++++++++++------- src/error.rs | 6 +++++- src/hex.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + 7 files changed, 77 insertions(+), 18 deletions(-) create mode 100644 src/hex.rs diff --git a/src/client/mod.rs b/src/client/mod.rs index 54ccdc4..148b82c 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -1803,11 +1803,19 @@ impl Session { }); let m = match conn.channels.lookup(channel_id) { Some(m) => m, - None => bail!(ErrorInt::RtspUnassignedChannelError { - conn_ctx: *conn.inner.ctx(), - msg_ctx: *msg_ctx, - channel_id, - }), + None => { + note_stale_live555_data( + tokio::runtime::Handle::try_current().ok(), + inner.presentation.tool.as_ref(), + inner.options, + ); + bail!(ErrorInt::RtspUnassignedChannelError { + conn_ctx: *conn.inner.ctx(), + msg_ctx: *msg_ctx, + channel_id, + data: data.into_body(), + }); + } }; let stream = &mut inner.presentation.streams[m.stream_i]; let (timeline, rtp_handler) = match &mut stream.state { diff --git a/src/client/rtp.rs b/src/client/rtp.rs index 9b846e5..55450c9 100644 --- a/src/client/rtp.rs +++ b/src/client/rtp.rs @@ -5,7 +5,6 @@ use bytes::{Buf, Bytes}; use log::{debug, trace}; -use pretty_hex::PrettyHex; use crate::client::PacketItem; use crate::{ConnectionContext, Error, ErrorInt, PacketContext}; @@ -44,7 +43,7 @@ impl std::fmt::Debug for Packet { .field("sequence_number", &self.sequence_number) .field("loss", &self.loss) .field("mark", &self.mark) - .field("payload", &self.payload.hex_dump()) + .field("payload", &crate::hex::LimitedHex::new(&self.payload, 64)) .finish() } } @@ -109,7 +108,7 @@ impl InorderParser { "corrupt RTP header while expecting seq={:04x?}: {:?}\n{:#?}", &self.next_seq, e, - data.hex_dump(), + crate::hex::LimitedHex::new(&data, 64), ), }) })?; diff --git a/src/codec/g723.rs b/src/codec/g723.rs index 609c4c5..7fb3da6 100644 --- a/src/codec/g723.rs +++ b/src/codec/g723.rs @@ -6,7 +6,6 @@ use std::num::NonZeroU32; use bytes::Bytes; -use pretty_hex::PrettyHex; const FIXED_CLOCK_RATE: u32 = 8_000; @@ -53,7 +52,7 @@ impl Depacketizer { if !Self::validate(&pkt) { return Err(format!( "Invalid G.723 packet: {:#?}", - pkt.payload.hex_dump() + crate::hex::LimitedHex::new(&pkt.payload, 64), )); } self.pending = Some(super::AudioFrame { diff --git a/src/codec/mod.rs b/src/codec/mod.rs index 354a4f5..0617e8f 100644 --- a/src/codec/mod.rs +++ b/src/codec/mod.rs @@ -13,7 +13,6 @@ use crate::client::rtp; use crate::ConnectionContext; use crate::Error; use bytes::{Buf, Bytes}; -use pretty_hex::PrettyHex; pub(crate) mod aac; pub(crate) mod g723; @@ -123,7 +122,10 @@ impl std::fmt::Debug for VideoParameters { .field("pixel_dimensions", &self.pixel_dimensions) .field("pixel_aspect_ratio", &self.pixel_aspect_ratio) .field("frame_rate", &self.frame_rate) - .field("extra_data", &self.extra_data.hex_dump()) + .field( + "extra_data", + &crate::hex::LimitedHex::new(&self.extra_data, 256), + ) .finish() } } @@ -143,7 +145,10 @@ impl std::fmt::Debug for AudioParameters { f.debug_struct("AudioParameters") .field("rfc6381_codec", &self.rfc6381_codec) .field("frame_length", &self.frame_length) - .field("extra_data", &self.extra_data.hex_dump()) + .field( + "extra_data", + &crate::hex::LimitedHex::new(&self.extra_data, 256), + ) .finish() } } @@ -201,7 +206,7 @@ impl std::fmt::Debug for AudioFrame { .field("loss", &self.loss) .field("timestamp", &self.timestamp) .field("frame_length", &self.frame_length) - .field("data", &self.data.hex_dump()) + .field("data", &crate::hex::LimitedHex::new(&self.data, 64)) .finish() } } @@ -245,7 +250,7 @@ impl std::fmt::Debug for MessageFrame { .field("stream_id", &self.stream_id) .field("loss", &self.loss) .field("timestamp", &self.timestamp) - .field("data", &self.data.hex_dump()) + .field("data", &crate::hex::LimitedHex::new(&self.data, 64)) .finish() } } @@ -334,8 +339,7 @@ impl std::fmt::Debug for VideoFrame { .field("new_parameters", &self.new_parameters) .field("is_random_access_point", &self.is_random_access_point) .field("is_disposable", &self.is_disposable) - .field("data_len", &self.data.len()) - //.field("data", &self.data.hex_dump()) + .field("data", &crate::hex::LimitedHex::new(&self.data, 64)) .finish() } } diff --git a/src/error.rs b/src/error.rs index 869a64c..6f479d1 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,6 +4,7 @@ use std::{fmt::Display, sync::Arc}; use crate::{ConnectionContext, PacketContext, RtspMessageContext}; +use bytes::Bytes; use thiserror::Error; /// An opaque `std::error::Error + Send + Sync + 'static` implementation. @@ -58,12 +59,15 @@ pub(crate) enum ErrorInt { }, #[error( - "[{conn_ctx}, {msg_ctx}] Received interleaved data on unassigned channel {channel_id}" + "[{conn_ctx}, {msg_ctx}] Received interleaved data on unassigned channel {channel_id}: \n\ + {:?}", + crate::hex::LimitedHex::new(data, 64) )] RtspUnassignedChannelError { conn_ctx: ConnectionContext, msg_ctx: RtspMessageContext, channel_id: u8, + data: Bytes, }, #[error("[{conn_ctx}, {pkt_ctx} stream {stream_id}]: {description}")] diff --git a/src/hex.rs b/src/hex.rs new file mode 100644 index 0000000..122c90c --- /dev/null +++ b/src/hex.rs @@ -0,0 +1,44 @@ +// Copyright (C) 2022 Scott Lamb +// SPDX-License-Identifier: MIT OR Apache-2.0 + +//! Quick wrapper around `pretty-hex` to limit output. +//! +//! TODO: remove this if [wolandr/pretty-hex#9](https://github.com/wolandr/pretty-hex/pull/9) +//! is merged. + +use pretty_hex::PrettyHex; + +pub struct LimitedHex<'a> { + inner: &'a [u8], + max_bytes: usize, +} + +impl<'a> LimitedHex<'a> { + pub fn new(inner: &'a [u8], max_bytes: usize) -> Self { + Self { inner, max_bytes } + } +} + +impl<'a> std::fmt::Debug for LimitedHex<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let omitted = self.inner.len().checked_sub(self.max_bytes); + let print = if omitted.is_some() { + &self.inner[..self.max_bytes] + } else { + self.inner + }; + writeln!(f, "Length: {0} (0x{0:x}) bytes", self.inner.len())?; + writeln!( + f, + "{:#?}", + print.hex_conf(pretty_hex::HexConfig { + title: false, + ..Default::default() + }) + )?; + if let Some(o) = omitted { + write!(f, "\n...{0} (0x{0:x}) bytes not shown...", o)?; + } + Ok(()) + } +} diff --git a/src/lib.rs b/src/lib.rs index 1fb4eaf..d589cd4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,7 @@ use std::ops::Range; mod error; mod rtcp; +mod hex; #[cfg(test)] mod testutil;