Using stubs vs. Hardcoding stuff into modules

[dpjpandone]

I can give some advice.

Firstly, modules are built outside of the context of any individual cam.  This is because modules are supposed to be portable: people can copy them from one card to another in a different cam and they're supposed to work.  TCC code on the cam handles loading modules and looking up symbols by name.  This is the way that modules find stubs.  If a symbol doesn't exist, the user will see an error and the module won't load (but ML won't crash, it's more like a warning than a real error).

If you define something in features.h for a cam, I think it won't be visible to the module during the build.  Even if it is, it won't work properly if you copy the module and use it with a different cam, so this isn't a correct approach.

I was suggesting that *things which are functions* could be moved to stubs.  I haven't looked at the variables like more_hacks_are_supported, so I don't know what these do or what the best way to handle them is.

You should be able to test your ideas in qemu.  First get the existing code working and test it emulates this far (should do I think).  Then make changes and see if you broke anything.

I suppose it depends what you mean by "doing stubs wrong"   In the module code, where it's just an address, I haven't checked how it's used.  In a stub context, it will be treated as a function pointer according to the definition of NSTUB (or ARM32_FN / THUMB_FN if you use those).  Is it appropriate to treat these addresses in that way?  That depends how the module uses them.  In the exact text you gave, you put them in as comments, which will do nothing (and the symbol won't get exported, so the modules won't find it).

Probably, but that's a bigger question than you might realise.  I think the intention when ML added lots of supported models was that they'd all get all the features eventually.  Later on (I guess) as you've found, FEATURE and CONFIG limit or specify features, but that doesn't work well with modules.  Modules are *supposed* to be cam independent, but actually this was never true, there were some edge cases that became apparent when we worked more on modern cams and found that some structs and other internals had changed enough that some assumptions around how modules worked failed.

So, what's an elegant way to let modules behave the right way on a range of cams?  My personal feeling is modules could, instead of doing this:

Code: [Select]

if (is_camera("5D3", "1.2.3"))
{
    screen_x = 0x0101; screen_y = 0x0202;
}
else if { // many other cameras omitted }

Do something like this:

Code: [Select]

    get_screen_x_y(screen) // screen is a struct with .x and .y members

That is: cleanly separate modules from ML code, so they can only work via exported functions.  Modules ask the cam what the right values are for that model.  No hard-coding values inside modules.

So in your specific example, create a stub for *all* cams maybe called is_more_hacks_supported() (this is a bad name, but you get the idea).  This would probably default to returning false, and be overridden by cams that did have support.  Then the module code is greatly simplified - no per cam checks, but something like this:

Code: [Select]

    more_hacks_supported = is_more_hacks_supported();

Is this the best way to go?  That's harder to say.  It would increase the size of the ML binary, while reducing the size of modules.  Size constraints are a real concern on some cams.

you put them in as comments

didn't want those to compile yet

So in your specific example, create a stub for *all* cams maybe called is_more_hacks_supported() (this is a bad name, but you get the idea).  This would probably default to returning false, and be overridden by cams that did have support.  Then the module code is greatly simplified - no per cam checks, but something like this:

Code: [Select]

    more_hacks_supported = is_more_hacks_supported();


is this the correct syntax to do such a thing?

Code: [Select]

NSTUB(1 - is_more_hacks_supported)
and cams that do not support it the stub needs to be 0 ? Or you start by saying:

Code: [Select]

more_hacks_supported = 0; //defaults to 0
more_hacks_supported = is_more_hacks_supported() //returns value set in stub

If there is no stub in another cam's Stubs.s does it return 0 automatically?

[names_are_hard]

Code: [Select]

NSTUB(1 - is_more_hacks_supported)

This won't compile.  See the definition in include/stub.h:

Code: [Select]

#define NSTUB(addr,name) \
.global name; \
.extern name; \
name = addr

If you instead did NSTUB(0, some_name), you would be saying "I want magiclantern to be aware there is an external symbol, named some_name, at address 0".  I'm not sure it has a type at this stage, and without one I'm not sure what happens if you try to use it.  It's C, so you can cast it to force it to do...  something.  If you try to read or write to it, on digic 4 and 5 cams it might succeed or it might crash.  On digic 7 and 8 cams it will definitely crash.  Digic 6 I'm not sure about, probably a crash.

You don't want to use stubs for this part.  Stubs tell ML where things are in Canon code.  Presumably, for simple variables, you want to locate them in ML code, making stubs unnecessary.

In my example I shouldn't have called is_more_hacks_supported() a stub, it's just an exported function (I call it both by mistake).

[dpjpandone]

Code: [Select]

* Types of NSTUB values and how to find them:
 * 0xFFnnnnnn - ROM address. Go to address in old ROM then find the matching ARM assembly code in new ROM to find the new value.
 * 0xnnnnn    - Data address. Search the operands column for =0xnnnnn then find the same region in new ROM to find the new value.
 * 0x000nnnnn - System routine. Search operands column for sub_0nnnnn then find the same region in new ROM to find the new value (do these ever change?).
means these are ROM adresses:

Code: [Select]

NSTUB(0xFF16E318,  lvfaceEnd)
NSTUB(0xFF23FF10,  aewbSuspend)
NSTUB(0xFF181340, CartridgeCancel)

Code: [Select]

#ifndef __STUB_H__
#define __STUB_H__

#define NSTUB(addr,name) \
.global name; \
.extern name; \
name = addr

#endif /* __STUB_H__ */

where does this go? The Stubs files in platform\Stubs.s looks like these:

Code: [Select]

NSTUB(0xFF136F68, _audio_ic_write)                          // str:Reg_0x_02X_Data_04x)
this ^ actually has a comment that might give a clue as to what/how info this can receive
or

Code: [Select]

NSTUB(0xFFA0321C - RAM_OFFSET,  AbortEDmac)
this^ is a different format, does the format itself tell me how/what to send?

or

Code: [Select]

NSTUB(0xFF137168,  SetAudioVolumeOut)
This^ I assume would accept a value how loud to set the volume, but how do I know what values I can send? and how to format said values?

then there's stuff like the ones I want to make into stubs. They just tell the camera to do a thing like "stop calculating autoexposure and white balance because" (because it's making write's slower) how should these be formatted:

Code: [Select]

0xFF16E318,  lvfaceEnd)
0xFF23FF10,  aewbSuspend)
0xFF181340, CartridgeCancel)
maybe the answers to these questions are really obvious to someone who is well-groomed in C and assembly, I've done some cool stuff with arduino making multi-axis motion control stuff for cameras/VFX work, but I usually found a well-documented library that did the lower level stuff and played around with a well-commented example to understand how it works so I can use it properly. If my questions are not ML-specific and you think I should study some other resources first so that i can ask better questions I don't want to waste your time maybe you could point me toward some preliminary study material that would help me become a better contributor to this community?

[names_are_hard]

Most of those are questions about C, yes.  I don't know good current resources to learn C, I learned too long ago   maybe try the r/learnprogramming FAQ.

That comment about NSTUB values is not really true.  It's a hint, not a rule.  It's saying those ranges of addresses *suggest* a possible meaning.  The real meaning is not to do with the address range, it's arbitrary, whatever the specific ROM does.

All NSTUB does is define an external symbol in the ML binary.  It tells you nothing about the purpose or meaning.  It purely maps a symbol name to an address.  Using your _audio_ic_write example, the *meaning* of the symbol name is defined in src/audio.h:

Code: [Select]

126 extern void
127 _audio_ic_write(
128         unsigned                cmd
129 );

We've decided to treat that symbol as a function.  Almost all stubs are functions (maybe all?).  But this is by convention, the NSTUB macro doesn't declare them functions.

[kitor]

Almost all stubs are functions (maybe all?)

Not all, there are some data pointers (vram info, etc)