Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add comment functions #41

Open
rmnvgr opened this issue Jul 9, 2021 · 5 comments
Open

Add comment functions #41

rmnvgr opened this issue Jul 9, 2021 · 5 comments
Labels

Comments

@rmnvgr
Copy link

rmnvgr commented Jul 9, 2021

GExiv2 has three functions to manipulate comments:

It would be nice to have them as, contrary to what the gexiv2_metadata_get_comment() doc says, it can return metadata not accessible otherwise.

@felixc felixc added the feature label Jul 11, 2021
@felixc
Copy link
Owner

felixc commented Jul 11, 2021

Thanks for the suggestion! It looks like they are already exposed via the underlying gexiv2-sys FFI wrapper, which makes exposing them here simpler:

https://github.com/felixc/gexiv2-sys/blob/f53a6da4a6bd1d3a35d207c8d72cd500b1c7009c/src/lib.rs#L156-L158

(It also means that if you need this right now, a temporary workaround might be to call these functions directly)

I think (but don't really remember) that I originally didn't expose them because I saw the comment you mentioned that suggests these are just wrappers for bulk-operating on fields that are already individually accessible… but you note that

it can return metadata not accessible otherwise.

Can you elaborate on that? What is not possible to do currently that only these wrappers provide?

@rmnvgr
Copy link
Author

rmnvgr commented Jul 11, 2021

For example, if you take the dirty.jpg test file from mat2, it contains a "Created with GIMP" comment.

If you try each of the fields mentioned in the docs, none of them return the comment. However, gexiv2_metadata_get_comment() returns the comment correctly.

@jonas-hagen
Copy link

There is a JPEG comment, which is stored in the JPEG meta data along with width, height, depth information. This has nothing to do with Exif, XMP and IPTC metadata and is thus no accessible by get_tag_* functions.

It would be nice to have these functions wrapped to access the JPEG comment as well. Not all jpeg decoders for rust support reading the comment.

@felixc
Copy link
Owner

felixc commented Aug 2, 2021

Thanks for the clarification of where that extra metadata was coming from. In principle I agree it would be very desirable to be able to set/unset JPEG comments through this library as well, so I'm on board with the general goal here. Do you happen to know if there's any other JPEG metadata other than these comments that we should generalize this to?

What I'm worried about is that exposing gexiv2_metadata_get/set/clear_comment() may be a sort of roundabout side-effect-based of accomplishing the real goal, and I don't know if we want to encourage its use when upstream gexiv2 seem to be deprecating it. If we're using it to get at the JPEG comment, it seems undesirable that it also does stuff to a somewhat arbitrary set of other tags...

I wonder if upstream gexiv2 would be open to a PR implementing a set of functions for JPEG comments specifically (and/or any other JPEG metadata, if any such exists)... How would you feel about that approach?

@rmnvgr
Copy link
Author

rmnvgr commented Aug 6, 2021

Well I don't see that gexiv2 is deprecating the use of gexiv2_metadata_get/set/clear_comment(). I only read that they are recommending to "use Exiv2 tags directly rather than this method, which is more useful for quick or casual use".

It may be nice indeed to have specific functions for accessing only the comment, but if they are accepted, it will take time before downstreams are updated. Having the already existing functions available in rexiv2 would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants