CHDK PTP interface

srsa_4c

4451

Re: CHDK PTP interface

« Reply #840 on: 03 / September / 2012, 19:18:51 »

Advertisements

Quote from: reyalp on 03 / September / 2012, 17:58:24

Most of the stuff add to generic/capt_seq.c should be in thumb code somewhere.
It might make more sense to do most of the stuff in capt_seq_hook_raw_here() within the regular raw hook (or the same spytask loop where the raw hook is called)
get_next_chunk_data_for_ptp also looks like it doesn't need to be in capt_seq, but not sure where it goes.

I added the code to generic/capt_seq.c because that seemed to be somewhat related. Since the thumb-arm barrier in CHDK is not entirely clear to me, is there something I should watch out for when moving functions into thumb code?
edit:
Answer: platform/generic/shooting.c

« Last Edit: 03 / September / 2012, 20:01:14 by srsa_4c »

Logged

reyalp

14080

Re: CHDK PTP interface

« Reply #841 on: 03 / September / 2012, 20:12:44 »

Quote from: srsa_4c on 03 / September / 2012, 19:18:51

Quote from: reyalp on 03 / September / 2012, 17:58:24
Most of the stuff add to generic/capt_seq.c should be in thumb code somewhere.
It might make more sense to do most of the stuff in capt_seq_hook_raw_here() within the regular raw hook (or the same spytask loop where the raw hook is called)
get_next_chunk_data_for_ptp also looks like it doesn't need to be in capt_seq, but not sure where it goes.
I added the code to generic/capt_seq.c because that seemed to be somewhat related. Since the thumb-arm barrier in CHDK is not entirely clear to me, is there something I should watch out for when moving functions into thumb code?

The main point is thumb code is smaller, so anything that's not tightly tied to the ARM asm could should usually be in thumb. If you are using a __attribute__((naked)) function to save and restore registers, that should be in arm.

edit:
note if you use ASM_SAFE in the call (or don't need to save extra registers), then you can go directly to the thumb function, e.g. core_spytask_can_start is a thumb function and most dryos cams call it with just
"BL core_spytask_can_start" in boot.c, and the toolchain converts it to core_spytask_can_start_from_arm

« Last Edit: 03 / September / 2012, 20:22:20 by reyalp »

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: CHDK PTP interface

« Reply #842 on: 03 / September / 2012, 21:05:50 »

Quote from: reyalp on 03 / September / 2012, 20:12:44

note if you use ASM_SAFE in the call (or don't need to save extra registers), then you can go directly to the thumb function, e.g. core_spytask_can_start is a thumb function and most dryos cams call it with just
"BL core_spytask_can_start" in boot.c, and the toolchain converts it to core_spytask_can_start_from_arm

I think that's what I wanted to know.

Something (not) completely unrelated:
How should I handle this situation? Two of the earliest cameras are affected (I plan to release my Ixus30/SD200 port soon). send_resp() has only 2 parameters in every other camera. Would it be better to create a new #define in camera.h for this, or can I just do what I did here? That would cause a 2 or 4 byte size increase in every port, but - as I understand - would be safe otherwise.

Logged

reyalp

14080

Re: CHDK PTP interface

« Reply #843 on: 03 / September / 2012, 21:31:27 »

Quote from: srsa_4c on 03 / September / 2012, 21:05:50

Quote from: reyalp on 03 / September / 2012, 20:12:44
note if you use ASM_SAFE in the call (or don't need to save extra registers), then you can go directly to the thumb function, e.g. core_spytask_can_start is a thumb function and most dryos cams call it with just
"BL core_spytask_can_start" in boot.c, and the toolchain converts it to core_spytask_can_start_from_arm
I think that's what I wanted to know.

Something (not) completely unrelated:
How should I handle this situation? Two of the earliest cameras are affected (I plan to release my Ixus30/SD200 port soon). send_resp() has only 2 parameters in every other camera. Would it be better to create a new #define in camera.h for this, or can I just do what I did here? That would cause a 2 or 4 byte size increase in every port, but - as I understand - would be safe otherwise.

Yeah, I think sending an extra arg is fine. If I remember right we already send deta->recv_data more than (some ?) firmwares actually use.

Regarding remote capture, I was thinking about putting the init stuff in lua, but I don't want to duplicate effort.

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: CHDK PTP interface

« Reply #844 on: 03 / September / 2012, 21:56:54 »

Quote from: reyalp on 03 / September / 2012, 21:31:27

Regarding remote capture, I was thinking about putting the init stuff in lua, but I don't want to duplicate effort.

If you have time to do it, just go ahead.
How about moving most of generic/capt_seq.c into (say) core/hooks.c?

Logged

reyalp

14080

Re: CHDK PTP interface

« Reply #845 on: 03 / September / 2012, 22:11:34 »

Quote from: srsa_4c on 03 / September / 2012, 21:56:54

Quote from: reyalp on 03 / September / 2012, 21:31:27
Regarding remote capture, I was thinking about putting the init stuff in lua, but I don't want to duplicate effort.
If you have time to do it, just go ahead.
How about moving most of generic/capt_seq.c into (say) core/hooks.c?

I'd probably put the remote capture related stuff in it's own file (core/ remote_capture.c or something, some of the stuff in the main ptp.c could probably go there too). Without that, capt_seq_hook_raw_here is very small. capt_seq_hook_set_nr needs defines from the cameras capt_seq.c

None of this is urgent.

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: CHDK PTP interface

« Reply #846 on: 06 / September / 2012, 15:36:07 »

I started to move code from generic/capt_seq.c to core/remote_capture.c in a temporary repo
( https://subversion.assembla.com/svn/chdk-s1.remotecapture/ ). It's still in progress. I'm a bit unsure about the handling of affected variables. I changed some from static to volatile (to be able to access them from multiple source modules), and I created "setter" and "getter" functions for some others.
My later plan includes using the dng module to get the CHDK exif (optionally).
I'm not completely sure about naming the new file "remote_capture.c", because the filewritetask hook could possibly be used for more.
To do the separation, I had to move the declaration of the struct "cam_ptp_data_chunk" into platform_camera.h. As platform_camera.h only holds #define's in current CHDK code, this looks pretty ugly to me.
"cam_ptp_data_chunk" may turn out to be the same in a large number of camera generations, but this is only a suspicion.

Logged

reyalp

14080

Re: CHDK PTP interface

« Reply #847 on: 06 / September / 2012, 16:57:44 »

Quote from: srsa_4c on 06 / September / 2012, 15:36:07

I started to move code from generic/capt_seq.c to core/remote_capture.c in a temporary repo
( https://subversion.assembla.com/svn/chdk-s1.remotecapture/ ).

I started doing something similar when I tried to put the init stuff in Lua cleanly, ran into similar issues and ran out of time before I got to the point of having anything to check in.

Quote

I changed some from static to volatile (to be able to access them from multiple source modules), and I created "setter" and "getter" functions for some others.

I'd say getters/setters is probably cleaner, neither one is really safe to communicate across tasks, but for simple state variables it should be OK. Overall, the current code (in remote-capture-test) is quite convoluted with the different globals and statics. I think it can be made much more modular, but it's a bit tricky.

Quote

I'm not completely sure about naming the new file "remote_capture.c", because the filewritetask hook could possibly be used for more.

Agreed. The direction I was going was to have minimal filewrite_main_hook in capt_seq.c which currently just called into the remote capture code, but could have other stuff later.

Quote

To do the separation, I had to move the declaration of the struct "cam_ptp_data_chunk" into platform_camera.h. As platform_camera.h only holds #define's in current CHDK code, this looks pretty ugly to me.

"cam_ptp_data_chunk" may turn out to be the same in a large number of camera generations, but this is only a suspicion.

If there is only one known variant, it might be better to just start out assuming it's always that way, and deal with it later if it turns out not to be. If there are only a few variants, they are likely to correspond to DryOS revs (old vxworks like a410 and more recent like a540 seem to be the same), so you could just have a few different ones using existing DryOS ifdefs.

In my code, started putting in a function to copy from the platform defined one into the generic one in generic/capt_seq.c

Quote

My later plan includes using the dng module to get the CHDK exif (optionally).

I'm not sure it's even worth making this optional, if the client doesn't want it they can just throw it out.

On question does arise, if you do a sub-image, what dimensions go in the header ?

Another thing I noticed trying to work on the init stuff is that startline, numlines could have different possible values for raw vs YUV (raw buffer being a bit bigger than max size jpeg, and jpeg potentially being lower res.)

Have you found / verified that a YUV buffer is actually available ? If not, maybe V1 should just be jpeg and raw, and not worry about any of the YUV stuff for now.

Logged

Don't forget what the H stands for.

Microfunguy

4037

Re: CHDK PTP interface

« Reply #848 on: 06 / September / 2012, 17:10:46 »

Quote from: reyalp on 06 / September / 2012, 16:57:44

I'd say getters/setters is probably cleaner, neither one is really safe to communicate across tasks

Why is that ?

Logged

srsa_4c

4451

Re: CHDK PTP interface

« Reply #849 on: 06 / September / 2012, 21:43:05 »

Quote from: reyalp on 06 / September / 2012, 16:57:44

I'd say getters/setters is probably cleaner, neither one is really safe to communicate across tasks, but for simple state variables it should be OK.

What I don't like about the getters/setters is the overhead (especially when the variable is shared between arm and thumb code).

Quote

Overall, the current code (in remote-capture-test) is quite convoluted with the different globals and statics. I think it can be made much more modular, but it's a bit tricky.

I know ...

Quote from: srsa_4c on 22 / August / 2012, 21:22:34

code is ugly and imperfect (suggestions / improvements are welcome)

Quote

In my code, started putting in a function to copy from the platform defined one into the generic one in generic/capt_seq.c

How does it look like?

Quote

I'm not sure it's even worth making this optional, if the client doesn't want it they can just throw it out.
On question does arise, if you do a sub-image, what dimensions go in the header ?

I assumed that if somebody needs a subrange of the RAW, a header may not be needed anyway. Or do you think the output should be valid DNG? If so, the byteswapping should probably be done in the client.

Quote

Another thing I noticed trying to work on the init stuff is that startline, numlines could have different possible values for raw vs YUV (raw buffer being a bit bigger than max size jpeg, and jpeg potentially being lower res.)

Have you found / verified that a YUV buffer is actually available ? If not, maybe V1 should just be jpeg and raw, and not worry about any of the YUV stuff for now.

It should be available. Otherwise the camera would need to decode the jpeg again to provide the recreview. I think I'm going to dump the RAM in the FileWriteTask hook to look for the buffer, its pointers and the image dimensions. Then look them up in the firmware.

Logged