Announcing rexiv2 library for image metadata + request for code review


#1

Hello Rustaceans!

(Note: I’ve crossposted this on the subreddit because I’m not sure where discussions like this are happening/should happen. If people feel this is spammy I’ll gladly delete one or the other post)

I’m fairly new to Rust, so to help me learn the language I decided to build some photography workflow tools for myself in the language. The first thing I needed was a library to enable read/write access to the metadata stored in media files (Exif, XMP, and IPTC).

rexiv2 is my library for doing this. It’s a Rust wrapper around the gexiv2 library, which is itself a GObject-based wrapper around the Exiv2 library, which provides what we need. This is the same metadata library used by Gimp and Shotwell. I’ve just pushed to GitHub a version 0.1.0-pre unstable preview that I believe provides all of the necessary basic functionality, albeit in a potentially un-ergonomic, un-Rust-like, rough kind of way.

Links: source, docs, crates.io.

Seeing as how I’m very new to the language, I’d really appreciate feedback/suggestions on anything and everything: coding style, API conventions, wasteful or inefficient copies/allocations that I’m doing, potential FFI mistakes… everything!

I also have some specific questions:

  1. How should one pass an array of strings to a C FFI function? You can see how I received one in the get_tag_multiple_strings method, but I’m not sure how to do the inverse in set_tag_multiple_strings.

  2. What should APIs return, String or &str? I think I grasp the distinction, and I see why it makes sense to consume &strs, but is it possible to do the same thing with what you return? I ran into problems with lifetimes.

  3. Are the stability attributes used in the core language/standard library meant for use by libraries like this as well? Should I be tagging things unstable? Is that worthwhile given the version number?

  4. Building through Cargo already informs rustc and rustdoc that this is a lib type crate, and that it’s called rexiv2. What are the #![crate_type = lib] and crate_name annotations for? Should they be there?

  5. What’s the best practice for third-party library dependencies? My Linux package management background strongly protests against bundling the source, but making users go elsewhere to download and install stuff isn’t great either. Should build.rs look for the libraries and print a message? Attempt to download them? Fetch the source and build them?

Thank you also to all the folks on IRC who helped out with my beginner questions!


#2

[quote=“felixc, post:1, topic:414”]

  1. How should one pass an array of strings to a C FFI function?
    [/quote]That seems like a job for c_vec that’s recently been dropped from the std lib. It’s preserved here: https://crates.io/crates/c_vec

[quote]2) What should APIs return, String or &str?
[/quote]Slices are views into data owned by someone. Someone needs to own a String, usually you return it directly, but your struct could own the Strings and return slices (not to say it makes sense in this case).

[quote]
4) Building through Cargo already informs rustc and rustdoc that this is a lib type crate, and that it’s called rexiv2. What are the #![crate_type = lib] and crate_name annotations for? Should they be there?
[/quote]I guess you might need them if you don’t use cargo. I believe they’re redundant.

[quote]
Should build.rs look for the libraries and print a message? Attempt to download them? Fetch the source and build them?
[/quote]At the very least gexiv2.rs should be in a separate gexiv2-sys crate and have its build.rs check with pkg-config and complain if the library isn’t available in the system.
http://doc.crates.io/build-script.html


#3

Ooh, nice! I’ve been wanting an EXIF lib for a project for a little while now… it’s great when someone else solves my problems. :smile:


#4

Thanks for the pointer; I looked into it but ultimately with a great deal of help from some folks on IRC (thanks Sergio and shep-home!) we were able to do without. Shep-home’s example: http://is.gd/EFS9tm (you can then take ptrs.as_ptr()).

Gotcha — that meshes with my understanding as well, so I’ll leave the return types as String.

Ah! Somehow I missed that part of the docs while researching how Cargo handles this. I’ll look into splitting the crates at some point.

Thank you for this info!


#5

Thanks for sharing.

Looking at the getter…

while !(*c_vals.offset(cur_offset) as i8 == 0) {

*c_vals.offset(cur_offset) is a pointer, right? Shouldn’t you cast it to usize or even put it like this !(*c_vals.offset(cur_offset)).is_null()


#6

Thanks for pointing this out! I’d initially done the odd cast and comparison to 0 because I misinterpreted the gexiv2 docs, which say the array is “zero-terminated”. Examining the implementation shows that it is indeed a regular null, as you suspected.

Now I’m just trying to understand why the original incorrect implementation actually (partially) worked!


#7

[quote=“felixc, post:6, topic:414”]
Now I’m just trying to understand why the original incorrect implementation actually (partially) worked!
[/quote]Well, comparison to zero is correct but you need to compare the whole value and usize is the correct type to cast to because it has the same size as a pointer. Comparing a single byte would still work most of the time but stop early when encountering a 256-byte aligned pointer.