contentauth / c2pa-rs

Rust SDK for the core C2PA (Coalition for Content Provenance and Authenticity) specification
Other
99 stars 42 forks source link

"Bad BMFF" when signing mp4 file via builder.sign_file #462

Closed schnerd closed 1 week ago

schnerd commented 2 months ago

Context

I have an MP4 file that I'd like to attach C2PA metadata to. I'm trying to use Builder.sign_file to sign the video file (and importantly calculate the hash so I don't end up with a claim.hardBindings.missing error).

When I run Builder.sign_file on the mp4, I get the following error:

Error: asset could not be parsed: Bad BMFF

After some debugging, I see that this specific line is triggering the error:

https://github.com/contentauth/c2pa-rs/blob/ca3df05381f5bccb864502d09274432aa37e6018/sdk/src/asset_handlers/bmff_io.rs#L945-L946

If I log the unmapped error it is a Error: Bad file descriptor (os error 9). I've tried this on mp4s from many different sources and it's the same error.

Reproduction

I created a minimal test case that replicates the issue: https://github.com/schnerd/c2pa-rs/commit/ebb069854ca6b2d94c9aa27846058d7657b5ce7a

Note that the mp4 I added there is a test mp4 with no c2pa metadata (the existing mp4s in the fixtures directory already contain c2pa metadata, so didn't want to test with those)

Other notes

I initially tried Manifest.embed_from_memory, but that doesn't seem to generate hashes for mp4s, I think because get_object_locations_from_stream is not implemented for BmffIO. I'm now using Manifest.embed instead, which seems to work in the meantime. But it looks like Builder.sign_file is the direction that things are headed in the future so figured it's still worth opening an issue (but feel free to close if it's expected because it's an unstable api as of 0.32.0)

mauricefisher64 commented 1 week ago

You are correct the Builder API is where we are heading. It is a work in progress so you may encounter issues. Please try again with the latest since many things have been stabilized.

schnerd commented 1 week ago

👍 – I'm fine with closing this issue as Manifest.embed worked for me in the interim, I will take another look at the Builder API soon!