Skip to content

Commit c80d20f

Browse files
committed
fix: preserve first_sample_flags on trun encode
The encoder hardcoded first_sample_flags to false and only wrote per-sample flags when ALL entries had flags set. When decoding a trun with first_sample_flags, entry[0] gets Some(flags) but entries[1..N] get None (they inherit default_sample_flags from tfhd). On re-encode, the all-or-nothing check failed and no flags were written at all, losing the keyframe indication on the first sample. Fix: detect when only the first entry has flags and emit first_sample_flags instead of dropping them. Adds a roundtrip test.
1 parent b81f0c4 commit c80d20f

1 file changed

Lines changed: 131 additions & 3 deletions

File tree

src/moof/traf/trun.rs

Lines changed: 131 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,23 +96,28 @@ impl AtomExt for Trun {
9696
}
9797

9898
fn encode_body_ext<B: BufMut>(&self, buf: &mut B) -> Result<TrunExt> {
99+
let all_flags = self.entries.iter().all(|s| s.flags.is_some());
100+
let first_only_flags = !all_flags
101+
&& self.entries.first().is_some_and(|s| s.flags.is_some())
102+
&& self.entries.iter().skip(1).all(|s| s.flags.is_none());
103+
99104
let ext = TrunExt {
100105
version: TrunVersion::V1,
101106
data_offset: self.data_offset.is_some(),
102-
first_sample_flags: false,
107+
first_sample_flags: first_only_flags,
103108

104109
// TODO error if these are not all the same
105110
sample_duration: self.entries.iter().all(|s| s.duration.is_some()),
106111
sample_size: self.entries.iter().all(|s| s.size.is_some()),
107-
sample_flags: self.entries.iter().all(|s| s.flags.is_some()),
112+
sample_flags: all_flags,
108113
sample_cts: self.entries.iter().all(|s| s.cts.is_some()),
109114
};
110115

111116
(self.entries.len() as u32).encode(buf)?;
112117

113118
self.data_offset.encode(buf)?;
114119
if ext.first_sample_flags {
115-
0u32.encode(buf)?; // TODO first sample flags
120+
self.entries[0].flags.unwrap().encode(buf)?;
116121
}
117122

118123
for entry in &self.entries {
@@ -125,3 +130,126 @@ impl AtomExt for Trun {
125130
Ok(ext)
126131
}
127132
}
133+
134+
#[cfg(test)]
135+
mod test {
136+
use super::*;
137+
138+
/// Verify that first_sample_flags survives encode→decode roundtrip.
139+
///
140+
/// ffmpeg commonly writes trun boxes where only the first entry has flags
141+
/// (via first_sample_flags) and the rest inherit default_sample_flags from
142+
/// tfhd. After decode, entry[0].flags = Some(keyframe), entries[1..N].flags = None.
143+
/// The encoder must preserve this by emitting first_sample_flags.
144+
#[test]
145+
fn first_sample_flags_roundtrip() {
146+
let trun = Trun {
147+
data_offset: Some(100),
148+
entries: vec![
149+
TrunEntry {
150+
duration: Some(512),
151+
size: Some(1000),
152+
flags: Some(0x02000000), // keyframe (sample_depends_on=2)
153+
cts: None,
154+
},
155+
TrunEntry {
156+
duration: Some(512),
157+
size: Some(200),
158+
flags: None, // inherits default_sample_flags from tfhd
159+
cts: None,
160+
},
161+
TrunEntry {
162+
duration: Some(512),
163+
size: Some(200),
164+
flags: None,
165+
cts: None,
166+
},
167+
],
168+
};
169+
170+
let mut buf = Vec::new();
171+
trun.encode(&mut buf).expect("encode");
172+
173+
let decoded = Trun::decode(&mut &buf[..]).expect("decode");
174+
175+
// entry[0] must have the keyframe flags from first_sample_flags
176+
assert_eq!(decoded.entries[0].flags, Some(0x02000000));
177+
// entries[1..N] must have None (they use default_sample_flags from tfhd)
178+
assert_eq!(decoded.entries[1].flags, None);
179+
assert_eq!(decoded.entries[2].flags, None);
180+
assert_eq!(decoded.data_offset, Some(100));
181+
assert_eq!(decoded.entries.len(), 3);
182+
}
183+
184+
/// When multiple entries have explicit flags (not just the first),
185+
/// the encoder must use per-sample flags, not first_sample_flags.
186+
#[test]
187+
fn mixed_flags_uses_per_sample() {
188+
let trun = Trun {
189+
data_offset: Some(100),
190+
entries: vec![
191+
TrunEntry {
192+
duration: Some(512),
193+
size: Some(1000),
194+
flags: Some(0x02000000), // keyframe
195+
cts: None,
196+
},
197+
TrunEntry {
198+
duration: Some(512),
199+
size: Some(200),
200+
flags: Some(0x01010000), // non-keyframe (explicit)
201+
cts: None,
202+
},
203+
TrunEntry {
204+
duration: Some(512),
205+
size: Some(200),
206+
flags: None, // no flags
207+
cts: None,
208+
},
209+
],
210+
};
211+
212+
let mut buf = Vec::new();
213+
trun.encode(&mut buf).expect("encode");
214+
215+
let decoded = Trun::decode(&mut &buf[..]).expect("decode");
216+
217+
// Mixed Some/None that isn't first-only: neither first_sample_flags
218+
// nor sample_flags can represent this, so flags are dropped.
219+
// This is a known limitation — callers should fill in defaults
220+
// before encoding if they need all flags preserved.
221+
assert_eq!(decoded.entries[0].flags, None);
222+
assert_eq!(decoded.entries[1].flags, None);
223+
assert_eq!(decoded.entries[2].flags, None);
224+
}
225+
226+
/// When all entries have explicit flags, per-sample flags are used.
227+
#[test]
228+
fn all_flags_roundtrip() {
229+
let trun = Trun {
230+
data_offset: Some(100),
231+
entries: vec![
232+
TrunEntry {
233+
duration: Some(512),
234+
size: Some(1000),
235+
flags: Some(0x02000000),
236+
cts: None,
237+
},
238+
TrunEntry {
239+
duration: Some(512),
240+
size: Some(200),
241+
flags: Some(0x01010000),
242+
cts: None,
243+
},
244+
],
245+
};
246+
247+
let mut buf = Vec::new();
248+
trun.encode(&mut buf).expect("encode");
249+
250+
let decoded = Trun::decode(&mut &buf[..]).expect("decode");
251+
252+
assert_eq!(decoded.entries[0].flags, Some(0x02000000));
253+
assert_eq!(decoded.entries[1].flags, Some(0x01010000));
254+
}
255+
}

0 commit comments

Comments
 (0)