Introduce unrecognized cases to keep parsing forward compatible

theRealRobG · theRealRobG · commit 02d2cb9f8a36 · 2024-09-06T23:45:37.000-04:00
diff --git a/mambaSharedFramework/HLSValueTypes.swift b/mambaSharedFramework/HLSValueTypes.swift
@@ -255,25 +255,41 @@ public struct HLSChannels: Equatable, FailableStringLiteralConvertible {
     /// This parameter is an array of Special Usage Identifiers.
     public let specialUsageIdentifiers: [SpecialUsageIdentifier]
 
-    public enum SpecialUsageIdentifier: String {
+    public enum SpecialUsageIdentifier: RawRepresentable, Equatable {
         /// The audio is binaural (either recorded or synthesized). It SHOULD NOT be dynamically spatialized. It is best
         /// suited for delivery to headphones.
-        case binaural = "BINAURAL"
+        case binaural
         /// The audio is pre-processed content that SHOULD NOT be dynamically spatialized. It is suitable to deliver to
         /// either headphones or speakers.
-        case immersive = "IMMERSIVE"
+        case immersive
         /// The audio is a downmix derivative of some other audio. If desired, the downmix may be used as a subtitute
         /// for alternative Renditions in the same group with compatible attributes and a greater channel count. It MAY
         /// be dynamically spatialized.
-        case downmix = "DOWNMIX"
+        case downmix
+        /// The audio identifier is not recognized by this library; however, we provide the raw identifier string that
+        /// existed in the manifest.
+        case unrecognized(String)
+
+        public var rawValue: String {
+            switch self {
+            case .binaural: return "BINAURAL"
+            case .immersive: return "IMMERSIVE"
+            case .downmix: return "DOWNMIX"
+            case .unrecognized(let string): return string
+            }
+        }
+
+        public init?(rawValue: String) {
+            self.init(str: Substring(rawValue))
+        }
 
         /// Allows `init` without having to allocate a new `String` object.
-        init?(str: Substring) {
+        init(str: Substring) {
             switch str {
             case "BINAURAL": self = .binaural
             case "IMMERSIVE": self = .immersive
             case "DOWNMIX": self = .downmix
-            default: return nil
+            default: self = .unrecognized(String(str))
             }
         }
     }
@@ -287,15 +303,7 @@ public struct HLSChannels: Equatable, FailableStringLiteralConvertible {
             switch index {
             case 0: count = Self.parseChannelCount(str: str)
             case 1: spatialAudioCodingIdentifiers = Self.parseSpatialAudioCodingIdentifiers(str: str)
-            case 2:
-                guard let ids = Self.parseSpecialUsageIdentifiers(str: str) else {
-                    // In the case that we don't recognize one of the special usage identifiers, leading to nil being
-                    // parsed out, I believe it is better to fail the entire parsing, as otherwise we could mislead the
-                    // user of the library into thinking that there are less special usage identifiers than there
-                    // actually are in the CHANNELS attribtue.
-                    return nil
-                }
-                specialUsageIdentifiers = ids
+            case 2: specialUsageIdentifiers = Self.parseSpecialUsageIdentifiers(str: str)
             default: break // In the future there may be more parameters defined.
             }
         }
@@ -331,16 +339,8 @@ public struct HLSChannels: Equatable, FailableStringLiteralConvertible {
         return identifiers
     }
 
-    private static func parseSpecialUsageIdentifiers(str: Substring) -> [SpecialUsageIdentifier]? {
-        let split = str.split(separator: ",")
-        var identifiers = [SpecialUsageIdentifier]()
-        for id in split {
-            guard let specialUsageId = SpecialUsageIdentifier(str: id) else {
-                return nil
-            }
-            identifiers.append(specialUsageId)
-        }
-        return identifiers
+    private static func parseSpecialUsageIdentifiers(str: Substring) -> [SpecialUsageIdentifier] {
+        str.split(separator: ",").map { SpecialUsageIdentifier(str: $0) }
     }
 }
 
@@ -435,36 +435,42 @@ public struct HLSVideoLayout: Equatable, FailableStringLiteralConvertible {
     /// is predominantly monoscopic then the Multivariant Playlist SHOULD specify `REQ-VIDEO-LAYOUT="CH-MONO,CH-STEREO"`.
     public let predominantLayout: VideoLayoutIdentifier
 
-    public enum VideoLayoutIdentifier: String {
+    public enum VideoLayoutIdentifier: RawRepresentable, Equatable {
         /// Monoscopic.
         ///
         /// Indicates that a single image is present.
-        case chMono = "CH-MONO"
+        case chMono
         /// Stereoscopic.
         ///
         /// Indicates that both left and right eye images are present.
-        case chStereo = "CH-STEREO"
+        case chStereo
+        /// The video layout identifier is not recognized by this library; however, we provide the raw identifier string
+        /// that existed in the manifest.
+        case unrecognized(String)
+
+        public var rawValue: String {
+            switch self {
+            case .chMono: return "CH-MONO"
+            case .chStereo: return "CH-STEREO"
+            case .unrecognized(let string): return string
+            }
+        }
+
+        public init?(rawValue: String) {
+            self.init(str: Substring(rawValue))
+        }
 
-        init?(str: Substring) {
+        init(str: Substring) {
             switch str {
             case "CH-MONO": self = .chMono
             case "CH-STEREO": self = .chStereo
-            default: return nil
+            default: self = .unrecognized(String(str))
             }
         }
     }
 
     public init?(string: String) {
-        var layouts = [VideoLayoutIdentifier]()
-        for str in string.split(separator: ",") {
-            if let layout = VideoLayoutIdentifier(str: str) {
-                layouts.append(layout)
-            } else {
-                // Favor failing to parse the whole array if we find an unrecognized layout, so that we don't risk mis-
-                // reporting the existing layouts.
-                return nil
-            }
-        }
+        let layouts = string.split(separator: ",").map { VideoLayoutIdentifier(str: $0) }
         guard let firstLayout = layouts.first else {
             return nil
         }
diff --git a/mambaTests/Tag Validator Tests/GenericDictionaryTagValidatorTests.swift b/mambaTests/Tag Validator Tests/GenericDictionaryTagValidatorTests.swift
@@ -762,8 +762,7 @@ class GenericDictionaryTagValidatorTests: XCTestCase {
                                         .resolution,
                                         .frameRate,
                                         .hdcpLevel,
-                                        .videoRange,
-                                        .reqVideoLayout]
+                                        .videoRange]
 
         validate(tag: PantosTag.EXT_X_STREAM_INF,
                  tagData: tagData,
@@ -913,8 +912,7 @@ class GenericDictionaryTagValidatorTests: XCTestCase {
                                         .programId,
                                         .resolution,
                                         .hdcpLevel,
-                                        .videoRange,
-                                        .reqVideoLayout]
+                                        .videoRange]
 
         validate(tag: PantosTag.EXT_X_I_FRAME_STREAM_INF,
                  tagData: tagData,
diff --git a/mambaTests/Util Tests/Value Types/HLSChannelsTests.swift b/mambaTests/Util Tests/Value Types/HLSChannelsTests.swift
@@ -124,11 +124,13 @@ class HLSChannelsTests: XCTestCase {
         XCTAssertEqual(expectedChannels, actualChannels)
     }
 
-    // In the case that we don't recognize the special usage identifier, I think it is better to fail parsing the entire
-    // CHANNELS attribute, as otherwise we risk misleading the user of the library into thinking that the special usage
-    // is less than it actually is.
     func test_sixChannelUnknownSpecialUsageIdentifier() {
         let actualChannels = HLSChannels(string: sixChannelUnknownSpecialUsageIdentifier)
-        XCTAssertNil(actualChannels)
+        let expectedChannels = HLSChannels(
+            count: 6,
+            spatialAudioCodingIdentifiers: [],
+            specialUsageIdentifiers: [.unrecognized("NEW-IDENTIFIER")]
+        )
+        XCTAssertEqual(expectedChannels, actualChannels)
     }
 }
diff --git a/mambaTests/Util Tests/Value Types/HLSVideoLayoutTests.swift b/mambaTests/Util Tests/Value Types/HLSVideoLayoutTests.swift
@@ -23,19 +23,26 @@ import mamba
 
 class HLSVideoLayoutTests: XCTestCase {
     let empty = ""
-    let invalidVideoLayout = "CH-TRI"
+    let unrecognizedVideoLayout = "CH-TRI"
     let monoLayout = "CH-MONO"
     let stereoLayout = "CH-STEREO"
     let stereoWithMonoLayout = "CH-STEREO,CH-MONO"
     let monoWithStereoLayout = "CH-MONO,CH-STEREO"
-    let monoWithStereoWithUnknownLayout = "CH-MONO,CH-STEREO,CH-TRI"
+    let monoWithStereoWithUnrecognizedLayout = "CH-MONO,CH-STEREO,CH-TRI"
 
     func test_empty() {
         XCTAssertNil(HLSVideoLayout(string: empty))
     }
 
-    func test_invalidVideoLayout() {
-        XCTAssertNil(HLSVideoLayout(string: invalidVideoLayout))
+    func test_unrecognizedVideoLayout() {
+        guard let videoLayout = HLSVideoLayout(string: unrecognizedVideoLayout) else {
+            return XCTFail("Expected to parse REQ-VIDEO-LAYOUT from \(unrecognizedVideoLayout).")
+        }
+        XCTAssertEqual(videoLayout.layouts, [.unrecognized(unrecognizedVideoLayout)])
+        XCTAssertEqual(videoLayout.predominantLayout, .unrecognized(unrecognizedVideoLayout))
+        XCTAssertFalse(videoLayout.contains(.chMono))
+        XCTAssertFalse(videoLayout.contains(.chStereo))
+        XCTAssertTrue(videoLayout.contains(.unrecognized(unrecognizedVideoLayout)))
     }
 
     func test_monoLayout() {
@@ -78,7 +85,14 @@ class HLSVideoLayoutTests: XCTestCase {
         XCTAssertTrue(videoLayout.contains(.chStereo))
     }
 
-    func test_monoWithStereoWithUnknownLayout() {
-        XCTAssertNil(HLSVideoLayout(string: monoWithStereoWithUnknownLayout))
+    func test_monoWithStereoWithUnrecognizedLayout() {
+        guard let videoLayout = HLSVideoLayout(string: monoWithStereoWithUnrecognizedLayout) else {
+            return XCTFail("Expected to parse REQ-VIDEO-LAYOUT from \(monoWithStereoWithUnrecognizedLayout).")
+        }
+        XCTAssertEqual(videoLayout.layouts, [.chMono, .chStereo, .unrecognized("CH-TRI")])
+        XCTAssertEqual(videoLayout.predominantLayout, .chMono)
+        XCTAssertTrue(videoLayout.contains(.chMono))
+        XCTAssertTrue(videoLayout.contains(.chStereo))
+        XCTAssertTrue(videoLayout.contains(.unrecognized("CH-TRI")))
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -124,11 +124,13 @@ class HLSChannelsTests: XCTestCase {`
`124`	`124`	`XCTAssertEqual(expectedChannels, actualChannels)`
`125`	`125`	`}`
`126`	`126`
`127`		`- // In the case that we don't recognize the special usage identifier, I think it is better to fail parsing the entire`
`128`		`- // CHANNELS attribute, as otherwise we risk misleading the user of the library into thinking that the special usage`
`129`		`- // is less than it actually is.`
`130`	`127`	`func test_sixChannelUnknownSpecialUsageIdentifier() {`
`131`	`128`	`let actualChannels = HLSChannels(string: sixChannelUnknownSpecialUsageIdentifier)`
`132`		`- XCTAssertNil(actualChannels)`
	`129`	`+ let expectedChannels = HLSChannels(`
	`130`	`+ count: 6,`
	`131`	`+ spatialAudioCodingIdentifiers: [],`
	`132`	`+ specialUsageIdentifiers: [.unrecognized("NEW-IDENTIFIER")]`
	`133`	`+ )`
	`134`	`+ XCTAssertEqual(expectedChannels, actualChannels)`
`133`	`135`	`}`
`134`	`136`	`}`