eric/pmomusic
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases 3 Wiki Activity

Refactor: Extract FLAC frame detection to shared module

Browse Source 
Both StreamingFlacSink and StreamingOggFlacSink had duplicate FLAC frame
detection logic. The OGG-FLAC sink had the improved validation, but the
regular FLAC sink was still using the old unvalidated detection.

Changes:
- Created new module: pmoaudio-ext/src/sinks/flac_frame_utils.rs
  * parse_flac_block_size() - comprehensive frame header validation
  * find_complete_frames_boundary() - for regular FLAC streaming
  * find_complete_frames_with_samples() - for OGG-FLAC with granule tracking
  * Includes unit tests for validation

- Updated streaming_ogg_flac_sink.rs:
  * Removed duplicate functions
  * Now uses shared flac_frame_utils module

- Updated streaming_flac_sink.rs:
  * Removed old unvalidated find_complete_frames_boundary()
  * Now uses shared flac_frame_utils with improved validation
  * Benefits from same false positive prevention as OGG-FLAC

- Updated mod.rs to include flac_frame_utils module

Result: Both FLAC and OGG-FLAC streams now use the same comprehensive
frame header validation to prevent false positive sync code detection.
This commit is contained in:
Claude
2025-11-13 10:23:36 +00:00
parent ba3ef23e67
commit 46dee1de7b
4 changed files with 206 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

199
pmoaudio-ext/src/sinks/flac_frame_utils.rs Normal file
View File

@@ -0,0 +1,199 @@
//! Utilities for FLAC frame detection and validation
//!
//! This module provides functions to detect and validate FLAC frame boundaries
//! in a stream of bytes. It implements comprehensive FLAC frame header validation
//! to avoid false positives from random data that matches the sync pattern.
/// Validate and parse FLAC block size from frame header
///
/// Returns the number of samples in the frame if the header is valid, or None if:
/// - The header is truncated
/// - The sync code is incorrect
/// - Any reserved bits are set
/// - Sample rate, channel assignment, or bits per sample codes are invalid
///
/// This comprehensive validation is essential to avoid false positives, as the
/// FLAC sync pattern (0xFF 0xF8-0xFE) can appear randomly in compressed audio data.
pub(crate) fn parse_flac_block_size(data: &[u8], offset: usize) -> Option<u32> {
if offset + 4 > data.len() {
return None;
}
// FLAC frame header starts with sync code 0xFF 0xF8-0xFF
if data[offset] != 0xFF || data[offset + 1] < 0xF8 {
return None;
}
// Validate reserved bit (bit 1 of byte 1 must be 0)
if (data[offset + 1] & 0x02) != 0 {
return None; // Reserved bit set = not a valid frame header
}
let byte2 = data[offset + 2];
let byte3 = data[offset + 3];
// Byte 2 contains block size code in bits 4-7
let block_size_code = (byte2 >> 4) & 0x0F;
// Byte 2 bits 0-3 contain sample rate code
let sample_rate_code = byte2 & 0x0F;
// Validate sample rate code (0x0F is invalid)
if sample_rate_code == 0x0F {
return None; // Invalid sample rate = not a valid frame header
}
// Byte 3 bits 4-7 contain channel assignment
let channel_assignment = (byte3 >> 4) & 0x0F;
// Validate channel assignment (values 0x0B-0x0F are reserved/invalid)
if channel_assignment >= 0x0B {
return None; // Invalid channel assignment = not a valid frame header
}
// Byte 3 bits 1-3 contain bits per sample code
let bits_per_sample = (byte3 >> 1) & 0x07;
// Validate bits per sample (values 0x03 and 0x07 are reserved)
if bits_per_sample == 0x03 || bits_per_sample == 0x07 {
return None; // Invalid bits per sample = not a valid frame header
}
// Validate reserved bit in byte 3 (bit 0 must be 0)
if (byte3 & 0x01) != 0 {
return None; // Reserved bit set = not a valid frame header
}
// Decode block size according to FLAC spec
let block_size = match block_size_code {
0x00 => return None, // Reserved
0x01 => 192,
0x02..=0x05 => 576 * (1 << (block_size_code - 2)),
0x06 => return None, // Get 8-bit value from end of header (not fully validated here)
0x07 => return None, // Get 16-bit value from end of header (not fully validated here)
0x08..=0x0F => 256 * (1 << (block_size_code - 8)),
_ => return None,
};
Some(block_size)
}
/// Find the position where we should split the buffer to send complete FLAC frames
///
/// Returns the byte position just before the last FLAC frame starts.
///
/// FLAC frames start with a validated sync code (see `parse_flac_block_size`).
/// The sync code marks the START of a frame. To send complete frames, we find the
/// last sync code and send everything BEFORE it (which contains complete frames),
/// keeping the data from the last sync code onward for the next iteration.
///
/// We need at least 2 validated sync codes to identify one complete frame.
pub(crate) fn find_complete_frames_boundary(data: &[u8]) -> usize {
if data.len() < 4 {
return 0;
}
let mut sync_positions = Vec::new();
// Search for FLAC sync codes and validate frame headers
for i in 0..data.len() - 1 {
let byte1 = data[i];
let byte2 = data[i + 1];
// Check for potential sync code pattern
if byte1 == 0xFF && byte2 >= 0xF8 && byte2 <= 0xFE {
// Validate the complete frame header before accepting
if parse_flac_block_size(data, i).is_some() {
sync_positions.push(i);
}
}
}
// We need at least 2 sync codes to identify one complete frame
// The last sync code marks the start of a potentially incomplete frame
// Return the position of the last sync code - everything before it is complete
if sync_positions.len() >= 2 {
*sync_positions.last().unwrap()
} else {
0
}
}
/// Find complete FLAC frames and calculate total samples
///
/// Returns (byte_position, total_samples) where:
/// - byte_position: Position of the last frame boundary (or 0 if less than 2 frames)
/// - total_samples: Sum of samples in all complete frames (excluding the last incomplete one)
///
/// This is useful for OGG-FLAC streams that need to track granule position.
pub(crate) fn find_complete_frames_with_samples(data: &[u8]) -> (usize, u64) {
if data.len() < 4 {
return (0, 0);
}
let mut sync_positions = Vec::new();
let mut frame_samples = Vec::new();
// Search for FLAC sync codes and validate frame headers
for i in 0..data.len() - 1 {
let byte1 = data[i];
let byte2 = data[i + 1];
// Check for potential sync code pattern
if byte1 == 0xFF && byte2 >= 0xF8 && byte2 <= 0xFE {
// Validate the complete frame header before accepting
if let Some(samples) = parse_flac_block_size(data, i) {
sync_positions.push(i);
frame_samples.push(samples);
}
}
}
// We need at least 2 sync codes to identify one complete frame
if sync_positions.len() >= 2 {
let boundary = *sync_positions.last().unwrap();
// Sum samples for all complete frames (all except the last incomplete one)
let total_samples: u64 = frame_samples
.iter()
.take(sync_positions.len() - 1)
.map(|&s| s as u64)
.sum();
(boundary, total_samples)
} else {
(0, 0)
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_reject_false_positive_at_position_7() {
// Real-world example: first frame at 0, false positive at 7
let data = vec![
0xFF, 0xF8, 0xC9, 0xA8, // Valid frame header at position 0
0x00, 0x8D, 0x4C,
0xFF, 0xFE, 0x00, 0x00, // False positive at position 7 (0xFE has reserved bit set)
];
// Position 0 should be valid
assert!(parse_flac_block_size(&data, 0).is_some());
// Position 7 should be rejected (reserved bit validation)
assert!(parse_flac_block_size(&data, 7).is_none());
}
#[test]
fn test_boundary_detection_with_validation() {
// Create data with valid frame at 0 and false positive at 7
let mut data = vec![0xFF, 0xF8, 0xC9, 0xA8]; // Valid header
data.extend_from_slice(&[0u8; 2000]); // Frame data
data.extend_from_slice(&[0xFF, 0xF8, 0xC9, 0xA8]); // Second valid frame at ~2004
let boundary = find_complete_frames_boundary(&data);
// Should find 2 valid frames and return position of second one
assert!(boundary > 2000);
}
}

3
pmoaudio-ext/src/sinks/mod.rs
View File

@@ -10,6 +10,9 @@ mod flac_cache_sink;
#[cfg(feature = "cache-sink")]
pub use flac_cache_sink::{FlacCacheSink, FlacCacheSinkStats, TrackStats};
#[cfg(feature = "http-stream")]
mod flac_frame_utils;
#[cfg(feature = "http-stream")]
mod streaming_flac_sink;

43
pmoaudio-ext/src/sinks/streaming_flac_sink.rs
View File

@@ -62,6 +62,7 @@ use std::sync::Arc;
use std::task::{Context, Poll};
use std::time::Duration;
use super::flac_frame_utils;
use async_trait::async_trait;
use bytes::Bytes;
use pmoaudio::{
@@ -727,46 +728,6 @@ impl NodeLogic for StreamingFlacSinkLogic {
}
}
/// Find the position where we should split the buffer to send complete FLAC frames.
/// Returns the byte position just before the last FLAC frame starts.
///
/// FLAC frames start with a sync code: 14 bits set to 1, followed by a 0 bit.
/// This corresponds to byte patterns: 0xFF 0xF8 through 0xFF 0xFF.
///
/// The sync code marks the START of a frame. To send complete frames, we find the
/// last sync code and send everything BEFORE it (which contains complete frames),
/// keeping the data from the last sync code onward for the next iteration.
fn find_complete_frames_boundary(data: &[u8]) -> usize {
if data.len() < 4 {
return 0;
}
let mut sync_positions = Vec::new();
// Search for FLAC sync codes
// FLAC sync is 14 bits of 1: first byte is always 0xFF
// Second byte: 0xF8-0xFF (most common: 0xF8 for fixed blocksize, 0xF9 for variable)
// We search more conservatively for 0xF8-0xFE to avoid false positives
for i in 0..data.len() - 1 {
let byte1 = data[i];
let byte2 = data[i + 1];
// Check for FLAC sync pattern: 0xFF followed by 0xF8-0xFE
// We exclude 0xFF 0xFF as it's less common and more likely to be a false positive
if byte1 == 0xFF && byte2 >= 0xF8 && byte2 <= 0xFE {
sync_positions.push(i);
}
}
// We need at least 2 sync codes to identify one complete frame
// The last sync code marks the start of a potentially incomplete frame
// Return the position of the last sync code - everything before it is complete
if sync_positions.len() >= 2 {
*sync_positions.last().unwrap()
} else {
0
}
}
/// Broadcaster task: reads FLAC bytes from encoder and broadcasts to all clients.
/// Implements precise real-time pacing based on audio timestamps.
@@ -809,7 +770,7 @@ async fn broadcast_flac_stream(
// Find where to split: position of last sync code (start of last incomplete frame)
// Everything before this position contains only complete frames
let boundary = find_complete_frames_boundary(&accumulator);
let boundary = flac_frame_utils::find_complete_frames_boundary(&accumulator);
trace!(
"Buffer state: accumulator={} bytes, boundary={} bytes, will_send={}",

107
pmoaudio-ext/src/sinks/streaming_ogg_flac_sink.rs
View File

@@ -53,6 +53,7 @@ use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;
use std::task::{Context, Poll};
use super::flac_frame_utils;
use async_trait::async_trait;
use bytes::Bytes;
use pmoaudio::{
@@ -700,110 +701,6 @@ fn chunk_to_pcm_bytes(chunk: &AudioChunk, bits_per_sample: u8) -> Result<Vec<u8>
Ok(bytes)
}
/// Parse FLAC block size from frame header with validation
/// Returns number of samples in the frame, or None if parsing fails or header is invalid
fn parse_flac_block_size(data: &[u8], offset: usize) -> Option<u32> {
if offset + 4 > data.len() {
return None;
}
// FLAC frame header starts with sync code 0xFF 0xF8-0xFF
if data[offset] != 0xFF || data[offset + 1] < 0xF8 {
return None;
}
// Validate reserved bit (bit 1 of byte 1 must be 0)
if (data[offset + 1] & 0x02) != 0 {
return None; // Reserved bit set = not a valid frame header
}
let byte2 = data[offset + 2];
let byte3 = data[offset + 3];
// Byte 2 contains block size code in bits 4-7
let block_size_code = (byte2 >> 4) & 0x0F;
// Byte 2 bits 0-3 contain sample rate code
let sample_rate_code = byte2 & 0x0F;
// Validate sample rate code (0x0F is invalid)
if sample_rate_code == 0x0F {
return None; // Invalid sample rate = not a valid frame header
}
// Byte 3 bits 1-3 contain channel assignment
let channel_assignment = (byte3 >> 4) & 0x0F;
// Validate channel assignment (values 0x0B-0x0F are reserved/invalid)
if channel_assignment >= 0x0B {
return None; // Invalid channel assignment = not a valid frame header
}
// Byte 3 bits 1-3 contain bits per sample code
let bits_per_sample = (byte3 >> 1) & 0x07;
// Validate bits per sample (values 0x03 and 0x07 are reserved)
if bits_per_sample == 0x03 || bits_per_sample == 0x07 {
return None; // Invalid bits per sample = not a valid frame header
}
// Validate reserved bit in byte 3 (bit 0 must be 0)
if (byte3 & 0x01) != 0 {
return None; // Reserved bit set = not a valid frame header
}
// Decode block size according to FLAC spec
let block_size = match block_size_code {
0x00 => return None, // Reserved
0x01 => 192,
0x02..=0x05 => 576 * (1 << (block_size_code - 2)),
0x06 => return None, // Get 8-bit value from end of header (not fully validated)
0x07 => return None, // Get 16-bit value from end of header (not fully validated)
0x08..=0x0F => 256 * (1 << (block_size_code - 8)),
_ => return None,
};
Some(block_size)
}
/// Find complete FLAC frames and calculate total samples
/// Returns (byte_position, total_samples) or (0, 0) if no complete frames
fn find_complete_frames_with_samples(data: &[u8]) -> (usize, u64) {
if data.len() < 4 {
return (0, 0);
}
let mut sync_positions = Vec::new();
let mut frame_samples = Vec::new();
// Search for FLAC sync codes and validate frame headers
for i in 0..data.len() - 1 {
let byte1 = data[i];
let byte2 = data[i + 1];
// Check for potential sync code pattern
if byte1 == 0xFF && byte2 >= 0xF8 && byte2 <= 0xFE {
// Validate the complete frame header before accepting
if let Some(samples) = parse_flac_block_size(data, i) {
sync_positions.push(i);
frame_samples.push(samples);
}
}
}
// We need at least 2 sync codes to identify one complete frame
if sync_positions.len() >= 2 {
let boundary = *sync_positions.last().unwrap();
// Sum samples for all complete frames (all except the last incomplete one)
let total_samples: u64 = frame_samples.iter().take(sync_positions.len() - 1)
.map(|&s| s as u64)
.sum();
(boundary, total_samples)
} else {
(0, 0)
}
}
/// OGG wrapper + broadcaster task: reads FLAC bytes from encoder, wraps in OGG pages, and broadcasts.
/// Implements precise real-time pacing based on audio timestamps.
/// Ensures FLAC frames are only sent at frame boundaries to prevent sync errors in strict decoders like FFPlay.
@@ -891,7 +788,7 @@ async fn broadcast_ogg_flac_stream(
// Find where to split: position of last sync code (start of last incomplete frame)
// Also calculate total samples for granule position
let (boundary, samples_in_frames) = find_complete_frames_with_samples(&flac_accumulator);
let (boundary, samples_in_frames) = flac_frame_utils::find_complete_frames_with_samples(&flac_accumulator);
// Only broadcast if we have at least one complete frame (4KB minimum for efficiency)
// OGG pages can be larger than pure FLAC broadcasts since they include page overhead