-
Notifications
You must be signed in to change notification settings - Fork 22
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
Video MP4 output #646
Video MP4 output #646
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. I'll refactor types once I know what is needed for AAC decoding. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,13 +20,21 @@ pub struct RtpOutputStream { | |
pub ip: Option<Arc<str>>, | ||
/// (**default=`"udp"`**) Transport layer protocol that will be used to send RTP packets. | ||
pub transport_protocol: Option<TransportProtocol>, | ||
pub video: Option<OutputRtpVideoOptions>, | ||
pub audio: Option<OutputRtpAudioOptions>, | ||
pub video: Option<OutputVideoOptions>, | ||
pub audio: Option<OutputAudioOptions>, | ||
} | ||
|
||
#[derive(Debug, Serialize, Deserialize, Clone, JsonSchema)] | ||
#[serde(deny_unknown_fields)] | ||
pub struct OutputRtpVideoOptions { | ||
pub struct Mp4Output { | ||
pub path: String, | ||
pub video: Option<OutputVideoOptions>, | ||
pub audio: Option<OutputAudioOptions>, | ||
} | ||
|
||
#[derive(Debug, Serialize, Deserialize, Clone, JsonSchema)] | ||
#[serde(deny_unknown_fields)] | ||
pub struct OutputVideoOptions { | ||
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. You can't unify those values, after you add AAC here it will be incorrect for RTP. Start by duplicating and unifying when you know you can support unified interface everywhere. 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.
|
||
/// Output resolution in pixels. | ||
pub resolution: Resolution, | ||
/// Defines when output stream should end if some of the input streams are finished. If output includes both audio and video streams, then EOS needs to be sent on both. | ||
|
@@ -39,7 +47,7 @@ pub struct OutputRtpVideoOptions { | |
|
||
#[derive(Debug, Serialize, Deserialize, Clone, JsonSchema)] | ||
#[serde(deny_unknown_fields)] | ||
pub struct OutputRtpAudioOptions { | ||
pub struct OutputAudioOptions { | ||
/// (**default="sum_clip"**) Specifies how audio should be mixed. | ||
pub mixing_strategy: Option<MixingStrategy>, | ||
/// Condition for termination of output stream based on the input streams states. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,7 +93,7 @@ fn run_encoder_thread( | |
let chunk = EncodedChunk { | ||
data, | ||
pts: batch.start_pts, | ||
dts: None, | ||
dts: Some(batch.start_pts), | ||
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. why? |
||
kind: EncodedChunkKind::Audio(AudioCodec::Opus), | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ use compositor_render::{ | |
error::RequestKeyframeError, Frame, OutputFrameFormat, OutputId, Resolution, | ||
}; | ||
use crossbeam_channel::{bounded, Receiver, Sender}; | ||
use mp4::{Mp4FileWriter, Mp4OutputOptions}; | ||
|
||
use crate::{audio_mixer::OutputSamples, error::RegisterOutputError, queue::PipelineEvent}; | ||
|
||
|
@@ -13,6 +14,7 @@ use super::{ | |
PipelineCtx, Port, RawDataReceiver, | ||
}; | ||
|
||
pub mod mp4; | ||
pub mod rtp; | ||
|
||
/// Options to configure public outputs that can be constructed via REST API | ||
|
@@ -26,6 +28,7 @@ pub struct OutputOptions { | |
#[derive(Debug, Clone)] | ||
pub enum OutputProtocolOptions { | ||
Rtp(RtpSenderOptions), | ||
Mp4(Mp4OutputOptions), | ||
} | ||
|
||
/// Options to configure output that sends h264 and opus audio via channel | ||
|
@@ -61,6 +64,10 @@ pub enum Output { | |
sender: RtpSender, | ||
encoder: Encoder, | ||
}, | ||
Mp4 { | ||
writer: Mp4FileWriter, | ||
encoder: Encoder, | ||
}, | ||
EncodedData { | ||
encoder: Encoder, | ||
}, | ||
|
@@ -99,7 +106,13 @@ impl OutputOptionsExt<Option<Port>> for OutputOptions { | |
rtp::RtpSender::new(output_id, rtp_options.clone(), packets) | ||
.map_err(|e| RegisterOutputError::OutputError(output_id.clone(), e))?; | ||
|
||
Ok((Output::Rtp { sender, encoder }, port)) | ||
Ok((Output::Rtp { sender, encoder }, Some(port))) | ||
} | ||
OutputProtocolOptions::Mp4(mp4_opt) => { | ||
let writer = Mp4FileWriter::new(output_id, mp4_opt.clone(), packets) | ||
.map_err(|e| RegisterOutputError::OutputError(output_id.clone(), e))?; | ||
|
||
Ok((Output::Mp4 { writer, encoder }, None)) | ||
} | ||
} | ||
} | ||
|
@@ -160,32 +173,36 @@ impl OutputOptionsExt<RawDataReceiver> for RawDataOutputOptions { | |
impl Output { | ||
pub fn frame_sender(&self) -> Option<&Sender<PipelineEvent<Frame>>> { | ||
match &self { | ||
Output::Rtp { encoder, .. } => encoder.frame_sender(), | ||
Output::EncodedData { encoder } => encoder.frame_sender(), | ||
Output::Rtp { encoder, .. } | ||
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. personal opinion(here and below), but I think is less readable than the previous version and very annoying when you need to change anything |
||
| Output::Mp4 { encoder, .. } | ||
| Output::EncodedData { encoder } => encoder.frame_sender(), | ||
Output::RawData { video, .. } => video.as_ref(), | ||
} | ||
} | ||
|
||
pub fn samples_batch_sender(&self) -> Option<&Sender<PipelineEvent<OutputSamples>>> { | ||
match &self { | ||
Output::Rtp { encoder, .. } => encoder.samples_batch_sender(), | ||
Output::EncodedData { encoder } => encoder.samples_batch_sender(), | ||
Output::Rtp { encoder, .. } | ||
| Output::Mp4 { encoder, .. } | ||
| Output::EncodedData { encoder } => encoder.samples_batch_sender(), | ||
Output::RawData { audio, .. } => audio.as_ref(), | ||
} | ||
} | ||
|
||
pub fn resolution(&self) -> Option<Resolution> { | ||
match &self { | ||
Output::Rtp { encoder, .. } => encoder.video.as_ref().map(|v| v.resolution()), | ||
Output::EncodedData { encoder } => encoder.video.as_ref().map(|v| v.resolution()), | ||
Output::Rtp { encoder, .. } | ||
| Output::Mp4 { encoder, .. } | ||
| Output::EncodedData { encoder } => encoder.video.as_ref().map(|v| v.resolution()), | ||
Output::RawData { resolution, .. } => *resolution, | ||
} | ||
} | ||
|
||
pub fn request_keyframe(&self, output_id: OutputId) -> Result<(), RequestKeyframeError> { | ||
let encoder = match &self { | ||
Output::Rtp { encoder, .. } => encoder, | ||
Output::EncodedData { encoder } => encoder, | ||
Output::Rtp { encoder, .. } | ||
| Output::Mp4 { encoder, .. } | ||
| Output::EncodedData { encoder } => encoder, | ||
Output::RawData { .. } => return Err(RequestKeyframeError::RawOutput(output_id)), | ||
}; | ||
|
||
|
@@ -211,6 +228,10 @@ impl Output { | |
Output::RawData { video, .. } => { | ||
video.as_ref().map(|_| OutputFrameFormat::RgbaWgpuTexture) | ||
} | ||
Output::Mp4 { encoder, .. } => encoder | ||
.video | ||
.as_ref() | ||
.map(|_| OutputFrameFormat::PlanarYuv420Bytes), | ||
} | ||
} | ||
} |
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.
If this is a user-facing value, use Mp4 too. If it not user-facing, I would rename input mp4 for consistency