Danne and I have been adding as many cameras as possible in his bleeding edge crop_rec module. One problem we came up against is with Movie crop mode which is used in some of his settings.
#include "crop-mode-hack.h"
...
/* always disable Movie crop mode if using crop_rec presets, except for mcm mode */
if ((crop_preset == CROP_PRESET_mcm_mv1080_EOSM) || (crop_preset == CROP_PRESET_anamorphic_rewired_EOSM) || (crop_preset == CROP_PRESET_anamorphic_rewired_100D))
{
movie_crop_hack_enable();
}
else
{
if (is_EOSM || is_EOSM2 || is_100D) movie_crop_hack_disable();
}
This works fine for all cameras that include the Movie crop mode feature but the module will not compile on cameras that should work with crop_rec but don't have this feature.
Will load on:
100D, 650D, 700D, EOSM, EOSM2
Will NOT load on:
1100D (movie_crop_hack_enable, fps_override_shutter_blanking, movie_crop_hack_disable)
5D3.113 (movie_crop_hack_enable, movie_crop_hack_disable)
5D3.123 (movie_crop_hack_enable, movie_crop_hack_disable)
600D (movie_crop_hack_enable, fps_override_shutter_blanking, movie_crop_hack_disable)
60D (movie_crop_hack_enable, fps_override_shutter_blanking, movie_crop_hack_disable)
6D (movie_crop_hack_enable, movie_crop_hack_disable)
70D (movie_crop_hack_enable, movie_crop_hack_disable)
Not checked (compile ML for these cameras first):
500D, 50D, 550D, 5D2, 7D
Deleting that code block allows all cameras that are supported by crop_rec to compile but of course turning Movie crop mode on and off won't work.
Will load on:
100D, 5D3.113, 5D3.123, 650D, 6D, 700D, 70D, EOSM, EOSM2
Will NOT load on:
1100D (fps_override_shutter_blanking)
600D (fps_override_shutter_blanking)
60D (fps_override_shutter_blanking)
Not checked (compile ML for these cameras first):
500D, 50D, 550D, 5D2, 7D
(It would be great to add the 5D2 to this but we're waiting for the code to be published.)
I thought the solution would be to use a preprocessor conditional on that section of code. A problem is that the module wouldn't be portable but since it is usually compiled for a specific platform that didn't seem like a show stopper for now:
#if defined(CONFIG_EOSM) || defined(CONFIG_100D)
...
#endif
However, we found out that this wasn't working, the code block apparently was being skipped on all cameras.
So the question is, what is the correct way to write code for a feature that is specific to some cameras but not others?
One way would be to check whether a certain symbol (function or variable) is available. For example, in modules/bench/mem_bench.c, the following should be self-explaining:
/* optional routines */
extern WEAK_FUNC(ret_0) void* dma_memcpy(void* dest, void* srce, size_t n);
extern WEAK_FUNC(ret_0) void* edmac_memcpy(void* dest, void* srce, size_t n);
extern WEAK_FUNC(ret_0) void* edmac_copy_rectangle_adv(void* dst, void* src, int src_width, int src_x, int src_y, int dst_width, int dst_x, int dst_y, int w, int h);
#define HAS_DMA_MEMCPY ((void*)&dma_memcpy != (void*)&ret_0)
#define HAS_EDMAC_MEMCPY ((void*)&edmac_memcpy != (void*)&ret_0)
That means, for a function declared as WEAK_FUNC(ret_0):
- if it's available (either in ML code or some other loaded module), it will be called
- if it's not available, ret_0 (i.e. a function that does nothing and returns 0) will be called instead
- to find out whether the function is available or not, compare its address (function pointer) against ret_0
In your case, the best candidate would be is_crop_hack_supported(). Or, simply declare movie_crop_hack_enable/disable as WEAK_FUNC(ret_0); in that case, calling any of these on an unsupported camera would have no effect.
Thanks!
This should save the next EOSM from the brickyard.
Quote from: dfort on May 13, 2019, 08:34:56 AM
Thanks!
This should save the next EOSM from the brickyard.
8)
Has there been any progress on this module?
Sure -- it is being used in Danne's bleeding edge branch:
Instead of this:
modules/crop_rec.c
#include <crop-mode-hack.h>
It is now this:
modules/crop_rec.c
extern WEAK_FUNC(ret_0) unsigned int is_crop_hack_supported();
extern WEAK_FUNC(ret_0) unsigned int movie_crop_hack_enable();
extern WEAK_FUNC(ret_0) unsigned int movie_crop_hack_disable();
Now this is compiling for all cameras whether or not the Movie crop mode feature is enabled.
modules/crop_rec.c
/* always disable Movie crop mode if using crop_rec presets, except for mcm mode, Only eosm and 100D */
if (is_EOSM || is_100D)
{
/* always disable Movie crop mode if using crop_rec presets, except for mcm mode */
if ((crop_preset == CROP_PRESET_mcm_mv1080_EOSM) || (crop_preset == CROP_PRESET_anamorphic_rewired_EOSM) || (crop_preset == CROP_PRESET_anamorphic_rewired_100D))
{
if (is_EOSM || is_100D) movie_crop_hack_enable();
}
else
{
if (is_EOSM || is_100D) movie_crop_hack_disable();
}
}
It used to not compile on the 6D and 5D3 because is_crop_hack_supported() is not defined on those cameras.