issues with open and fopen while recording video

srsa_4c

4451

Re: issues with open() and fopen() while recording video

« Reply #20 on: 10 / June / 2014, 15:24:59 »

Advertisements

Quote from: philmoz on 09 / June / 2014, 18:18:19

Could you combine 'is_video_recording()' with a call to 'TakeSemaphore()' that has a very short timeout? If the TakeSemaphore fails then it might be a better indication that movie recording is actually in progress.

I'm thinking about leaving 'is_video_recording()' out of all this. 'fileio_semaphore' seems to be a good indication of an ongoing movie recording. I think I have found its equivalent on VxWorks (in attachment) too, it's called "FileAccessSem" in the ixus30 fw.
We can't take this semaphore during a call to Fopen_Fut, because all _Fut functions appear to 'take' it.
So, taking care of open() seems to be possible (also in the attachment). Question is, what else do we want to protect.
- Fopen_Fut: we could 'take' and 'give' the semaphore right before the call, but doing that does not seem too professional nor is it 100% reliable. We could also switch to 'Fopen_FileStream' and protect it the same way as open().
- Other file system related functions: ?

edit:
link added

fileio_semaphore_for_vxworks.diff (46.86 kB - downloaded 15 times.)

« Last Edit: 10 / June / 2014, 17:26:53 by srsa_4c »

Logged

reyalp

14034

Re: issues with open() and fopen() while recording video

« Reply #21 on: 07 / September / 2014, 19:28:21 »

Spent a little more time looking at this, it seems ugly no matter what

Quote from: srsa_4c on 10 / June / 2014, 15:24:59

I'm thinking about leaving 'is_video_recording()' out of all this. 'fileio_semaphore' seems to be a good indication of an ongoing movie recording.

Or other long running file operation, if there are any, but I guess that should probably be handled the same. If the startup image indexing triggered an IO error that could be a problem, but I'd expect that to be lots of individual opens/closes.

Video playback may be similar, but if so it should probably be treated the same way as recording.

Quote

- Fopen_Fut: we could 'take' and 'give' the semaphore right before the call, but doing that does not seem too professional nor is it 100% reliable.

Definitely not nice, but for this case it might not be too bad. You only have a problem if a movie starts recording between the "test" can the actual Fopen call. That should be rare. It could happen in a script that tries to do logging and video, but given that it would cause an IO error if it worked correctly, the script would have to be written differently anyway. So not great, but maybe not a terrible option if we don't have something better.

Quote

We could also switch to 'Fopen_FileStream' and protect it the same way as open().

Not sure what implications of this are. We would presumably have to identify the equivalents of all the other Fut functions.

Quote

- Other file system related functions: ?

Presumably, all the ones that have the fileio_semaphore protection now and all of the _Fut io functions. I verified on a540 that opening a file in lua, starting movie record, and then trying to write causes the same sort of hang.

This raises the possibility that open will work, but reads, writes or the subsequent close will fail. The last would be particularly bad, since no one expects close to fail and the file handle would be left open. But as above, maybe this isn't a big problem in practice since since starting / stopping video is pretty rare. In any case, a complete lockup isn't better.

An alternative might be to forcefully release the semaphore from whatever movie task it's used in, but presumably they lock out all IO for a reason. I haven't looked to see if it's anywhere in the movie code we already hook. IIRC there are some cameras that let you take stills in video (causing a momentary break in the video) It might be worth looking at what they do.

« Last Edit: 07 / September / 2014, 19:58:36 by reyalp »

Logged

Don't forget what the H stands for.

srsa_4c

4451

Re: issues with open() and fopen() while recording video

« Reply #22 on: 08 / September / 2014, 18:41:52 »

Quote from: reyalp on 07 / September / 2014, 19:28:21

Quote
We could also switch to 'Fopen_FileStream' and protect it the same way as open().
Not sure what implications of this are. We would presumably have to identify the equivalents of all the other Fut functions.

This would be similar to the _Open() -> _open() replacement we used to do (fsionotify would be skipped on file opening). However, doing that may not be "healthy", so I'm not saying we should do this. In case I have not mentioned yet, you can find many (but not all) related function names in the "rescue" firmware of ixus30/40 (starts at 0xfff00000). The _Fut functions used today are still quite similar, Fopen_Fut calls Fopen_FileStream by passing it its input arguments.

Quote

Quote
- Other file system related functions: ?
Presumably, all the ones that have the fileio_semaphore protection now and all of the _Fut io functions. I verified on a540 that opening a file in lua, starting movie record, and then trying to write causes the same sort of hang.

This raises the possibility that open will work, but reads, writes or the subsequent close will fail. The last would be particularly bad, since no one expects close to fail and the file handle would be left open.

Should we show a warning on UI or log something to the console when a file operation did not succeed?

Quote

An alternative might be to forcefully release the semaphore from whatever movie task it's used in, but presumably they lock out all IO for a reason.

Since I don't think their OS is incapable of maintaining multiple "disk" files at once, I rather think it's related to performance (and reduces possible additional fragmentation of card space).

Quote

IIRC there are some cameras that let you take stills in video (causing a momentary break in the video) It might be worth looking at what they do.

That involves at least 2 fw tasks, but possibly more

Logged

philmoz

3450

Re: issues with open() and fopen() while recording video

« Reply #23 on: 02 / November / 2014, 20:21:54 »

Quote from: srsa_4c on 10 / June / 2014, 15:24:59

I'm thinking about leaving 'is_video_recording()' out of all this. 'fileio_semaphore' seems to be a good indication of an ongoing movie recording. I think I have found its equivalent on VxWorks (in attachment) too, it's called "FileAccessSem" in the ixus30 fw.
We can't take this semaphore during a call to Fopen_Fut, because all _Fut functions appear to 'take' it.
So, taking care of open() seems to be possible (also in the attachment). Question is, what else do we want to protect.
- Fopen_Fut: we could 'take' and 'give' the semaphore right before the call, but doing that does not seem too professional nor is it 100% reliable. We could also switch to 'Fopen_FileStream' and protect it the same way as open().
- Other file system related functions: ?

edit:
link added

How confident are you that the VxWorks fileio_semaphore values are correct?

Seems like this is worth adding to SVN if the semaphore values are good.

Also, why only protect the _Open call with the semaphore for VxWorks? _Close and _Write are also protected for DryOS (or is this just due to the age of the patch).

Edit: I've replaced some of the fopen/fclose code with open/close, with the patch from srsa_4c, this seems to eliminate most of the lockups. ~~It might not be necessary to do anything about the fopen/fclose calls~~ Lua uses fopen/fclose - running a script could still lockup if it tries to do file IO.

Update patch attached that works against current trunk - note, this includes some diagnostic code to capture and display various statistics for open/close/write.

Edit2: Updated patch to include semaphore timeout check in opendir and closedir. This prevents lockup if you try and save the edge overlay while recording a video

As I understand it the current state (with this patch) is:
- 1.2
- file IO using open/close available while recording video, can change and run scripts, load modules, etc.
- file IO using fopen/fclose while recording video will lockup the camera until video stops or battery pulled.
- risk of FSIoNotify assert shutting down camera at random times.
- 1.3
- file IO using open/close not available while recording video, cannot change script, load new modules, save settings, etc.
- file IO using fopen/fclose while recording video will lockup the camera until video stops or battery pulled.
- FSIoNotofy crash eliminated.

Phil.

fileio_v3.diff (47.83 kB - downloaded 13 times.)

« Last Edit: 03 / November / 2014, 18:07:09 by philmoz »

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

srsa_4c

4451

Re: issues with open() and fopen() while recording video

« Reply #24 on: 03 / November / 2014, 19:32:16 »

Quote from: philmoz on 02 / November / 2014, 20:21:54

How confident are you that the VxWorks fileio_semaphore values are correct?

I have taken a look at some old, newer and new Vx firmware and the file semaphore init routine (must be part of their FileSem library) always seemed the same (1st check is for "FileAccessSem", I'm assuming they did not move lines in their source around). The routines following the init function are "StartFileAccess_FileSem" and "EndFileAccess_FileSem". They have wrappers which are used in most _Fut functions. We/I could perhaps add the file access semaphore init function to the sigfinder's list and use a script (I do have such a script) to mass-disassemble its content for visual inspection.

Quote

Also, why only protect the _Open call with the semaphore for VxWorks? _Close and _Write are also protected for DryOS (or is this just due to the age of the patch).

I either wasn't paying attention or because it was good enough for basic testing.

Quote

- file IO using fopen/fclose while recording video will lockup the camera until video stops or battery pulled.

We could perhaps add the above mentioned imperfect protection to fopen, so it has less chance to lock the camera?

Logged

philmoz

3450

Re: issues with open() and fopen() while recording video

« Reply #25 on: 03 / November / 2014, 19:49:45 »

Quote from: srsa_4c on 03 / November / 2014, 19:32:16

Quote from: philmoz on 02 / November / 2014, 20:21:54
How confident are you that the VxWorks fileio_semaphore values are correct?
I have taken a look at some old, newer and new Vx firmware and the file semaphore init routine (must be part of their FileSem library) always seemed the same (1st check is for "FileAccessSem", I'm assuming they did not move lines in their source around). The routines following the init function are "StartFileAccess_FileSem" and "EndFileAccess_FileSem". They have wrappers which are used in most _Fut functions. We/I could perhaps add the file access semaphore init function to the sigfinder's list and use a script (I do have such a script) to mass-disassemble its content for visual inspection.

Quote
Also, why only protect the _Open call with the semaphore for VxWorks? _Close and _Write are also protected for DryOS (or is this just due to the age of the patch).
I either wasn't paying attention or because it was good enough for basic testing.

Quote
- file IO using fopen/fclose while recording video will lockup the camera until video stops or battery pulled.
We could perhaps add the above mentioned imperfect protection to fopen, so it has less chance to lock the camera?

I've added the detection and stub entries to SVN.

Will play with protecting fopen and see what happens (should not be necessary to protect fclose).

It might also be worth converting existing code from fopen/fclose to open/close where possible.

Phil.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

reyalp

14034

Re: issues with open() and fopen() while recording video

« Reply #26 on: 03 / November / 2014, 23:05:28 »

Thanks Phil, this looks like a big improvement.

Quote from: philmoz on 03 / November / 2014, 19:49:45

It might also be worth converting existing code from fopen/fclose to open/close where possible.

The Lua IO code uses fopen/fclose because that's what the standard Lua code uses. AFAIK other users are pretty minimal at this point.

The Lua stuff could probably be converted, but it would involve changing a lot of upstream code. An alternative would be to roll our own minimal clone of fopen etc based open/close.

Firmware fopen handles have a 32k buffer attached to them, so a minimal conversion to open/close might affect scripts that do a lot of small IOs.

Logged

Don't forget what the H stands for.

philmoz

3450

Re: issues with open() and fopen() while recording video

« Reply #27 on: 04 / November / 2014, 02:27:30 »

Quote from: reyalp on 03 / November / 2014, 23:05:28

Thanks Phil, this looks like a big improvement.
Quote from: philmoz on 03 / November / 2014, 19:49:45
It might also be worth converting existing code from fopen/fclose to open/close where possible.
The Lua IO code uses fopen/fclose because that's what the standard Lua code uses. AFAIK other users are pretty minimal at this point.

The Lua stuff could probably be converted, but it would involve changing a lot of upstream code. An alternative would be to roll our own minimal clone of fopen etc based open/close.

Firmware fopen handles have a 32k buffer attached to them, so a minimal conversion to open/close might affect scripts that do a lot of small IOs.

Update patch with semaphore check in fopen. Patch still has diagnostic code.
Seems to work ok, for example if you try and run llibtst.lua while a video is recording it gets an error, rather than locking up.

We might need to add the check to fclose, fread, fwrite etc - if a script opened a file, then started video recording and finally tried to write or close the file while recording it could still lockup.

The current patch seems to cover most cases, so not sure how far we need to take this (for now).

Phil.

fileio_v4.diff (5.68 kB - downloaded 16 times.)

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

philmoz

3450

Re: issues with open() and fopen() while recording video

« Reply #28 on: 04 / November / 2014, 06:05:11 »

Another thought occurred to me on this.

At the moment (with the current patch), 1.3 should be free of FsIoNotify crashes, and mostly free of lockups while recording video.

The downside is functions that work in 1.2 when recording video will now fail (loading a script, or module, saving settings etc).

What about a user selectable option to allow 'potentially unsafe' IO operations to continue?
If the option is enabled, then instead of failing when TakeSemaphore times out we just revert to the old functionality and call the underlying firmware function without the semaphore protection.

There would be minimal risk of triggering the FsIoNotify function since the firmware is already inside an IO loop (for the video recording), it should not be opening and closing other files.

This would only apply for open/close/write/opendir/closedir, doing it in fopen would trigger a lockup.

I had a quick play with this and it seems to work - the main difference to the 1.2 functionality is that things are slower (due to the 1/2 second timeout in TakeSemaphore). 1/2 second is probably too conservative , I tried it at 200ms and it still seems to work.

Phil.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

srsa_4c

4451

Re: issues with open() and fopen() while recording video

« Reply #29 on: 04 / November / 2014, 17:20:27 »

Quote from: philmoz on 04 / November / 2014, 06:05:11

What about a user selectable option to allow 'potentially unsafe' IO operations to continue?
If the option is enabled, then instead of failing when TakeSemaphore times out we just revert to the old functionality and call the underlying firmware function without the semaphore protection.

Sounds good to me. Perhaps there could be a camera-specific #define to disable this choice on cameras that are known to lock.

- If we allow file operations to fail, perhaps we could give some information about the failures (on the console?).
- We could also create a queue for file descriptors that failed to close and try to close them periodically.

(The above suggestions might fall into the "overdoing it" category...)

Logged