Use config struct to pass hls params#177
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the HLS (HTTP Live Streaming) module to use a centralized HlsConfig struct instead of passing individual parameters throughout the codebase. The change improves maintainability and reduces parameter coupling.
- Replaces individual HLS parameters (duration, need_record, path, aof_ratio, live_ts_count) with a single
HlsConfigstruct - Centralizes configuration logic by creating a new
configlibrary - Updates all HLS-related constructors and function signatures to accept the config struct
Reviewed Changes
Copilot reviewed 14 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| protocol/hls/src/flv2hls.rs | Updates constructor to accept HlsConfig and extracts individual parameters from config |
| protocol/hls/src/m3u8.rs | Refactors constructor to use HlsConfig for path, need_record, and live_ts_count |
| protocol/hls/src/remuxer.rs | Simplifies HlsRemuxer constructor to accept single config parameter |
| protocol/hls/src/flv_data_receiver.rs | Updates FlvDataReceiver to pass HlsConfig to underlying components |
| protocol/hls/src/test_flv2hls.rs | Updates test to use new constructor signature |
| library/config/src/lib.rs | Adds live_ts_count field to HlsConfig struct |
| application/xiu/src/service.rs | Updates service to pass entire HLS config instead of individual fields |
| application/xiu/src/main.rs | Updates imports to use new config library |
| application/xiu/src/lib.rs | Removes old config module and adds external config crate |
| application/xiu/Cargo.toml | Adds dependency on new config library |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| let need_record= hls_config | ||
| .as_ref() | ||
| .and_then(|config| Some(config.need_record)) |
There was a problem hiding this comment.
The .and_then(|config| Some(config.need_record)) pattern is unnecessarily complex. Since need_record is a boolean field, you can simplify this to .map(|config| config.need_record) or access it directly if the field exists.
| .and_then(|config| Some(config.need_record)) | |
| .map(|config| config.need_record) |
| super::{define::FlvDemuxerData, errors::MediaError, m3u8::M3u8}, bytes::BytesMut, config::HlsConfig, xflv::{ | ||
| define::{frame_type, FlvData}, | ||
| demuxer::{FlvAudioTagDemuxer, FlvVideoTagDemuxer}, | ||
| }, | ||
| streamhub::{define::{StreamHubEventSender, StreamHubEvent}, stream::StreamIdentifier}, |
There was a problem hiding this comment.
[nitpick] The imports are formatted on a single line making them hard to read. Consider breaking this into multiple lines for better readability.
| super::{define::FlvDemuxerData, errors::MediaError, m3u8::M3u8}, bytes::BytesMut, config::HlsConfig, xflv::{ | |
| define::{frame_type, FlvData}, | |
| demuxer::{FlvAudioTagDemuxer, FlvVideoTagDemuxer}, | |
| }, | |
| streamhub::{define::{StreamHubEventSender, StreamHubEvent}, stream::StreamIdentifier}, | |
| super::{define::FlvDemuxerData, errors::MediaError, m3u8::M3u8}, | |
| bytes::BytesMut, | |
| config::HlsConfig, | |
| xflv::{ | |
| define::{frame_type, FlvData}, | |
| demuxer::{FlvAudioTagDemuxer, FlvVideoTagDemuxer}, | |
| }, | |
| streamhub::{ | |
| define::{StreamHubEventSender, StreamHubEvent}, | |
| stream::StreamIdentifier, | |
| }, |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
thanks for your contributions!! |
No description provided.