Suggestion for revision of the current 'camera.h' system. - page 2 - General Discussion and Assistance - CHDK Forum  
« previous next »
Pages: 1 [2] 3   Go Down

Suggestion for revision of the current 'camera.h' system.

  • 26 Replies
  • 7014 Views
*

Offline msl

  • *****
  • 1273
  • A720 IS, SX220 HS 1.01a
    • CHDK-DE links
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #10 on: 07 / April / 2011, 06:52:50 »
Advertisements
The CHDK shell has also a nice cross reference tool to analyze camera.h. So we can find unnecessary items.

I'm not a friend of splitted camera.h. A well-organized file is sufficient. It is also important to use a uniform format. I've started to organize this file something for CHDK-DE. This could be also a basis.

But I let me also convince them that a splitted file is better.

msl
Logged
CHDK-DE:  CHDK-DE links
*

Offline reyalp

  • ******
  • 12864
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #11 on: 07 / April / 2011, 12:51:04 »
Quote from: msl on 07 / April / 2011, 06:52:50
The CHDK shell has also a nice cross reference tool to analyze camera.h. So we can find unnecessary items.

I'm not a friend of splitted camera.h. A well-organized file is sufficient.
I thought this at first too, but I think two things make split file better
1) easier to compare, you can use normal diff tools
2) easier to auto-generate other stuff, like wiki tables or lua.

Quote
It is also important to use a uniform format.
I agree with this, and with a split file there will more tendency for people making ports to put other stuff that doesn't belong in there. Maintainers will have to work against this, but making the files easier to compare will help.
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3225
    • Photos
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #12 on: 07 / April / 2011, 18:42:02 »
Quote from: msl on 07 / April / 2011, 06:52:50
The CHDK shell has also a nice cross reference tool to analyze camera.h. So we can find unnecessary items.
Thanks for pointing that out - it's a very cool feature. Perhaps this could be used as a basis for automatically documenting the options for the wiki.

Quote
I'm not a friend of splitted camera.h. A well-organized file is sufficient. It is also important to use a uniform format. I've started to organize this file something for CHDK-DE. This could be also a basis.
I'm trying to understand what everyone believes are the pros and cons of this change.

Do you have any specific concerns or issues with splitting the file?

While re-organizing the file will help, the very fact that it needs to be re-organized is a sign that there is a problem.

In my experience coding this way is a poor design practice - having common stuff in the main file and specific stuff in localized files is much easier to work with and maintain. I don't think anyone would ever consider merging all the camera 'boot.c' files into one file and using '#if / #else' to compile the right code; but this is what has happened with camera.h.

Finally the perl script I posted earlier adds some comments to each platform_camera.h file and to the re-written camera.h file. The comments I added are shown below, and I think these will help people understand and use the system. The script can be easily changed to generate different / explained / clearer comment if anyone has any views on this.

camera.h comments:
Code: [Select]
// camera.h

// This file contains the default values for various settings that may change across camera models.
// Setting values specific to each camera model can be found in the platform/XXX/platform_camera.h file for camera.

// If adding a new settings value put a suitable default value in here, along with documentation on
// what the setting does and how to determine the correct value.
// If the setting should not have a default value then add it here using the '#undef' directive
// along with appropriate documentation.

#ifndef CAMERA_H
#define CAMERA_H
...

platform_camera.h comments:
Code: [Select]
// Camera - G12 - platform_camera.h

// This file contains the various settings values specific to the G12 camera.
// This file is referenced via the 'include/camera.h' file and should not be loaded directly.

// If adding a new settings value put a suitable default in 'include/camera.h',
// along with documentation on what the setting does and how to determine the correct value.
// If the setting should not have a default value then add it in 'include/camera.h'
// using the '#undef' directive along with appropriate documentation.

// Override any default values with your camera specific values in this file. Try and avoid
// having override values that are the same as the default value.

// When overriding a setting value there are two cases:
// 1. If removing the value, because it does not apply to your camera, use the '#undef' directive.
// 2. If changing the value it is best to use an '#undef' directive to remove the default value
//    followed by a '#define' to set the new value.
...

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)
*

Offline msl

  • *****
  • 1273
  • A720 IS, SX220 HS 1.01a
    • CHDK-DE links
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #13 on: 08 / April / 2011, 04:36:09 »
Quote from: philmoz on 07 / April / 2011, 18:42:02
Do you have any specific concerns or issues with splitting the file?

I think, for programmers is the splitting a good thing. For non programmers like me it is difficult to get the overview. I write the German CHDK user manual (ca. 20,000 downloads). For this is camera.h a very important file. I can find many informations, e.g. all cameras with nd filter. Now I can use any text editor has a search function.

With splitting the file we also split the informations. That's why we need an easy to use system to collect  informations for a group of cameras.

msl
Logged
CHDK-DE:  CHDK-DE links
*

Offline philmoz

  • *****
  • 3225
    • Photos
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #14 on: 08 / April / 2011, 05:10:32 »
Quote from: msl on 08 / April / 2011, 04:36:09
Quote from: philmoz on 07 / April / 2011, 18:42:02
Do you have any specific concerns or issues with splitting the file?

I think, for programmers is the splitting a good thing. For non programmers like me it is difficult to get the overview. I write the German CHDK user manual (ca. 20,000 downloads). For this is camera.h a very important file. I can find many informations, e.g. all cameras with nd filter. Now I can use any text editor has a search function.

With splitting the file we also split the informations. That's why we need an easy to use system to collect  informations for a group of cameras.

msl

Thanks for that - excellent point.

So if the change included automatic generation of a document that contained all the information in one place (like CHDK-Shell does) that would satisfy your requirements?

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)
*

Offline msl

  • *****
  • 1273
  • A720 IS, SX220 HS 1.01a
    • CHDK-DE links
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #15 on: 08 / April / 2011, 05:29:16 »
Quote from: philmoz on 08 / April / 2011, 05:10:32
So if the change included automatic generation of a document that contained all the information in one place (like CHDK-Shell does) that would satisfy your requirements?

Yes of course. This would be a wonderful thing. Thx for your tireless commitment.

msl
Logged
CHDK-DE:  CHDK-DE links
*

Offline Microfunguy

  • ****
  • 4037
    • StereoData Maker
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #16 on: 08 / April / 2011, 10:06:02 »
Nothing to do with me but I will ask out of interest ..

If CHDK and CHDK-DE are so similar, why do both have to exist ?
Logged
*

Offline msl

  • *****
  • 1273
  • A720 IS, SX220 HS 1.01a
    • CHDK-DE links
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #17 on: 08 / April / 2011, 10:47:36 »
Quote from: Microfunguy on 08 / April / 2011, 10:06:02
If CHDK and CHDK-DE are so similar, why do both have to exist ?

You can compare both source codes (available as SVN) und you see the differences.

CHDK-DE is specially designed for German users (default language a.s.o.), has an extended command set for scripts, another DOF calculator (maybe better as the old calculator), some GUI extensions and many more.

... but this is not the topic of this thread.

msl

« Last Edit: 08 / April / 2011, 11:12:48 by msl »
Logged
CHDK-DE:  CHDK-DE links
*

Offline Bernd R

  • ****
  • 430
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #18 on: 08 / April / 2011, 13:07:40 »
>I'd appreciate any feedback on this idea (both for and against).
>If there's enough support I'll start working on the conversion program and CHDK patch.

I think your idea good, maybe you can extend it, so its also possible to overwrite the palette values in this new camera.h platform file.

then merging of a port is more easy.
Logged
Ixus 1000 HS
*

Offline whim

  • ******
  • 2034
  • A495/590/620/630 ixus70/115/220/230/300/870 S95
Re: Suggestion for revision of the current 'camera.h' system.
« Reply #19 on: 09 / April / 2011, 05:29:52 »
Hi Phil,

Sorry for late reaction, but msl already seemed to have read my thoughts on this  :D

Tested your script, and it seems to do the job very nicely.

A suggestion: include commented-out 'default section' in the header of platform-camera.h.

AFAIK most 'porters' will start by copying a reference camera, and this will assure
they get to see all the available defines.  This implies, of course, that the
default section contains all of them, preferably with description ...

(extract from camprops_xref.log for CHDK trunk 1132)
Quote
......
Properties NOT INCLUDED, because MISSING in Default Values:  * ASPECT_GAMES_XCORRECTION * ASPECT_GAMES_XCORRECTION(x) * ASPECT_GAMES_YCORRECTION * ASPECT_GAMES_YCORRECTION(y) * ASPECT_GRID_XCORRECTION * ASPECT_GRID_XCORRECTION(x) * ASPECT_GRID_YCORRECTION * ASPECT_GRID_YCORRECTION(y) * CAM_ACTIVE_AREA_X1 * CAM_ACTIVE_AREA_X2 * CAM_ACTIVE_AREA_Y1 * CAM_ACTIVE_AREA_Y2 * cam_CalibrationIlluminant1 * cam_CFAPattern * CAM_COLORMATRIX1 * CAM_JPEG_HEIGHT * CAM_JPEG_WIDTH * CAM_STARTUP_CRASH_FILE_OPEN_FIX * DNG_EXT_FROM * DNG_EXT_TO * GAMES_SCREEN_HEIGHT * GAMES_SCREEN_WIDTH * OPT_CURVES * PARAM_CAMERA_NAME * SYNCHABLE_REMOTE_NOT_ENABLED
......

On the other hand, this would seriously increase the amount of code lines ...

I'm currently rewriting my parsing code to adapt to the new situation, and will soon issue
a small fix to prevent CHDK-Shell from falling over in case this gets implemented before
it's ready.

just my 2 cents,

wim

« Last Edit: 09 / April / 2011, 05:39:51 by whim »
Logged
Pages: 1 [2] 3   Go Up
« previous next »
 

Related Topics