CHDK PTP interface - page 86 - General Discussion and Assistance - CHDK Forum

CHDK PTP interface

  • 1241 Replies
  • 487556 Views
*

Offline reyalp

  • ******
  • 14079
Re: CHDK PTP interface
« Reply #850 on: 06 / September / 2012, 23:26:50 »
Advertisements
Why is that ?
Which one ?
I should clarify the "not safe". It's possible to use either in ways that are safe for us, but they aren't a safe way to do inter-task communication in general.
As far as using volatile vs getters/setters
- getters/setters are more flexible: If you want to do something more complicated later, the calling code doesn't have to change
- people tend to attribute magic to volatile that it doesn't have.
What I don't like about the getters/setters is the overhead (especially when the variable is shared between arm and thumb code).

I wouldn't worry too much about this, especially if it's a case like the write hook polling something waiting for another task. The polling will only happen every 10ms at most, a simple functions call is very quick. Something that only happens a few times in a ptp transaction or shot will also be no concern.
Quote
How does it look like?
Unfinished ;)
I had function like
Code: [Select]
void filewrite_get_jpeg_chunk(remotecap_data_chunk *dest,void *src,int n) {
    cam_ptp_data_chunk *pdc = src;
    dst[n].address=pdc[n].address;
    dst[n].length=pdc[n].length;
}
void filewrite_main_hook(char *name, cam_ptp_data_chunk *pdc)
{
    remotecap_jpeg_available(name,MAX_CHUNKS_FOR_JPG,pdc);
...
but after I wrote that it seemed like it might be better to just leave the loop in filewrite_main_hook.

Quote
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.
I agree byteswapping should be left to the client. For subframe, I think having the exif could be useful, but we could just send the full-frame exif and let the client adjust it or pad the missing data to the expected size.
Hmm, DNG usually has a thumbnail too.
Don't forget what the H stands for.

*

Offline srsa_4c

  • ******
  • 4451
Re: CHDK PTP interface
« Reply #851 on: 07 / September / 2012, 14:18:12 »
Quote
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.
Some preliminary trials show that the YUV source is indeed available during "recreview". Its pixel format is UYVY in A410 (and probably in every other camera too). Getting it correctly may require some scripting magic (like pressing SET to hold the cam in review mode and halfshoot after the transfer has finished) OR a long enough review time.

edit:
This could get interesting. At max. resolution I seem to get no sign of a (full sized) yuv image, only the raw. There is always a lower resolution y411 in another buffer, its size and pixel shape varies with the selected picture size. The uncompressed source yuv is always present at M and S picture sizes.

edit2:
The UYVY image seems to have only one use: source for the compression engine. Activating the record review (by pressing SET) causes immediate decoding of the JPEG to another buffer with y411 as pixel format. I see no point in getting that.
« Last Edit: 08 / September / 2012, 15:41:05 by srsa_4c »

*

Offline reyalp

  • ******
  • 14079
Re: CHDK PTP interface
« Reply #852 on: 08 / September / 2012, 17:26:06 »
The UYVY image seems to have only one use: source for the compression engine. Activating the record review (by pressing SET) causes immediate decoding of the JPEG to another buffer with y411 as pixel format. I see no point in getting that.
So if I understand right, on this camera (a410 ?) the full res YUV buffer seems to be needed only for reduced size jpeg, otherwise the image processor does the raw->jpeg all in one step ? This would match well with the observation that reduced res jpeg is slower than full res on some (mostly older, digic II maybe digic III) cameras.
I suppose a possible alternative is that the full res case does create a YUV buffer, but it's destroyed before the point you are making the dump ?

Recreview decoding doesn't surprise me, they have fast decode for play mode anyway.

As an aside, the normal viewport buffer addresses don't seem to work in recreview mode. I'm not sure if this is true for all cameras, it is for d10 and a540. These cameras both have vid_get_viewport_live_fb, it's possible the correct address is returned by one of the other existing functions (I think someone tested this and found it wasn't but I'm not 100% sure.) If you find a reliable way to get it, we could make recreview work in ptp live view, which would be nice. Showing chdk zebra/histogram in recreview could also be useful.

Quote
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'm not really sure how to coordinate work with this branch and the one main CHDK svn repo.
Don't forget what the H stands for.

*

Offline srsa_4c

  • ******
  • 4451
Re: CHDK PTP interface
« Reply #853 on: 08 / September / 2012, 22:46:25 »
So if I understand right, on this camera (a410 ?) the full res YUV buffer seems to be needed only for reduced size jpeg, otherwise the image processor does the raw->jpeg all in one step ?
It appears to be so.
Quote
I suppose a possible alternative is that the full res case does create a YUV buffer, but it's destroyed before the point you are making the dump ?
I would think that's unlikely. At least I can't see any sign of that buffer. The RAW buffer seems to still contain the RAW data, and of course there is the JPEG data.
Quote
As an aside, the normal viewport buffer addresses don't seem to work in recreview mode. I'm not sure if this is true for all cameras, it is for d10 and a540. These cameras both have vid_get_viewport_live_fb, it's possible the correct address is returned by one of the other existing functions (I think someone tested this and found it wasn't but I'm not 100% sure.) If you find a reliable way to get it, we could make recreview work in ptp live view, which would be nice. Showing chdk zebra/histogram in recreview could also be useful.
By looking at some dumps, there is an active viewport buffer (still talking about review mode right after shooting), vid_get_viewport_fb_d() points to it. In another dump, which was created after shooting AND pressing SET (recreview hold), vid_get_viewport_fb_d() still seems to be valid.

Quote
Quote
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'm not really sure how to coordinate work with this branch and the one main CHDK svn repo.

Take a look at that repo. If you find that the code I committed there should go into ptp-remote-capture-test, then I'll update that repo too. I started this because you mentioned
Quote
Regarding remote capture, I was thinking about putting the init stuff in lua, but I don't want to duplicate effort.
and I didn't want to clash with that. Moreover, my code still sucks (not optimal, needs cleanup).
BTW, I used a (full, unnecessarily) dump of the main CHDK repo (rev. 2133) for mine (I'm practicing with svn).

How should I continue with work?


*

Offline reyalp

  • ******
  • 14079
Re: CHDK PTP interface
« Reply #854 on: 08 / September / 2012, 23:12:32 »
By looking at some dumps, there is an active viewport buffer (still talking about review mode right after shooting), vid_get_viewport_fb_d() points to it. In another dump, which was created after shooting AND pressing SET (recreview hold), vid_get_viewport_fb_d() still seems to be valid.
OK, so we could check recreview in vid_get_viewport_active_buffer. I'll try that and report back.

Quote
Take a look at that repo. If you find that the code I committed there should go into ptp-remote-capture-test, then I'll update that repo too. I started this because you mentioned
Quote
Regarding remote capture, I was thinking about putting the init stuff in lua, but I don't want to duplicate effort.
and I didn't want to clash with that. Moreover, my code still sucks (not optimal, needs cleanup).
BTW, I used a (full, unnecessarily) dump of the main CHDK repo (rev. 2133) for mine (I'm practicing with svn).
Yes, I that confused me a bit. A branch in the same repo makes it easier to compare and move changes around, and doesn't really cost any space.

I got the re-work I started last weekend working (on a540 at least), in the attached patch. There's still some loose ends but I think it's going in the right direction. If it looks OK to you, I'll put it in the ptp-remote-capture-test branch. If you think your branch is a better starting point, I can live with that too.
Don't forget what the H stands for.

*

Offline reyalp

  • ******
  • 14079
Re: CHDK PTP interface
« Reply #855 on: 09 / September / 2012, 03:21:11 »
Updated patch.
- initial lua based init implementation. Untested, need to set up the client stuff.
- much simplified ptp.c code. Error returns may change slightly, but current rs command seems to work
- now compiles if camera doesn't have filewritetask
Don't forget what the H stands for.

*

Offline reyalp

  • ******
  • 14079
Re: CHDK PTP interface
« Reply #856 on: 09 / September / 2012, 03:51:53 »
By looking at some dumps, there is an active viewport buffer (still talking about review mode right after shooting), vid_get_viewport_fb_d() points to it. In another dump, which was created after shooting AND pressing SET (recreview hold), vid_get_viewport_fb_d() still seems to be valid.
OK, so we could check recreview in vid_get_viewport_active_buffer. I'll try that and report back.
on d10, after switching the recreview_hold variable to the sig finder value, this seems to work. However, it only works in *hold* mode, you'd need some other variable to detect a fixed review time like 10 sec.
Don't forget what the H stands for.

*

Offline srsa_4c

  • ******
  • 4451
Re: CHDK PTP interface
« Reply #857 on: 09 / September / 2012, 11:48:32 »
Updated patch.
- initial lua based init implementation. Untested, need to set up the client stuff.
- much simplified ptp.c code. Error returns may change slightly, but current rs command seems to work
- now compiles if camera doesn't have filewritetask
First (quick) impression: it works, the code looks much better organized now. I copied over YUV support from my version, also works, but it may have to be modified (it will produce 0 byte long files with cameras without explicit yuv support OR when L picture size is selected). I think you can commit this.
edit: there's still some cleanup to do (removed functions' declarations)
...
However, it only works in *hold* mode, you'd need some other variable to detect a fixed review time like 10 sec.
True. I haven't checked that yet.
« Last Edit: 09 / September / 2012, 14:26:22 by srsa_4c »


*

Offline reyalp

  • ******
  • 14079
Re: CHDK PTP interface
« Reply #858 on: 09 / September / 2012, 16:31:54 »
First (quick) impression: it works, the code looks much better organized now. I copied over YUV support from my version, also works, but it may have to be modified (it will produce 0 byte long files with cameras without explicit yuv support OR when L picture size is selected). I think you can commit this.
Committed in changeset 2135. Please feel free to work in this branch.
Quote
edit: there's still some cleanup to do (removed functions' declarations)
Yes, there's still quite a bit to be done.

One thing that i'm not sure about is the
Code: [Select]
state_shooting_progress=SHOOTING_PROGRESS_DONE; //shoot() needs this...
In remotecap_get_data_chunk... since this can be called when remotecap is reset (on error or init(0)) it would happen at random times in the shooting process. Same for hook_raw_save_complete. I'm not clear exactly what the implications would be, but it seems like there could be problems.

init in Lua, also introduces some more potential synchronization issues, in theory a PTP_CHDK_RemoteCaptureGetData could be running at the same time a script decides to cancel remote capture. I guess the timeout could have the same problem too.

A few other thoughts:
- should the remote capture selection only apply to the next shot, like exposure overrides, or should you need to explicitly cancel it ? If it's left on after the PTP connection goes away, that could confuse users. We don't explicitly know whether there is a PTP connection, but I guess we could check USB power is present (and clear remotecap if it's not found ?).
Setting it once per shot shouldn't be much of a burden if you are using script to shoot, but I guess you could potentially set it once and have the user manually shooting with the shutter ?
Clearing it on script termination (like key presses and script overrides) is probably not good, because you could trigger the shot and have the script end before the data is retrieved (if you use button presses at least, shoot() may wait until everything is done). Users who do want it cleared on script exit could do so in restore().
- should we actually prohibit calling init in playback mode ? Since it just sets up some flags, this doesn't actually cause any harm. You can initialize it and switch to playback mode, so it's still possible to have remotecap enabled in play.
- hook_raw_save_complete() is only called after all the chunks are downloaded, meaning the the raw hook will be blocked until the jpeg is complete. This seems like it might have unwanted impacts, perhaps depending on where the raw hook is on a specific camera.
- The current raw remote capture implementation bypasses raw_savefile() completely, but this function also does some other stuff beyond raw saving, like handling bracketing in continuous mode and shot_histogram. It might be better to remove the remote_capture stuff to raw save_file. This would mean you'd block spytask, which in turn would block capt_seq thought
- The current implementation of remotecap_get_data_chunk requires an extra ptp transaction for each file type, because you only find out you are done by requesting another chunk and getting an empty response. This probably doesn't matter much, but if we can cleanly flag "this is the last chunk" it will save a little time. The status returned by this function could also possibly be made clearer.
Don't forget what the H stands for.

*

Offline srsa_4c

  • ******
  • 4451
Re: CHDK PTP interface
« Reply #859 on: 09 / September / 2012, 16:46:29 »
Quite long post to answer, I'll update this post unless you reply.

One thing that i'm not sure about is the
Code: [Select]
state_shooting_progress=SHOOTING_PROGRESS_DONE; //shoot() needs this...
In remotecap_get_data_chunk... since this can be called when remotecap is reset (on error or init(0)) it would happen at random times in the shooting process. Same for hook_raw_save_complete. I'm not clear exactly what the implications would be, but it seems like there could be problems.
The reason for this was that when I first tried my monolithic rs command, I kept getting "script is already running" afterwards. If I remember correctly, this was needed for shoot() to finish (?). I'm going to take a look at this again. Maybe it's not needed anymore or is misplaced.
...
Quote
- hook_raw_save_complete() is only called after all the chunks are downloaded, meaning the the raw hook will be blocked until the jpeg is complete. This seems like it might have unwanted impacts, perhaps depending on where the raw hook is on a specific camera.
I assumed the two hooks to never overlap. If they do, the port is IMHO broken (imagine what would happen if FileWriteTask's open stage found an open file). That was the reason I used the same variable and status codes for the two hooks.
...
Quote
- The current implementation of remotecap_get_data_chunk requires an extra ptp transaction for each file type, because you only find out you are done by requesting another chunk and getting an empty response. This probably doesn't matter much, but if we can cleanly flag "this is the last chunk" it will save a little time. The status returned by this function could also possibly be made clearer.
Yes, the condition check could be shifted into remotecap_get_data_chunk() - it could return a value for example.
...
Quote
- should we actually prohibit calling init in playback mode ? Since it just sets up some flags, this doesn't actually cause any harm. You can initialize it and switch to playback mode, so it's still possible to have remotecap enabled in play.
This again was necessary for my version of rs, with the integrated shoot().
...
Quote
- The current raw remote capture implementation bypasses raw_savefile() completely, but this function also does some other stuff beyond raw saving, like handling bracketing in continuous mode and shot_histogram. It might be better to remove the remote_capture stuff to raw save_file. This would mean you'd block spytask, which in turn would block capt_seq thought
The reason of the bypass was exactly what you say, it would be complicated to do otherwise.
I've never tested how FileWriteTask acts in continuous mode...
...
« Last Edit: 09 / September / 2012, 17:23:42 by srsa_4c »

 

Related Topics