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

Already on GitHub? Sign in to your account

Fixes for bug 14119 #2287

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

These are the changes required to fix defect 14119 which I opened a few days ago. The problem was that all of the $INFO[slideshow.*] variables were blank when used in SlideShow.xml.
There were two problems.
The first problem was an incorrect condition in an if statement in GUIInfoManager.cpp. The condition was always false, so no metadata would be loaded from the picture file when going through this code path. I fixed the condition.
The second problem was that when metadata was explicitly set using PictureInfoTag.SetInfo(), it would cause all future calls to load the metadata from the picture file to be ignored. I created a new private variable to distinguish between setting metadata with SetInfo(), and setting metadata by loading it from the picture file. I used the variable to allow loading of metadata from the picture file even after some metadata had been set using SetInfo(). The metadata provided by SetInfo() is used if it does not appear in the file. If the data exists in the file, it overrides that provided by SetInfo().

The exiftime and resolution values are currently set in the same file (PictureInfoTag.cpp), way at the bottom in the SetInfo() method. There is a switch statement that only handles setting exiftime and resolution. If SetInfo is called with any value other than those two, nothing happens. I think this is to handle the case where pictures were created using a scanner rather than a camera. An addon can get the info from the file attributes, then call SetInfo to make sure the picture has at least those two useful pieces of metadata.

The way the code is written, Item.GetPictureInfoTag() will allocate a PictureInfoTag if one is not already associated for the item. All we really want here is if the metadata has not already been loaded, then load it. By adding the extra condition, it will in many cases fail because GetPictureInfoTag() has not yet been called.

I don't really understand the reason for HasPictureInfoTag(), since, whenever you call GetPictureInfoTag(), it will either return an existing PictureInfoTag, or allocate one and return it. So, item.GetPictureInfoTag()->anyFunction() will never get a null pointer exception that is caused by not having a PictureInfoTag associated with the item.

Member

jmarshallnz commented Feb 23, 2013

Not if anyFunction is const.

@ghost

ghost commented Feb 23, 2013

what jmarshall meant is not if item is const. getfootag is overloaded with a const version which does not, obviously, allocate. thus the need for hasfootag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment