Task title: Improve gThumb scripting with new user prompt code

Benefits: This project will make gThumb scripting more powerful, for batch processing of digital photos.

Requirements: A basic understanding of C. Familiarity with C string handling and hash tables.

Task description:

The trunk version of gthumb includes a powerful scripting feature. Certain filename codes (like "%f", to iterate over each file name) can be inserted in user-defined scripts. Blocks of square-bracket-enclosed text can also be inserted as placeholders for values returned from user prompts.

A new feature was recently added: curly brackets can be used to enclose a strftime date code that is expanded for each photo, based on its date. (See http://code.google.com/p/google-highly-open-participation-gnome/issues/detail?id=82).

This can be used to create subdirectories based on photo dates, for automatic photo sorting (as an example), using this code:

mkdir -p %p/{%Y-%m-%d}; cp %f %p/{%Y-%m-%d}/

However, users will often want to add a chunk of text (a comment) tied to a particular date. This can not be done, currently.

For instance, instead of creating a subdirectory called "2008-05-01", they might wish to expand it to "2008-05-01_MyBirthday". To do this, I would like to add another code - an all-upper-case text prompt enclosed curly brackets. It would be used like this:

mkdir -p %p/{%Y-%m-%d_}{COMMENT}; cp %f %p/{%Y-%m-%d_}{COMMENT}/

The {COMMENT} would be tied to the last expanded strftime string found in the script iteration being handled. A hash table would tie the expanded date codes to the comments. If no hash entry existed already, a user prompt would be generated to obtain input. Once the date text and comment were recorded in the hash, they could be re-used in later iterations of the script. In the above example, "2008-05-01" would be tied to "MyBirthday". All future occurrences (for all iterations) of a strftime code expanded to "2008-05-01" would also use "MyBirthday" in the {COMMENT} field, automatically.

The new code would go in src/dlg-scripts.c, after the strftime-expansion code.

You would provide a patch against trunk.

You will need to be able to compile gThumb from svn trunk, without experiencing any build errors. (Please, try it before claiming this task!)

You should be comfortable dealing with C strings and hash tables.

There is already ample code to copy and modify in dlg-scripts.c. There is existing code showing how to recognize a bracket-enclosed token, and how to call a prompt dialog to obtain input. You may want to re-factor some of this code to avoid code duplication (i.e., generalize it).

The existing code assumes that anything in curly brackets is a strftime code, so you'll have to modify the code to make an exception for a token in inside curly brackets consisting entirely of upper-case letters.

You will need to modify the edit-scripts glade file to mention the new feature, and update doc/C/gthumb.xml to fully explain the new function.

I hope I've explained this clearly. Basically, instead of a per-file prompt (which is implemented by lower-case text in square brackets), I want a per-date-string prompt, which may be shared between several file iterations.

Links: http://svn.gnome.org/viewvc/gthumb/trunk/src/dlg-scripts.c?view=markup http://code.google.com/p/google-highly-open-participation-gnome/issues/detail?id=82

Estimated time: 7 days.

Primary contact: Michael Chudobiak (mjc@avtechpulse.com)

I claim this task.

I've got some code written in, but I can't test it properly because get_metadata_time is always returning 0. I have all the libraries mentioned by autogen.sh, except for that XDisplayChangeGamma or whatever. I put a call to printf in just after the formatted time is appended to g_string_append, and I always get the result "%B evaluates with time exif_time 0 to December"

Also, I forsee a possible bug; if you write a script like the following:

{%M}{PROMPT}{%y}{PROMPT}

if both time values have the same result, the hash lookup will not see them differently. I could add another layer of hashing against the strftime string, or leave as-is. I think that since the identifiers should mostly be unique, that won't be a major problem, but I could be wrong.

Okay, I think I found the problem with my timestamps. get_metadata_time references get_mplayer_time, which is dependent on the midentify script, which only works for movies. Clearly, this is not the function we want for timestamping. The function to call seems to be the one generating the timestamp in the bar along the bottom of the image.

I've modified the call to get_metadata_time to instead use a FileData struct, and try exif_time, and, failing that, the modification time. This always gives me a good timestamp.

However, now that I'm actually testing it, there seems to be an issue with the destruction of a hash of hashes. Does anyone know how a hash of hashes should be implemented? I used g_hash_table_new_full(g_str_hash, g_str_equal, g_free, forward_hash_destroy), where forward_hash_destroy just calls g_hash_table_destroy. This segfaults some time after exiting the last call to forward_hash_destroy. Any solutions known?

Rideau3,

Hi, welcome! What's you real name?

1) Regarding: {%M}{PROMPTA}{%y}{PROMPTB}:

I suggested in the intro above that "{COMMENT} would be tied to the last expanded strftime string found in the script iteration being handled", so both {PROMPTA} and {PROMPTB} would be tied to {%y}. This keeps it simple, but reduces functionality.

If you want to implement it so that {PROMPTA} was tied to {%M}, and {PROMPTB} was tied to {%y}, I would be OK with that too. We won't really know how people will want to use it until they try it and start filing bug reports :-)

Perhaps you'll see a better way once you start coding and testing... I'm pretty flexible here. There is probably a better way to link the prompts to the strftime codes, but I haven't though of it yet.

2) Regarding get_metadata_time:

Oops, there is a bug in the new strftime code. It should call get_metadata_time (which it does) and if that returns 0, then using the file mtime (that part is missing). If you check the usage of get_metadata_time in other parts of gthumb (like in src/gth-browser.c), you'll see that it is usually used that way (falling back to mtime).

get_metadata_time should return the exif DateTime value for jpeg files. Are you seeing something else? For videos, it tries to use midentify, but it's OK if midentify is not installed - it just returns 0 then. (The midentify bit will be removed soon, to be replaced with gstreamer code, once gstreamer reads date metadata - see http://bugzilla.gnome.org/show_bug.cgi?id=503582. The midentify code is an ugly hack.)

3) Hash of hashes.

You lost me here. Why do you need a hash of hashes? I haven't tried that before, personally...

• Mike

Hi!

My name is Sean Hunt, if you need it.

1) Oh, I hadn't thought of that. I had put the code directly into the function for strftime strings. I've already coded it in that way so I might as well leave that in. Perhaps I could make it so that {lowercasepromptname} is bound to the last one in the string. Seems like it wouldn't integrate easily to the way I've already done things, though. I guess I can see depending on how easily this integrates otherwise.

2) Yeah, that's what I figured. I was testing on .png files, so that's why there was no ExifData to use.

3) I needed a hash of hashes to store data for the prompts. I need the first hash to look up the prompt name and the second to find the strftime-formatted data.

It may just have been a symptom of some other memory-related mistakes I made - I had forgotten that g_string_new doesn't allocate new data (predictably after having used it correctly to recall the previous strftime value).

I realized that it's probably faster for me, though, if I just pick some separator character that can't be used in the strftime function ('}' comes to mind) and lookup the hash by the prompt name, strftime format string, and time string all mashed together. It also solves problem (1). I'm also thinking of making each prompt with the same name refer to the same thing (whatever came before the first one). That seems more intuitive.

Also, it's possible to introduce invalid characters to the hash key via the strftime function. Is there any function I can call to check whether or not a character is valid within a hash key? (the function seems to come out of GConf, so I'd hate to have to dig for the source)

0) I don't need your name for anything - I just prefer to know it :-)

1) Your call.

2) jpegs are the target use case (from digital cameras).

3) I think the "key = mash of 3 things" approach sounds good. I've never considered the issue of illegal characters in hash keys. I'm not sure what is illegal. However, maybe you could use shell_escape or a similar function to translate illegal characters.

• Mike

I'm finished!

Turns out the illegal keys weren't the hashes, but from GConf complaining about the values remembered for prompting.

I added code that makes each prompt name bound to the same text each time it appears in the same script. That is, in the script "mv %f %n-{%M}-{COMMENT}-{%Y}-{COMMENT}", COMMENT will be dependent only on the month name, never the year.

I found that implementing a sort of lookahead to always match the last value is much more difficult, because it would require that I remember string indices and then do some insertions.

So instead, I made it so that you can prefix anything in {} brackets with a "#" placed after the opening bracket. This causes the code to be evaluated normally, except that it is removed, rather than replaced. This can be used to bind a prompt to a date that isn't used, or to bind a prompt to a date before it would otherwise be used. So the script 'mkdir %p/{#%B}{COMMENT}; cp %f %p/{COMMENT}/%b' would move each picture to a subfolder named by the month.

This patch includes patches to the documentation you mentioned.

The "#" stuff is a clever addition. Nice! The general approach looks good to me.

The next step is to tidy up the patch a bit, by checking style and simplifying as much as possible:

1) Please always put a space between function names the the argument brackets: foo (bar) rather than foo(bar).

2) Double-check for unnecessary code, like this line:

time_t exif_time = get_metadata_time (NULL, filename);

3) Personally, I prefer to remove brackets when they are not required, to reduce clutter - unless it fits more nicely into the style of the code around it. For example, I prefer:

if (!exif_time) exif_time = fd->mtime;

instead of:

if (!exif_time) { exif_time = fd->mtime; }

4) It is probably worth adding an example of "#" usage to the documentation. It's a bit difficult to understand the point of it at first.

I haven't tested the code yet - I'll do that after the updated patch is in.

• Mike

Okay. Here's the fixed patch. I've added one example of "#" documentation, but I couldn't think of any more.

Great! Patch committed, task done!

http://svn.gnome.org/viewvc/gthumb?rev=2179&view=rev

• Mike