safx / LightroomPlugin-ClarifaiTagger

A Lightroom plugin suggests keywords of photo for you
MIT License
43 stars 15 forks source link

Windows Version Error #1

Open sw3d-de opened 8 years ago

sw3d-de commented 8 years ago

Works on Mac without problem. On Windows I get an error message: An internal error has occurred: [string "JSON.lua"]:383: nil passed to JSON:decode()

LowellMontgomery commented 8 years ago

Hmmm... JSON.lua is from an external source, but could be the implementation of it here is not Windows-friendly, somehow, since the developer of this app did say he'd only tested on Mac (and it worked). I can also confirm that it works (and quite well) on OS X. (Sorry I cannot be of any help to resolving an issue on Windows since I don't have a machine with Windows installed right now.)

I've also confirmed it works well with the latest version of the JSON.lua file from Jeffrey Friedl, so maybe that would resolve this issue. I can offer a pull request since I've got that change locally and confirmed it at least doesn't seem to break anything I've noticed. I think the newer version does include quite a few fixes, plus a lot more documentation of that file and a couple areas that look like they offer new functionality (which may or may not be useful for this project).

@safx Thanks for sharing this cool plugin... and documenting it so well, too. I'm surprised there aren't a lot more users for this. I'll write a little article about it and maybe see if that helps bring a bit more attention and/or people who can also verify/help fix any bug for Windows users.

mcrosson commented 8 years ago

@LowellMontgomery the patch you submitted for JSON.lua doesn't fix the underlying bug. I just cherry-picked your change and I am still seeing the nil error. I dug into the bug a bit and in ClarifaiAPI.lua the "local body, reshdrs = LrHttp.postMultipart(tagAPIURL, mimeChunks, headers);" line in the getTags_impl method is failing. From what I can tell, on Windows the thumbnailPath which is set just above the LrHttp.post call is a path to a file that is an invalid JPEG. Photoshop errors with "Could not complete your request because an invalid DQT JPEG segment QTable number is found (it must be < 4)". I have no idea what this means but the return's from the LrHttp.post call are all nil and if the file it's trying to submit is invalid... I have a hunch I found the source of the issue.

mcrosson commented 8 years ago

And now I'm stuck with "{error={errorCode="unknown",name="The parameter is incorrect." after trying the LrHttp.postMultipart call. I think we have a bug on the Windows platform or newer releases of LR.

Also, if I try to upload the generated thumbnail from the code to the Clarifai website demo it fails with "Error unable to load file".

LowellMontgomery commented 8 years ago

@mcrosson That's getting into parts of the project I haven't really dug into yet, but I've actually also seen the exact same error now (albeit only occasionally) on Mac. If you are getting that error consistently, I suspect you have found a significant bug in the plugin logic for Windows, but that same error is seen (apparently) if something just glitches (network issues?) and no JSON response is received or something other than what is expected (i.e. something from the server side). Anyway, we should try to fix that, but it would be helpful to have Windows users testing and/or helping find a workaround since the consistent failure only seems to happen on Windows. For the occasional failures, we should improve the behavior to maybe try again and/or at least provide a more "human" message for users.

mcrosson commented 8 years ago

@LowellMontgomery I gave up on trying to put together a fix. I ended up going a different direction...

I created my own Lightroom export plugin that leverages the Clarifai API for tagging. It uses the export process instead of a menu item to initiate the tagging and has quite a few extra features. Features like choosing the model/detection to use, more resizing options, auto-tagging, API usage stats and more. It should work out of the box on Windows / Mac.

Project page: https://github.com/mcrosson/lr_plugin_computer_vision_tagging Features/Changelog: https://github.com/mcrosson/lr_plugin_computer_vision_tagging/blob/master/docs/ProjectPlan.md

LowellMontgomery commented 8 years ago

Hi Mike,

Thanks for getting back to me so quickly.

Your implementation sounds awesome. I have done quite a bit in terms of feature expansion in my fork of the safx/LightroomPlugin-ClarifaiTagger, but maybe I should try your “extra features” and try to merge the best of both worlds. It sounds like you have done some interesting things there. Personally, I still want to be able to do additional metadata processing after the initial Clarifai-powered tagging. Does using it as an export process mean it’s the last step in your workflow, or was that just a way of sending the images to the system?

Some of the feature improvements I’ve added to the original safx implementation include: 1) Support for finding “already existing terms” in a hierarchical list instead of just a flat list. Without this, it created more work than it saved since adding terms meant creating duplicate terms that I then had to manually merge with existing terms, a tedious process.

2) Related to the above, I can also skip sections of the tree to look for existing keywords if I wouldn’t want to use those terms, e.g. “downtown” is a sub-location keyword for some region-specific keywords (created using John Beardsworths Search/Replace/Transfer plugin to copy geolocation metadata into a keyword hierarchy). So I skip the LOCATIONS section of my term tree. I also skip my people, and some long lists of plant and animal species since Clarifai isn’t yet smart enough to send those terms, anyway (so ignoring these very long sections of my term tree makes the plugin’s processes more efficient).

3) If two or more keywords have the same name, I provide checkboxes for both. Keywords with parents (all of mine), display a tooltip with the full “ancestry” if you hover over the checkbox or keyword.

4) I auto-select terms that are existing in my keyword list (as the safx version originally supported), but I now also support modifying that behavior to only select terms where Clarifai has set a high probability (the level of probability required is configurable in the settings panel).

And there are a few other small improvements I have in mind. But the additional features in your implementation also really sound cool, so I’ll have to see if we can bring the best features into one project.

Thanks again for your quick response,

Lowell

On Oct 13, 2016, at 2:47 AM, Mike C notifications@github.com wrote:

@LowellMontgomery https://github.com/LowellMontgomery I gave up on trying to put together a fix. I ended up going a different direction...

I created my own Lightroom export plugin that leverages the Clarifai API for tagging. It uses the export process instead of a menu item to initiate the tagging and has quite a few extra features. Features like choosing the model/detection to use, more resizing options, auto-tagging, API usage stats and more. It should work out of the box on Windows / Mac.

Project page: https://github.com/mcrosson/lr_plugin_computer_vision_tagging https://github.com/mcrosson/lr_plugin_computer_vision_tagging Features/Changelog: https://github.com/mcrosson/lr_plugin_computer_vision_tagging/blob/master/docs/ProjectPlan.md https://github.com/mcrosson/lr_plugin_computer_vision_tagging/blob/master/docs/ProjectPlan.md — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/safx/LightroomPlugin-ClarifaiTagger/issues/1#issuecomment-253380228, or mute the thread https://github.com/notifications/unsubscribe-auth/AImZNBmTA_t033BIsaasd6vY0zVz1cQFks5qzX-bgaJpZM4IwDSY.

mcrosson commented 8 years ago

@LowellMontgomery The export process is just the first step in the work flow, it's how I send images to Clarifai (or others hopefully). I still show a tagging dialog and ok/cancel button after the export runs. The real difference between the two projects is how you kick off the process (menu item here, export in mine).

I saw your pull request with some of the new features and was hoping to see if you'd be interested in changing gears a bit. All 4 of your points are awesome ideas and I'd appreciate any help getting them integrated into the plugin I put together. If you're interested in hacking on my plugin a bit, please let me know. I think we can work together to get everything you're suggesting into the project.

To speak to each point directly: 1) I saw this in your pull request and it would definitely be something I'd want to see implemented. I currently don't have this functionality built. 2) This sounds like a very useful addition to point 1. I currently don't have this functionality built. 3) This sounds like a really good addition to 1 and 2. I'm not sure how I'd put it together but if 1 and 2 end up added, I think this functionality would be a huge gain. 4) My plugin will auto-select existing terms and has an option to auto-apply tags with a minimum p value. However, auto-selecting tags with a minimum p value should be a pretty straight forward change and I'd be willing to add it as additional functionality.

LowellMontgomery commented 8 years ago

@mcrosson I'd be happy to help bring those features (1-4) into your project and brainstorm with you about other possible improvements. I think it's best we have regular communication so we can prioritize and be "on the same page" with regard to feature development. I'd rather not spend lots of time developing features that you aren't interested in seeing merged into the master and end up with a permanent fork that (at some point) wouldn't be merge-friendly. If you or safx weren't open to those features, I guess I'd be looking at creating a third project (a permanent fork) to deal with these things that I consider high priorities.

Having looked over your feature list and project screenshots, my interest is definitely piqued. I'll add a fork and create a feature branch for the above points and I'll see how simple it is to implement them... then we can also discuss some other interesting features that your plan doesn't (yet) include.

Anyway, since we are now getting far "off-topic" for this "issue ticket", I'd like to continue this conversation via private email and/or tickets on your project.

mcrosson commented 8 years ago

@LowellMontgomery Agreed, I can be reached via the e-mail in my github profile or feel free to open tickets on the project page.