address review: prefer the key with a usable manifest when keys name one cryptomatte layer

crates/codecs/src/codecs/exr_cryptomatte.rs

@@ -170,25 +170,38 @@ pub(crate) fn layers(bytes: &[u8], limits: &Limits) -> Result<Vec<CryptomatteLay
         // result grow with keys * channels. A second key naming a listed layer is skipped;
         // `decode` resolves a name to its first layer anyway.
         let mut streams_for = layer_streams(header);
-        for key in cryptomatte_keys(&header.own_attributes.other) {
-            let Some(name) = attribute(&header.own_attributes.other, &key, "name") else { continue };
-            let Some(channels) = streams_for.remove(&normalize_layer(&name)) else { continue };
-            if channels.is_empty() {
-                continue;
-            }
-            let manifest = attribute(&header.own_attributes.other, &key, "manifest")
-                .filter(|m| m.len() <= MANIFEST_CAP)
-                .map(|m| parse_manifest(m.as_str()))
-                .unwrap_or_default();
+        // A key whose manifest is missing or unparseable only stands in for its layer when no later
+        // key naming it has a usable one.
+        let mut fallback: std::collections::HashMap<String, (String, String)> = std::collections::HashMap::new();
+        let other = &header.own_attributes.other;
+        let mut push = |name: String, key: String, manifest: Vec<_>, channels: Vec<String>| {
             out.push(CryptomatteLayer {
                 part,
                 name,
-                key: key.clone(),
-                conversion: attribute(&header.own_attributes.other, &key, "conversion"),
+                conversion: attribute(other, &key, "conversion"),
                 manifest,
-                sidecar_manifest: attribute(&header.own_attributes.other, &key, "manif_file"),
+                sidecar_manifest: attribute(other, &key, "manif_file"),
+                key,
                 channels,
             });
+        };
+        for key in cryptomatte_keys(other) {
+            let Some(name) = attribute(other, &key, "name") else { continue };
+            let norm = normalize_layer(&name);
+            if streams_for.get(&norm).is_none_or(|c| c.is_empty()) {
+                continue;
+            }
+            let manifest = attribute(other, &key, "manifest").filter(|m| m.len() <= MANIFEST_CAP).map(|m| parse_manifest(m.as_str())).unwrap_or_default();
+            if manifest.is_empty() {
+                fallback.entry(norm).or_insert((name, key));
+            } else if let Some(channels) = streams_for.remove(&norm) {
+                push(name, key, manifest, channels);
+            }
+        }
+        for (norm, (name, key)) in fallback {
+            if let Some(channels) = streams_for.remove(&norm) {
+                push(name, key, Vec::new(), channels);
+            }
         }
     }
     out.sort_by(|a, b| (&a.part, &a.name).cmp(&(&b.part, &b.name)));

crates/codecs/tests/it/exr_cryptomatte.rs

@@ -299,3 +299,29 @@ fn keys_naming_one_layer_list_it_once() {
     let buf = decode_cryptomatte(&bytes, &found[0].name, &Limits::default()).unwrap();
     assert_eq!(buf.at(0, 0).len(), 1);
 }
+
+/// Of keys naming one layer, the first with a usable manifest lists it, not an earlier one
+/// whose manifest is missing.
+#[test]
+fn a_later_key_with_a_manifest_beats_an_earlier_one_without() {
+    let (w, h) = (4usize, 4usize);
+    let mk = |n: &str, v: f32| AnyChannel::new(n, FlatSamples::F32(vec![v; w * h]));
+    let list = vec![mk("crypto_asset00.red", cryptomatte_id("cube")), mk("crypto_asset00.green", 1.0)];
+    let mut attrs = LayerAttributes::named("crypto_asset");
+    attrs.other.insert(text("cryptomatte/a/name"), AttributeValue::Text(text("CryptoAsset")));
+    attrs.other.insert(text("cryptomatte/b/name"), AttributeValue::Text(text("crypto_asset")));
+    attrs.other.insert(text("cryptomatte/b/manifest"), AttributeValue::Text(text("{\"cube\":\"3f800000\"}")));
+    let layer = Layer::new(
+        (w, h),
+        attrs,
+        Encoding { compression: Compression::Uncompressed, blocks: Blocks::ScanLines, line_order: LineOrder::Increasing },
+        AnyChannels::sort(list.into_iter().collect()),
+    );
+    let image = exr::image::Image::from_layers(ImageAttributes::new(IntegerBounds::new((0, 0), (w, h))), vec![layer]);
+    let mut out = std::io::Cursor::new(Vec::new());
+    image.write().to_buffered(&mut out).expect("synthetic cryptomatte exr writes");
+    let found = cryptomatte_layers(&out.into_inner(), &Limits::default()).unwrap();
+    assert_eq!(found.len(), 1, "{found:?}");
+    assert_eq!(found[0].key, "b");
+    assert_eq!(found[0].manifest.len(), 1);
+}