From 3c8368f9a37e4d60ed4079e5fb8da58ea491f493 Mon Sep 17 00:00:00 2001 From: Vadim Getmanshchuk Date: Thu, 7 Apr 2022 13:35:02 -0700 Subject: [PATCH 1/4] A fix for `CLOSED-CAPTIONS=NONE` case and a few minor fixes The PR includes * a hotfix for issue #44. I'm not entirely sure and it's possible to * `#EXT-X-ENDLIST` is moved to be the last part of the media manifest. Theoretically it can appear anywhere in manifest, so the current placement is not breaking the standard, but not usually is what found in the wild and I believe will help with readability. As any placement is possible, the placement at the end is completely legal. * minor `cargo clippy` suggestions --- src/playlist.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/playlist.rs b/src/playlist.rs index 14f0999..2d72fcd 100644 --- a/src/playlist.rs +++ b/src/playlist.rs @@ -176,7 +176,12 @@ impl VariantStream { self.write_stream_inf_common_attributes(w)?; write_some_attribute_quoted!(w, ",AUDIO", &self.audio)?; write_some_attribute_quoted!(w, ",SUBTITLES", &self.subtitles)?; - write_some_attribute_quoted!(w, ",CLOSED-CAPTIONS", &self.closed_captions)?; + // handle `CLOSED-CAPTIONS=NONE` case + if self.closed_captions.as_deref().eq(&Some("NONE")) { + write_some_attribute!(w, ",CLOSED-CAPTIONS", &self.closed_captions)?; + } else { + write_some_attribute_quoted!(w, ",CLOSED-CAPTIONS", &self.closed_captions)?; + } writeln!(w)?; writeln!(w, "{}", self.uri) } From 2f92e3ae8c06a4ff346a6567bd275aa90c9541d3 Mon Sep 17 00:00:00 2001 From: Vadim Getmanshchuk Date: Fri, 8 Apr 2022 16:27:04 -0700 Subject: [PATCH 2/4] `#EXT-X-ENDLIST` is moved to be the last part of the media manifest. Theoretically it can appear anywhere in manifest, so the current placement is not breaking the standard, but not usually is what found in the wild and I believe will help with readability. As any placement is possible, the placement at the end is completely legal. --- src/playlist.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/playlist.rs b/src/playlist.rs index 2d72fcd..6b25548 100644 --- a/src/playlist.rs +++ b/src/playlist.rs @@ -437,9 +437,6 @@ impl MediaPlaylist { self.discontinuity_sequence )?; } - if self.end_list { - writeln!(w, "#EXT-X-ENDLIST")?; - } if let Some(ref v) = self.playlist_type { writeln!(w, "#EXT-X-PLAYLIST-TYPE:{}", v)?; } @@ -455,6 +452,9 @@ impl MediaPlaylist { for segment in &self.segments { segment.write_to(w)?; } + if self.end_list { + writeln!(w, "#EXT-X-ENDLIST")?; + } Ok(()) } From 5c842fd9f62a10d54c34f002378e7bbf4967d76a Mon Sep 17 00:00:00 2001 From: Vadim Getmanshchuk Date: Fri, 8 Apr 2022 16:31:37 -0700 Subject: [PATCH 3/4] minor cargo clippy suggestions --- src/playlist.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/playlist.rs b/src/playlist.rs index 6b25548..f3a8e02 100644 --- a/src/playlist.rs +++ b/src/playlist.rs @@ -152,8 +152,8 @@ impl VariantStream { pub fn from_hashmap(mut attrs: HashMap, is_i_frame: bool) -> VariantStream { VariantStream { is_i_frame, - uri: attrs.remove("URI").unwrap_or_else(String::new), - bandwidth: attrs.remove("BANDWIDTH").unwrap_or_else(String::new), + uri: attrs.remove("URI").unwrap_or_default(), + bandwidth: attrs.remove("BANDWIDTH").unwrap_or_default(), average_bandwidth: attrs.remove("AVERAGE-BANDWIDTH"), codecs: attrs.remove("CODECS"), resolution: attrs.remove("RESOLUTION"), @@ -231,7 +231,7 @@ impl AlternativeMedia { .and_then(|s| AlternativeMediaType::from_str(s).ok()) .unwrap_or_default(), uri: attrs.remove("URI"), - group_id: attrs.remove("GROUP-ID").unwrap_or_else(String::new), + group_id: attrs.remove("GROUP-ID").unwrap_or_default(), language: attrs.remove("LANGUAGE"), assoc_language: attrs.remove("ASSOC-LANGUAGE"), name: attrs.remove("NAME").unwrap_or_default(), @@ -593,7 +593,7 @@ pub struct Key { impl Key { pub fn from_hashmap(mut attrs: HashMap) -> Key { Key { - method: attrs.remove("METHOD").unwrap_or_else(String::new), + method: attrs.remove("METHOD").unwrap_or_default(), uri: attrs.remove("URI"), iv: attrs.remove("IV"), keyformat: attrs.remove("KEYFORMAT"), @@ -691,7 +691,7 @@ pub struct Start { impl Start { pub fn from_hashmap(mut attrs: HashMap) -> Start { Start { - time_offset: attrs.remove("TIME-OFFSET").unwrap_or_else(String::new), + time_offset: attrs.remove("TIME-OFFSET").unwrap_or_default(), precise: attrs.remove("PRECISE").or_else(|| Some("NO".to_string())), } } From ac0f881eefdfb74257e090b48d852528043bb701 Mon Sep 17 00:00:00 2001 From: Vadim Getmanshchuk Date: Thu, 14 Apr 2022 02:07:05 -0700 Subject: [PATCH 4/4] * added `QuotedOrUnquoted` enum * implemented `Default`, `From`, `Display` traits * updated `VariantStream`, `AlternativeMedia`, `SessionData`, `Key`, `Start` emums * updated `from_hashmap` methods for each enum * fixed tests --- src/parser.rs | 109 ++++++++++++++++++++++++++++++++++++------------ src/playlist.rs | 94 +++++++++++++++++++++-------------------- 2 files changed, 131 insertions(+), 72 deletions(-) diff --git a/src/parser.rs b/src/parser.rs index f83c15b..55d7303 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -12,6 +12,7 @@ use crate::playlist::*; use nom::IResult; use std::collections::HashMap; use std::f32; +use std::fmt; use std::result::Result; use std::str; use std::str::FromStr; @@ -542,13 +543,16 @@ fn extmap(i: &[u8]) -> IResult<&[u8], Map> { let uri = attrs.get("URI").cloned().unwrap_or_default(); let byte_range = attrs .get("BYTERANGE") - .map(|range| match byte_range_val(range.as_bytes()) { + .map(|range| match byte_range_val(range.to_string().as_bytes()) { IResult::Ok((_, range)) => Ok(range), IResult::Err(_) => Err("invalid byte range"), }) .transpose()?; - Ok(Map { uri, byte_range }) + Ok(Map { + uri: uri.to_string(), + byte_range, + }) })(i) } @@ -601,7 +605,7 @@ fn comment_tag(i: &[u8]) -> IResult<&[u8], String> { // Util // ----------------------------------------------------------------------------------------------- -fn key_value_pairs(i: &[u8]) -> IResult<&[u8], HashMap> { +fn key_value_pairs(i: &[u8]) -> IResult<&[u8], HashMap> { fold_many0( preceded(space0, key_value_pair), HashMap::new, @@ -612,7 +616,41 @@ fn key_value_pairs(i: &[u8]) -> IResult<&[u8], HashMap> { )(i) } -fn key_value_pair(i: &[u8]) -> IResult<&[u8], (String, String)> { +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum QuotedOrUnquoted { + Unquoted(String), + Quoted(String), +} + +impl Default for QuotedOrUnquoted { + fn default() -> Self { + QuotedOrUnquoted::Quoted(String::new()) + } +} + +impl From<&str> for QuotedOrUnquoted { + fn from(s: &str) -> Self { + if s.starts_with('"') && s.ends_with('"') { + return QuotedOrUnquoted::Quoted(s.trim_matches('"').to_string()); + } + QuotedOrUnquoted::Unquoted(s.to_string()) + } +} + +impl fmt::Display for QuotedOrUnquoted { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "{}", + match self { + QuotedOrUnquoted::Unquoted(s) => s, + QuotedOrUnquoted::Quoted(u) => u, + } + ) + } +} + +fn key_value_pair(i: &[u8]) -> IResult<&[u8], (String, QuotedOrUnquoted)> { map( tuple(( peek(none_of("\r\n")), @@ -625,16 +663,16 @@ fn key_value_pair(i: &[u8]) -> IResult<&[u8], (String, String)> { )(i) } -fn quoted(i: &[u8]) -> IResult<&[u8], String> { +fn quoted(i: &[u8]) -> IResult<&[u8], QuotedOrUnquoted> { delimited( char('\"'), - map_res(is_not("\""), from_utf8_slice), + map_res(is_not("\""), quoted_from_utf8_slice), char('\"'), )(i) } -fn unquoted(i: &[u8]) -> IResult<&[u8], String> { - map_res(is_not(",\r\n"), from_utf8_slice)(i) +fn unquoted(i: &[u8]) -> IResult<&[u8], QuotedOrUnquoted> { + map_res(is_not(",\r\n"), unquoted_from_utf8_slice)(i) } fn consume_line(i: &[u8]) -> IResult<&[u8], String> { @@ -687,6 +725,20 @@ fn from_utf8_slice(s: &[u8]) -> Result { String::from_utf8(s.to_vec()) } +fn quoted_from_utf8_slice(s: &[u8]) -> Result { + match String::from_utf8(s.to_vec()) { + Ok(q) => Ok(QuotedOrUnquoted::Quoted(q)), + Err(e) => Err(e), + } +} + +fn unquoted_from_utf8_slice(s: &[u8]) -> Result { + match String::from_utf8(s.to_vec()) { + Ok(q) => Ok(QuotedOrUnquoted::Unquoted(q)), + Err(e) => Err(e), + } +} + #[cfg(test)] mod tests { use super::*; @@ -729,10 +781,13 @@ mod tests { key_value_pairs(b"BANDWIDTH=395000,CODECS=\"avc1.4d001f,mp4a.40.2\"\r\nrest="), Result::Ok(( "\r\nrest=".as_bytes(), - vec![("BANDWIDTH", "395000"), ("CODECS", "avc1.4d001f,mp4a.40.2")] - .into_iter() - .map(|(k, v)| (String::from(k), String::from(v))) - .collect::>(), + vec![ + ("BANDWIDTH", "395000"), + ("CODECS", "\"avc1.4d001f,mp4a.40.2\"") + ] + .into_iter() + .map(|(k, v)| (String::from(k), v.into())) + .collect::>(), )), ); } @@ -745,13 +800,13 @@ mod tests { "\nrest".as_bytes(), vec![ ("BANDWIDTH", "86000"), - ("URI", "low/iframe.m3u8"), + ("URI", "\"low/iframe.m3u8\""), ("PROGRAM-ID", "1"), - ("RESOLUTION", "1x1"), + ("RESOLUTION", "\"1x1\""), ("VIDEO", "1") ].into_iter() - .map(|(k, v)| (String::from(k), String::from(v))) - .collect::>() + .map(|(k, v)| (String::from(k), v.into())) + .collect::>() )) ); } @@ -762,10 +817,13 @@ mod tests { key_value_pairs(b"BANDWIDTH=300000,CODECS=\"avc1.42c015,mp4a.40.2\"\r\nrest"), Result::Ok(( "\r\nrest".as_bytes(), - vec![("BANDWIDTH", "300000"), ("CODECS", "avc1.42c015,mp4a.40.2")] - .into_iter() - .map(|(k, v)| (String::from(k), String::from(v))) - .collect::>() + vec![ + ("BANDWIDTH", "300000"), + ("CODECS", "\"avc1.42c015,mp4a.40.2\"") + ] + .into_iter() + .map(|(k, v)| (String::from(k), v.into())) + .collect::>() )) ); } @@ -782,8 +840,8 @@ mod tests { ("VIDEO", "1") ] .into_iter() - .map(|(k, v)| (String::from(k), String::from(v))) - .collect::>() + .map(|(k, v)| (String::from(k), v.into())) + .collect::>() )) ); } @@ -792,10 +850,7 @@ mod tests { fn test_key_value_pair() { assert_eq!( key_value_pair(b"PROGRAM-ID=1,rest"), - Result::Ok(( - "rest".as_bytes(), - ("PROGRAM-ID".to_string(), "1".to_string()) - )) + Result::Ok(("rest".as_bytes(), ("PROGRAM-ID".to_string(), "1".into()))) ); } @@ -839,7 +894,7 @@ mod tests { fn quotes() { assert_eq!( quoted(b"\"value\"rest"), - Result::Ok(("rest".as_bytes(), "value".to_string())) + Result::Ok(("rest".as_bytes(), "\"value\"".into())) ); } diff --git a/src/playlist.rs b/src/playlist.rs index f3a8e02..e0c7013 100644 --- a/src/playlist.rs +++ b/src/playlist.rs @@ -3,6 +3,7 @@ //! The main type here is the `Playlist` enum. //! Which is either a `MasterPlaylist` or a `MediaPlaylist`. +use crate::QuotedOrUnquoted; use std::collections::HashMap; use std::f32; use std::fmt; @@ -140,28 +141,31 @@ pub struct VariantStream { pub codecs: Option, pub resolution: Option, pub frame_rate: Option, - pub hdcp_level: Option, + pub hdcp_level: Option, pub audio: Option, pub video: Option, pub subtitles: Option, - pub closed_captions: Option, + pub closed_captions: Option, // PROGRAM-ID tag was removed in protocol version 6 } impl VariantStream { - pub fn from_hashmap(mut attrs: HashMap, is_i_frame: bool) -> VariantStream { + pub fn from_hashmap( + mut attrs: HashMap, + is_i_frame: bool, + ) -> VariantStream { VariantStream { is_i_frame, - uri: attrs.remove("URI").unwrap_or_default(), - bandwidth: attrs.remove("BANDWIDTH").unwrap_or_default(), - average_bandwidth: attrs.remove("AVERAGE-BANDWIDTH"), - codecs: attrs.remove("CODECS"), - resolution: attrs.remove("RESOLUTION"), - frame_rate: attrs.remove("FRAME-RATE"), + uri: attrs.remove("URI").unwrap_or_default().to_string(), + bandwidth: attrs.remove("BANDWIDTH").unwrap_or_default().to_string(), + average_bandwidth: attrs.remove("AVERAGE-BANDWIDTH").map(|a| a.to_string()), + codecs: attrs.remove("CODECS").map(|c| c.to_string()), + resolution: attrs.remove("RESOLUTION").map(|r| r.to_string()), + frame_rate: attrs.remove("FRAME-RATE").map(|f| f.to_string()), hdcp_level: attrs.remove("HDCP-LEVEL"), - audio: attrs.remove("AUDIO"), - video: attrs.remove("VIDEO"), - subtitles: attrs.remove("SUBTITLES"), + audio: attrs.remove("AUDIO").map(|a| a.to_string()), + video: attrs.remove("VIDEO").map(|v| v.to_string()), + subtitles: attrs.remove("SUBTITLES").map(|s| s.to_string()), closed_captions: attrs.remove("CLOSED-CAPTIONS"), } } @@ -176,12 +180,7 @@ impl VariantStream { self.write_stream_inf_common_attributes(w)?; write_some_attribute_quoted!(w, ",AUDIO", &self.audio)?; write_some_attribute_quoted!(w, ",SUBTITLES", &self.subtitles)?; - // handle `CLOSED-CAPTIONS=NONE` case - if self.closed_captions.as_deref().eq(&Some("NONE")) { - write_some_attribute!(w, ",CLOSED-CAPTIONS", &self.closed_captions)?; - } else { - write_some_attribute_quoted!(w, ",CLOSED-CAPTIONS", &self.closed_captions)?; - } + write_some_attribute!(w, ",CLOSED-CAPTIONS", &self.closed_captions)?; writeln!(w)?; writeln!(w, "{}", self.uri) } @@ -224,23 +223,23 @@ pub struct AlternativeMedia { } impl AlternativeMedia { - pub fn from_hashmap(mut attrs: HashMap) -> AlternativeMedia { + pub fn from_hashmap(mut attrs: HashMap) -> AlternativeMedia { AlternativeMedia { media_type: attrs .get("TYPE") - .and_then(|s| AlternativeMediaType::from_str(s).ok()) + .and_then(|s| AlternativeMediaType::from_str(s.to_string().as_str()).ok()) .unwrap_or_default(), - uri: attrs.remove("URI"), - group_id: attrs.remove("GROUP-ID").unwrap_or_default(), - language: attrs.remove("LANGUAGE"), - assoc_language: attrs.remove("ASSOC-LANGUAGE"), - name: attrs.remove("NAME").unwrap_or_default(), - default: bool_default_false!(attrs.remove("DEFAULT")), - autoselect: bool_default_false!(attrs.remove("AUTOSELECT")), - forced: bool_default_false!(attrs.remove("FORCED")), - instream_id: attrs.remove("INSTREAM-ID"), - characteristics: attrs.remove("CHARACTERISTICS"), - channels: attrs.remove("CHANNELS"), + uri: attrs.remove("URI").map(|u| u.to_string()), + group_id: attrs.remove("GROUP-ID").unwrap_or_default().to_string(), + language: attrs.remove("LANGUAGE").map(|l| l.to_string()), + assoc_language: attrs.remove("ASSOC-LANGUAGE").map(|a| a.to_string()), + name: attrs.remove("NAME").unwrap_or_default().to_string(), + default: bool_default_false!(attrs.remove("DEFAULT").map(|s| s.to_string())), + autoselect: bool_default_false!(attrs.remove("AUTOSELECT").map(|s| s.to_string())), + forced: bool_default_false!(attrs.remove("FORCED").map(|f| f.to_string())), + instream_id: attrs.remove("INSTREAM-ID").map(|i| i.to_string()), + characteristics: attrs.remove("CHARACTERISTICS").map(|c| c.to_string()), + channels: attrs.remove("CHANNELS").map(|c| c.to_string()), } } @@ -346,14 +345,16 @@ pub struct SessionData { } impl SessionData { - pub fn from_hashmap(mut attrs: HashMap) -> Result { + pub fn from_hashmap( + mut attrs: HashMap, + ) -> Result { let data_id = match attrs.remove("DATA-ID") { Some(data_id) => data_id, None => return Err("EXT-X-SESSION-DATA field without DATA-ID".to_string()), }; - let value = attrs.remove("VALUE"); - let uri = attrs.remove("URI"); + let value = attrs.remove("VALUE").map(|v| v.to_string()); + let uri = attrs.remove("URI").map(|u| u.to_string()); // SessionData must contain either a VALUE or a URI, // but not both https://tools.ietf.org/html/rfc8216#section-4.3.4.4 @@ -375,9 +376,9 @@ impl SessionData { }; Ok(SessionData { - data_id, + data_id: data_id.to_string(), field, - language: attrs.remove("LANGUAGE"), + language: attrs.remove("LANGUAGE").map(|s| s.to_string()), }) } @@ -591,13 +592,13 @@ pub struct Key { } impl Key { - pub fn from_hashmap(mut attrs: HashMap) -> Key { + pub fn from_hashmap(mut attrs: HashMap) -> Key { Key { - method: attrs.remove("METHOD").unwrap_or_default(), - uri: attrs.remove("URI"), - iv: attrs.remove("IV"), - keyformat: attrs.remove("KEYFORMAT"), - keyformatversions: attrs.remove("KEYFORMATVERSIONS"), + method: attrs.remove("METHOD").unwrap_or_default().to_string(), + uri: attrs.remove("URI").map(|u| u.to_string()), + iv: attrs.remove("IV").map(|i| i.to_string()), + keyformat: attrs.remove("KEYFORMAT").map(|k| k.to_string()), + keyformatversions: attrs.remove("KEYFORMATVERSIONS").map(|k| k.to_string()), } } @@ -689,10 +690,13 @@ pub struct Start { } impl Start { - pub fn from_hashmap(mut attrs: HashMap) -> Start { + pub fn from_hashmap(mut attrs: HashMap) -> Start { Start { - time_offset: attrs.remove("TIME-OFFSET").unwrap_or_default(), - precise: attrs.remove("PRECISE").or_else(|| Some("NO".to_string())), + time_offset: attrs.remove("TIME-OFFSET").unwrap_or_default().to_string(), + precise: attrs + .remove("PRECISE") + .map(|a| a.to_string()) + .or_else(|| Some("NO".to_string())), } }