PTPObjectInfo related improvements #1039
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The (Object) Handle member is not part of the "ObjectInfo dataset" in the specification but added here to avoid having to carry it around by separate means throughout the code base, as it is regularly required in combination. While the spec always refers to this item as "ObjectHandle", the term "Handle" is only used in this context, so we omit the "Object"-prefix for brevity. Switching from oid/ObjectID, which is nowere to be found the spec, increases consistency with the spec and the rest of the code base. This allows to remove the struct _PTPCanon_New_Object from the codebase. @msmeissn I added 2 TODO comments for you take a close look, pointing out: * a memory leak that I did not want to touch because I can't test it * a presumed bug fix where newobject was 0 when passed to ptp_getobject
See last commit for reasoning of choosing this terminology. TLDR: consistency spec / other API
This should better be fixed once and for all by replacing the heap allocated `char* Filename` with a statically sized buffer. If only I knew the 'correct' size for that. See also here: petabyt/camlib@20fb75a#r147761481
This adds a set of macros implementing a generic array or list of TYPE. This is basically a TYPE* pointer and a length integer. This structure together with the typical use-cases repeats regularly throughout the codebase. It raises the level of abstraction and improves code readabilty. I'm a c++ developer and miss my STL ;). I'm starting here with PTPObjectHandles and PTPStorageIDs as those were already modeling an array abstraction. There are other arrays of stuff that can benefit from this later.
Looks like they changed the build image...
* apparently reallocarray() is not supported * the compiler seems to have an issue with a goto label position
|
macOS CI is failing 🔴 |
Thanks, I was aware... ;) |
msmeissn
approved these changes
Oct 10, 2024
| @@ -440,14 +439,17 @@ camera_prepare_canon_eos_capture(Camera *camera, GPContext *context) { | |||
| ptp_free_deviceinfo (¶ms->deviceinfo); | |||
| C_PTP (ptp_getdeviceinfo(params, ¶ms->deviceinfo)); | |||
| CR (fixup_cached_deviceinfo (camera, ¶ms->deviceinfo)); | |||
|
|
|||
| /* TODO: the following block of code seems to have no effect, remove? */ | |||
There was a problem hiding this comment.
i think we can comment it out.
it mirrors what the Canon EOS is doing, but its likely not going to break anything to disable it.
| return GP_ERROR; | ||
| GP_LOG_D ("object has OFC 0x%x", oi.ObjectFormat); | ||
|
|
||
| if (oi.StorageID) /* all done above */ | ||
| if (oi.StorageID) /* all done above (TODO: the meaning of this comment / code is unclear) */ |
There was a problem hiding this comment.
afaik with StoageID set we have "to card" capture and can now exit.
the code below is the "to SDRAM" capture and synthesizes a virtual filename.
| @@ -6956,6 +6940,8 @@ camera_wait_for_event (Camera *camera, int timeout, | |||
| free (ob->oi.Filename); | |||
| C_MEM (ob->oi.Filename = strdup (path->name)); | |||
| strcpy (path->folder,"/"); | |||
| /* TODO: @msmeissn the following goto will leak the `path` memory and | |||
There was a problem hiding this comment.
this came from commit 46adf5b refactoring. I need to check and rewrite this a bit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details see commit messages. Please note the two TODOs mentioned in the first commit.