[CODE CLEANUP] Macros for device capabilities?

[a1ex]

What if, instead of "if defined(CONFIG_50D) || defined(CONFIG_60D)" etc, we would use some macros to describe device capabilities?

e.g.

CAP_ROLLING_PITCHING // 60D, 7D, 5D3
CAP_DISPLAY_BUFFER_OVERRIDE // 5D2, 550D and newer
CAP_LCD_SENSOR // 550D, 500D
CAP_ASIF_DMA // all except 50D and VxWorks
CAP_EXMEM
CAP_PTP
...

[1%]

Don't some cameras still have different addresses/methods for the same feature?

[a1ex]

Sometimes they do. In many cases, these differences can be handled by some camera-specific wrappers.

I'm looking at the roll/pitch sensor - currently it's enabled for 60D and 7D, and it can be enabled for 7D too. The code is a bit scattered (there are 3 or 4 ifdef's for this feature), and it's easy to miss one of them when porting. With a CAP_ROLLING_PITCHING macro, enabling this feature for 5D3 would require enabling this macro, rather than looking for ifdefs that might be related to this feature.

Example: https://bitbucket.org/hudson/magic-lantern/changeset/6b186470005f

Sometimes it gets really tricky. The image format in DryOS cameras is YUV422, and in VxWorks ones is some sort of YUV411, and sometimes the code differences are quite significant. In this case I've created a separate file (zebra.c for DryOS and zebra-5dc for 5Dc, which should also be used for the 40D and other VxWorks ports).

[1%]

Makes sense. People could try features instead of randomly looking for ifdefs.

[nanomad]

I second this (I actually tried something similar once but failed horribly )

[jplxpto]

...
Sometimes it gets really tricky. The image format in DryOS cameras is YUV422, and in VxWorks ones is some sort of YUV411, and sometimes the code differences are quite significant. In this case I've created a separate file (zebra.c for DryOS and zebra-5dc for 5Dc, which should also be used for the 40D and other VxWorks ports).

I confess, I was a little discouraged when I looked at the zebras.c code
Any change to simplify that code will be welcomed

[a1ex]

Here's what I'm working on:

Code Select Expand


/** Audio menu **/

    #define FEATURE_ANALOG_GAIN
    #define FEATURE_DIGITAL_GAIN
    #define FEATURE_AGC_TOGGLE
    #define FEATURE_WIND_FILTER
    #define FEATURE_INPUT_SOURCE
    #define FEATURE_MIC_POWER
    #define FEATURE_HEADPHONE_MONITORING
    #define FEATURE_HEADPHONE_OUTPUT_VOLUME
    #define FEATURE_AUDIO_METERS
    #define FEATURE_BEEP
    #define FEATURE_WAV_RECORDING
    #define FEATURE_VOICE_TAGS

/** Expo menu **/

    #define FEATURE_WHITE_BALANCE
    #define FEATURE_EXPO_ISO
    #define FEATURE_EXPO_ISO_HTP
    #define FEATURE_EXPO_ISO_DIGIC
    #define FEATURE_EXPO_SHUTTER
    #define FEATURE_EXPO_APERTURE
    //~ #define FEATURE_INTERMEDIATE_ISO_PHOTO_DISPLAY

    #define FEATURE_PICSTYLE
    #define FEATURE_REC_PICSTYLE

    #define FEATURE_EXPO_LOCK
    #define FEATURE_EXPO_PRESET
    #define FEATURE_ML_AUTO_ISO
   
    #define FEATURE_EXPSIM
    #define FEATURE_EXPO_OVERRIDE

/** Overlay menu **/

    #define FEATURE_GLOBAL_DRAW
    #define FEATURE_ZEBRA
    #define FEATURE_ZEBRA_FAST
    #define FEATURE_FOCUS_PEAK
    #define FEATURE_FOCUS_PEAK_DISP_FILTER
    #define FEATURE_MAGIC_ZOOM
    #define FEATURE_CROPMARKS
    #define FEATURE_GHOST_IMAGE
    #define FEATURE_SPOTMETER
    #define FEATURE_FALSE_COLOR
    #define FEATURE_HISTOGRAM
    #define FEATURE_WAVEFORM
    #define FEATURE_VECTORSCOPE
   
    #define FEATURE_OVERLAYS_IN_PLAYBACK_MODE

/** Movie menu **/
    #define FEATURE_NITRATE
    #define FEATURE_NITRATE_WAV_RECORD

    #define FEATURE_REC_INDICATOR
    #define FEATURE_MOVIE_LOGGING
    #define FEATURE_MOVIE_RESTART
    #define FEATURE_MOVIE_AUTOSTOP_RECORDING
    #define FEATURE_REC_NOTIFY
    #define FEATURE_MOVIE_REC_KEY
    #define FEATURE_FORCE_LIVEVIEW
    #define FEATURE_SHUTTER_LOCK
    #define FEATURE_GRADUAL_EXPOSURE
   
    #define FEATURE_FPS_OVERRIDE
    #define FEATURE_FPS_RAMPING
    #define FEATURE_FPS_WAV_RECORD
   
    #define FEATURE_HDR_VIDEO
    #define FEATURE_MOVIE_RECORDING_50D
    #define FEATURE_MOVIE_RECORDING_50D_SHUTTER_HACK
    #define FEATURE_LVAE_EXPO_LOCK
    #define FEATURE_IMAGE_EFFECTS

/** Shoot menu **/

    #define FEATURE_HDR_BRACKETING
    #define FEATURE_INTERVALOMETER
    //~ #define FEATURE_LCD_SENSOR_REMOTE
    #define FEATURE_AUDIO_REMOTE_SHOT
    #define FEATURE_MOTION_DETECT
    #define FEATURE_SILENT_PIC
    #define FEATURE_SILENT_PIC_HIRES
    //~ #define FEATURE_SILENT_PIC_JPG

    #define FEATURE_MLU
    #define FEATURE_MLU_HANDHELD
    //~ #define FEATURE_MLU_HANDHELD_DEBUG
    //~ #define FEATURE_MLU_DIRECT_PRINT_SHORTCUT

    #define FEATURE_FLASH_TWEAKS
    //~ #define FEATURE_LV_3RD_PARTY_FLASH
   
    //~ #define FEATURE_PICQ_DANGEROUS

/** Focus menu **/
    #define FEATURE_TRAP_FOCUS
    #define FEATURE_FOLLOW_FOCUS
    #define FEATURE_RACK_FOCUS
    #define FEATURE_FOCUS_STACKING
    #define FEATURE_AF_PATTERNS
    //~ #define FEATURE_MOVIE_AF

/** Display menu **/
    #define FEATURE_LV_BRIGHTNESS
    #define FEATURE_LV_CONTRAST
    #define FEATURE_LV_SATURATION
    #define FEATURE_LV_DISPLAY_GAIN
    #define FEATURE_COLOR_SCHEME
    #define FEATURE_CLEAR_OVERLAYS
    #define FEATURE_DEFISHING_PREVIEW
    #define FEATURE_ANAMORPHIC_PREVIEW
    #define FEATURE_LEVEL_INDICATOR
    #define FEATURE_SCREEN_LAYOUT
    #define FEATURE_IMAGE_POSITION
    #define FEATURE_UPSIDE_DOWN
    #define FEATURE_IMAGE_ORIENTATION
    #define FEATURE_AUTO_MIRRORING_HACK
    #define FEATURE_FORCE_HDMI_VGA
    #define FEATURE_UNIWB_CORRECTION

/** Prefs menu **/

    #define FEATURE_SET_MAINDIAL
    #define FEATURE_PLAY_EXPOSURE_FUSION
    #define FEATURE_PLAY_COMPARE_IMAGES
    #define FEATURE_PLAY_TIMELAPSE
    #define FEATURE_PLAY_EXPOSURE_ADJUST
    #define FEATURE_PLAY_422
   
    #define FEATURE_IMAGE_REVIEW_PLAY
    #define FEATURE_QUICK_ZOOM
    #define FEATURE_LV_BUTTON_PROTECT
    #define FEATURE_LV_BUTTON_RATE
    #define FEATURE_QUICK_ERASE
   
    #define FEATURE_LV_ZOOM_SETTINGS
    //~ #define FEATURE_ZOOM_TRICK_5D3
   
    #define FEATURE_LV_FOCUS_BOX_FAST
    #define FEATURE_LV_FOCUS_BOX_SNAP
    #define FEATURE_LV_FOCUS_BOX_AUTOHIDE
   
    #define FEATURE_ARROW_SHORTCUTS
   
    #define FEATURE_STICKY_DOF
    #define FEATURE_STICKY_HALFSHUTTER
    #define FEATURE_SWAP_MENU_ERASE
   
    #define FEATURE_WARNINGS_FOR_BAD_SETTINGS
   
    #define FEATURE_POWERSAVE_LIVEVIEW
   
    #define FEATURE_CONFIG_SAVE
   
    #define FEATURE_LV_DISPLAY_PRESETS
   
    #define FEATURE_CROP_FACTOR_DISPLAY

    #define FEATURE_EYEFI  // EyeFi tricks confirmed working only on 600D-60D

/** Debug menu **/
   
    #define FEATURE_SHOW_OVERLAY_FPS
   
    #define FEATURE_DIGIC_POKE

    #define FEATURE_SCREENSHOT
    #define FEATURE_SCREENSHOT_422

    #define FEATURE_DONT_CLICK_ME
   
    #define FEATURE_STABILITY_TESTS
    #define FEATURE_BENCHMARKS
   
    #define FEATURE_SHOW_TASKS
    #define FEATURE_SHOW_CPU_USAGE
   
    #define FEATURE_SHOW_IMAGE_BUFFERS_INFO
    #define FEATURE_SHOW_FREE_MEMORY
    #define FEATURE_SHOW_SHUTTER_COUNT
    #define FEATURE_SHOW_CMOS_TEMPERATURE
   
    #define FEATURE_SHOW_BATTERY_INFO
   
    #define FEATURE_SNAP_SIM


[ilguercio]

It looks cool.
BTW:nitrate?

[a1ex]

Yep, seems correct according to this http://nofilmschool.com/2012/09/magic-lantern-for-the-canon-5d-mark-iii-coming-this-week/#comments

and also to this: http://en.wikipedia.org/wiki/Nat_%28information%29

[jplxpto]

Here's what I'm working on:

Code Select Expand


/** Audio menu **/

    #define FEATURE_ANALOG_GAIN
    #define FEATURE_DIGITAL_GAIN
    #define FEATURE_AGC_TOGGLE
    #define FEATURE_WIND_FILTER
    #define FEATURE_INPUT_SOURCE
    #define FEATURE_MIC_POWER
    #define FEATURE_HEADPHONE_MONITORING
    #define FEATURE_HEADPHONE_OUTPUT_VOLUME
    #define FEATURE_AUDIO_METERS
    #define FEATURE_BEEP
    #define FEATURE_WAV_RECORDING
    #define FEATURE_VOICE_TAGS

/** Expo menu **/

    #define FEATURE_WHITE_BALANCE
    #define FEATURE_EXPO_ISO
    #define FEATURE_EXPO_ISO_HTP
    #define FEATURE_EXPO_ISO_DIGIC
    #define FEATURE_EXPO_SHUTTER
    #define FEATURE_EXPO_APERTURE
    //~ #define FEATURE_INTERMEDIATE_ISO_PHOTO_DISPLAY

    #define FEATURE_PICSTYLE
    #define FEATURE_REC_PICSTYLE

    #define FEATURE_EXPO_LOCK
    #define FEATURE_EXPO_PRESET
    #define FEATURE_ML_AUTO_ISO
   
    #define FEATURE_EXPSIM
    #define FEATURE_EXPO_OVERRIDE

/** Overlay menu **/

    #define FEATURE_GLOBAL_DRAW
    #define FEATURE_ZEBRA
    #define FEATURE_ZEBRA_FAST
    #define FEATURE_FOCUS_PEAK
    #define FEATURE_FOCUS_PEAK_DISP_FILTER
    #define FEATURE_MAGIC_ZOOM
    #define FEATURE_CROPMARKS
    #define FEATURE_GHOST_IMAGE
    #define FEATURE_SPOTMETER
    #define FEATURE_FALSE_COLOR
    #define FEATURE_HISTOGRAM
    #define FEATURE_WAVEFORM
    #define FEATURE_VECTORSCOPE
   
    #define FEATURE_OVERLAYS_IN_PLAYBACK_MODE

/** Movie menu **/
    #define FEATURE_NITRATE
    #define FEATURE_NITRATE_WAV_RECORD

    #define FEATURE_REC_INDICATOR
    #define FEATURE_MOVIE_LOGGING
    #define FEATURE_MOVIE_RESTART
    #define FEATURE_MOVIE_AUTOSTOP_RECORDING
    #define FEATURE_REC_NOTIFY
    #define FEATURE_MOVIE_REC_KEY
    #define FEATURE_FORCE_LIVEVIEW
    #define FEATURE_SHUTTER_LOCK
    #define FEATURE_GRADUAL_EXPOSURE
   
    #define FEATURE_FPS_OVERRIDE
    #define FEATURE_FPS_RAMPING
    #define FEATURE_FPS_WAV_RECORD
   
    #define FEATURE_HDR_VIDEO
    #define FEATURE_MOVIE_RECORDING_50D
    #define FEATURE_MOVIE_RECORDING_50D_SHUTTER_HACK
    #define FEATURE_LVAE_EXPO_LOCK
    #define FEATURE_IMAGE_EFFECTS

/** Shoot menu **/

    #define FEATURE_HDR_BRACKETING
    #define FEATURE_INTERVALOMETER
    //~ #define FEATURE_LCD_SENSOR_REMOTE
    #define FEATURE_AUDIO_REMOTE_SHOT
    #define FEATURE_MOTION_DETECT
    #define FEATURE_SILENT_PIC
    #define FEATURE_SILENT_PIC_HIRES
    //~ #define FEATURE_SILENT_PIC_JPG

    #define FEATURE_MLU
    #define FEATURE_MLU_HANDHELD
    //~ #define FEATURE_MLU_HANDHELD_DEBUG
    //~ #define FEATURE_MLU_DIRECT_PRINT_SHORTCUT

    #define FEATURE_FLASH_TWEAKS
    //~ #define FEATURE_LV_3RD_PARTY_FLASH
   
    //~ #define FEATURE_PICQ_DANGEROUS

/** Focus menu **/
    #define FEATURE_TRAP_FOCUS
    #define FEATURE_FOLLOW_FOCUS
    #define FEATURE_RACK_FOCUS
    #define FEATURE_FOCUS_STACKING
    #define FEATURE_AF_PATTERNS
    //~ #define FEATURE_MOVIE_AF

/** Display menu **/
    #define FEATURE_LV_BRIGHTNESS
    #define FEATURE_LV_CONTRAST
    #define FEATURE_LV_SATURATION
    #define FEATURE_LV_DISPLAY_GAIN
    #define FEATURE_COLOR_SCHEME
    #define FEATURE_CLEAR_OVERLAYS
    #define FEATURE_DEFISHING_PREVIEW
    #define FEATURE_ANAMORPHIC_PREVIEW
    #define FEATURE_LEVEL_INDICATOR
    #define FEATURE_SCREEN_LAYOUT
    #define FEATURE_IMAGE_POSITION
    #define FEATURE_UPSIDE_DOWN
    #define FEATURE_IMAGE_ORIENTATION
    #define FEATURE_AUTO_MIRRORING_HACK
    #define FEATURE_FORCE_HDMI_VGA
    #define FEATURE_UNIWB_CORRECTION

/** Prefs menu **/

    #define FEATURE_SET_MAINDIAL
    #define FEATURE_PLAY_EXPOSURE_FUSION
    #define FEATURE_PLAY_COMPARE_IMAGES
    #define FEATURE_PLAY_TIMELAPSE
    #define FEATURE_PLAY_EXPOSURE_ADJUST
    #define FEATURE_PLAY_422
   
    #define FEATURE_IMAGE_REVIEW_PLAY
    #define FEATURE_QUICK_ZOOM
    #define FEATURE_LV_BUTTON_PROTECT
    #define FEATURE_LV_BUTTON_RATE
    #define FEATURE_QUICK_ERASE
   
    #define FEATURE_LV_ZOOM_SETTINGS
    //~ #define FEATURE_ZOOM_TRICK_5D3
   
    #define FEATURE_LV_FOCUS_BOX_FAST
    #define FEATURE_LV_FOCUS_BOX_SNAP
    #define FEATURE_LV_FOCUS_BOX_AUTOHIDE
   
    #define FEATURE_ARROW_SHORTCUTS
   
    #define FEATURE_STICKY_DOF
    #define FEATURE_STICKY_HALFSHUTTER
    #define FEATURE_SWAP_MENU_ERASE
   
    #define FEATURE_WARNINGS_FOR_BAD_SETTINGS
   
    #define FEATURE_POWERSAVE_LIVEVIEW
   
    #define FEATURE_CONFIG_SAVE
   
    #define FEATURE_LV_DISPLAY_PRESETS
   
    #define FEATURE_CROP_FACTOR_DISPLAY

    #define FEATURE_EYEFI  // EyeFi tricks confirmed working only on 600D-60D

/** Debug menu **/
   
    #define FEATURE_SHOW_OVERLAY_FPS
   
    #define FEATURE_DIGIC_POKE

    #define FEATURE_SCREENSHOT
    #define FEATURE_SCREENSHOT_422

    #define FEATURE_DONT_CLICK_ME
   
    #define FEATURE_STABILITY_TESTS
    #define FEATURE_BENCHMARKS
   
    #define FEATURE_SHOW_TASKS
    #define FEATURE_SHOW_CPU_USAGE
   
    #define FEATURE_SHOW_IMAGE_BUFFERS_INFO
    #define FEATURE_SHOW_FREE_MEMORY
    #define FEATURE_SHOW_SHUTTER_COUNT
    #define FEATURE_SHOW_CMOS_TEMPERATURE
   
    #define FEATURE_SHOW_BATTERY_INFO
   
    #define FEATURE_SNAP_SIM


Alex,

Thank you...

I'm glad to know that you're working on it.
I think these changes can simplify our lives.
The new ports should be simpler ... every new feature stable we can add more definition to a specific model of camera.

I think that all specific definitions should be placed in the directory ./platform /<model>.<version>/
All constants: addresses, events, properties, etc ...

In my view, the properties should also undergo some changes. Some I have already spoken to you.

[a1ex]

Here's the big change: https://bitbucket.org/hudson/magic-lantern/changeset/bda4b3a3fef2

Main points of interest:
- config-defines.h: first thing included, before all other headers
- 550D internals.h: what internals we have on each camera (e.g. 4:3 or 3:2 screen, DryOs or VxWorks etc)
- all_features.h: feature list (what works on most cameras)
- 550D features.h: features for a camera with a stable ML port - it includes all_features.h and then only lists exceptions
- 40D features.h: features for a camera with a very early ML port - tiny feature set; at the time of writing, ML for 40D has a single feature: AF patterns.

Now, there are a couple of things to solve, where I need your help:

- I did not really test it. I've ran ML on 60D with a single feature (intervalometer), then with another single feature (bracketing), then with a very small feature set - a few configurations like these.

- It compiles on all cameras, but I've only tried on 60D - seems to work.

- I may have forgotten some feature on some camera, or I may have added something that doesn't work or that doesn't make sense on that camera.

- It will be a very good idea to take every single feature and compile ML only with that feature (to make sure they were isolated well).

I hope this will make new ports a lot easier.

[1%]

For some reason the ML menu doesn't come up for me anymore. I hit erase and nothing happens.

*Fixed, it was something in debug.c

* Found another problem. Now magic zoom flickers with vsync enabled or not.

[jplxpto]

...
I hope this will make new ports a lot easier.

Alex, thank you!

I think this work will make the code more readable and easy control of features of each camera.
New ports were easier to integrate.

I think the new developers will find it easier to understand the code and seek a features.

[scrax]

that's great for me, fantastic

[jplxpto]

Sometimes they do. In many cases, these differences can be handled by some camera-specific wrappers.

I'm looking at the roll/pitch sensor - currently it's enabled for 60D and 7D, and it can be enabled for 7D too. The code is a bit scattered (there are 3 or 4 ifdef's for this feature), and it's easy to miss one of them when porting. With a CAP_ROLLING_PITCHING macro, enabling this feature for 5D3 would require enabling this macro, rather than looking for ifdefs that might be related to this feature.

I think this will help us a lot.

[jplxpto]

Sometimes it gets really tricky. The image format in DryOS cameras is YUV422, and in VxWorks ones is some sort of YUV411, and sometimes the code differences are quite significant. In this case I've created a separate file (zebra.c for DryOS and zebra-5dc for 5Dc, which should also be used for the 40D and other VxWorks ports).

I personally did not like this option. I understand the need but, your option is that there is much redundant code.
I think, there should be 3 files of zebras (zebras.c, zebras-dryos.c/zebras-y422.c & zebras-vxworks.c/zebras-411.c).
The first should have all the common code and other code specific to each type.
Thus, to be corrected a bug, or an improvement is made common to both types, both will benefit from the work.

[nanomad]

Agreed, like it has been done (sort of) for the audio code

[a1ex]

* Found another problem. Now magic zoom flickers with vsync enabled or not.

It was broken for one month, without anyone noticing.

You will have to change the vsync method. Before, it happened to work because of some other bug which was causing high CPU usage and was altering the timings.

[1%]

Used to just be able to enable vsync lite and use 60D stuff but not any more.

I found NSTUB(0xFF0FA824, str:[EVF]_evfReadOutDoneInterrupt_frames_continuou) but not sure how this works.

[a1ex]

I did it like this:
- first try to send the vsync signal from all states/inputs (just call lv_vsync_signal without any if). It should be flicker-free, but with too high CPU usage (MZ will refresh more than once per frame).
- then, narrow down (the idea is to refresh MZ only once per LV frame). In 60D, the evfReadOutDoneInterrupt transition happened to work.
- to see where these numbers come from, look here: http://www.a1ex.bitbucket.io/ML/states

So, the signal should be send only once per LV frame, and with the right timing offset (so MZ is drawn after the EDMAC fills one frame, but before that frame is sent to display device).

On 60D, now it's quite usable even in 50p.

[jplxpto]

The LEDs are on all cameras and is the first interface to be used in any port for that,
I think that their control must be independent files. The associated constants must be defined in the directory specific to each camera.