Messages

Probably not.  Feel free to try it and see.

This is a slightly strange set of symptoms.  What happens if you start the cam with no card?

That's a shame, academics will nearly always share their code.  At least you tried

There's still enough info in the paper to attempt to use the technique, though obviously that's more work.

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 Expand


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.

I used it because I wanted a github solution from official hg repo which include all history beside it's a repo that well tested (on DIGIC 5), I didn't want to work on "magiclantern_simplified" because it has a lot of changes, and it still not well tested on DIGIC 5, I just didn't want to have some random or hidden surprises and spend time on it . . this thing is temporarily, my final goal is to port everything back in one branch and repo like magiclantern_simplified of course.

Working on DIGIC 4 and give them crop_rec support is one of my goals, but it's not the highest priority for me currently . .

Makes sense.  I wasn't saying it was wrong of you to use it, just that I didn't know your reasons   It will make your future pain around porting higher, of course.

I don't require crop_rec to give Digic 4 cams crop_rec ability in order to take that code.  I require those cams to be no worse than before.  If they simply do nothing with crop_rec stuff that's fine by me.  If we can restrict them with a feature flag or similar, that's acceptable.  Is it possible to define what work needs to be done to make this branch possible to merge?  Defined so that it is possible to know how to test if the conditions are met.  What currently happens on digic 4 and 5 cams?  What problems exist?

I'd really like to integrate the code that is getting used.  I have multiple reasons:
- it's cool code that enables cool features! 
- having one repo that everyone can use is better than 6 repos and everyone has to wonder which is the right one for their camera, etc
- if people are using old cams with my repo I have more confidence I haven't broken old cams while adding support for new ones

The last point has some caveats: if people are using my repo on old cams, I'll want some more tests first!  It's had some limited testing on old cams and no new problems or bugs reported so far.

[lvfaceEnd]

Yes, I think all of them return value, what should I do in this case?

Also it seems lvfaceEnd and aewbSuspend take values, but not quite sure, but might be decompilation error in Ghidra, how to make sure?
I used "(void *)" blindly, I saw a1ex used it in sd_uhs for SD_ReConfiguration function, I just reused it and didn't notice problems . .

Here's the definition in that case:

Code Select Expand


static int (*SD_ReConfiguration)() = 0;


That is, SD_ReConfiguration is a function pointer to a function that takes no arguments (I think?  I didn't check what the implied arguments are if you leave that empty), and returns an int.  It is initialised to untyped 0, which is probably not ideal (does it give a compile error?).

Taking values is okay, the concern is potential conflict between the calling convention you declare (perhaps implicitly) in ML code, and the calling convention used by the external function.  ARM has a standard calling convention: https://stackoverflow.com/questions/261419/what-registers-to-save-in-the-arm-c-calling-convention.  Compilers will normally generate code according to those rules, but they don't have to, and it is possible to override the compiler with e.g. pragmas.  Certainly, some Canon code doesn't use this convention.

Let's imagine you have some generated ML code that wants to call a Canon function you've named "do_stuff()", and you've told ML uses standard ARM calling convention.  This means the compiler will generate assembly which assumes the following rules are true (and other rules, this is a specific example):

r4-r8 are callee-save registers
r9 might be a callee-save register or not (on some variants of AAPCS it is a special register)
r10-r11 are callee-save registers

"Callee-save" means the caller expects those registers will not be changed by the function call.  This why if you look at the disassembly in most cases you'll see them being saved at the start of a function and restored at the end.

Therefore, the compiler will assume that the values in some variables, etc, will never be changed before and after the call.  But if the Canon function is unusual, those can change.  If you don't tell ML this is one of those functions, this kind of thing can happen:

Code Select Expand


some_magiclantern_variable = 0;  // the compiler *might* choose to put this in r4
some_canon_function(); // in C, this function cannot change your variable...
if (some_magiclantern_variable != 0)
    // this should never happen, let's crash!
    assert(-1);  // but in asm, it certainly can, by changing the content of r4 and returning, and you assert here


In that code you'd expect the assert to never trigger.  But it can if conditions are right.  There are other edge cases.  Some functions don't return.  Some functions expect the return address is in register you're passing to it.  Some expect they can return to your saved return address.  This doesn't hurt you very often, but if it does it's really confusing.

So, you need to make sure that what you tell ML about Canon functions is correct.  Most Canon functions use standard ARM calling convention, which should work with the defaults.  Some do not and you should confirm this before using them.  You can then instruct the compiler to do the right thing, e.g. in dryos.h:

Code Select Expand


extern void __attribute__((noreturn)) cstart(void);


Code Select Expand


NSTUB(1 - is_more_hacks_supported)


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

Code Select Expand


#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).

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

more_hacks_are_supported is basically just to hide the option from the menu in cams that don't support.  Is there a more elegant way to accomplish this?

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 Expand


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 Expand


    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 Expand


    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.

I can't check it thoroughly because I don't have a 5D3 and I don't know what precise code you're working from.  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.

My repo is based on unified and has merged at least these branches: qemu, digic6-dumper, lua_fix.  Later, I split qemu out into its own repo.

I actively don't want to keep all of the branches, it was impossible to understand what they were for.  Code that is good should be integrated, code that is not used should be removed.

If there are branches which are useful, I can try to integrate them, but as noted, I don't have the ability to test on digic 4 or 5 cams.  I don't want to break any cams.  Some things can be tested in Qemu, some things can't.  I want to integrate good code, but I won't take code that breaks things.  I will put in a lot of work to fix things if I can see it's worthwhile (lua_fix merge was about 40k lines of diff and took several days and a lot of testing).

Also, somebody would need to tell me what the useful branches are, and what they're for.  There are literally hundreds of open branches on Heptapod.

Nobody should be using magiclantern_hg_02 and I don't know why Bilal is working from that repo   I made that one strictly as an archive, it keeps all the history of the hg repo.  I will never take changes back into https://github.com/reticulatedpines/magiclantern_hg_02

This is where I do work: https://github.com/reticulatedpines/magiclantern_simplified

That repo, however, was made for Digic 6, 7, 8 work.  Because I didn't understand the branch model for official ML (still don't!), I took the branches I needed, since there were several hundred open branches.  That didn't include crop_rec.  I later added crop_rec experimentally as a branch, but I believe it needs work before it can be integrated with the main code.  Bilal tells me it causes problems with Digic 4 cams.  I want my repo to only have one long-lived branch.  I think nobody really wants all these forks and special builds from different places, it's confusing.

So, I think somebody needs to fix the problems crop_rec branch has on digic 4 cams, but I don't have any of those cams to test on.

We can add the addresses like this commit, under:

Code Select Expand

    if (is_camera("5D3", "1.1.3"))
    {
            lvfaceEnd  = (void *) 0xFF16D77C;
            aewbSuspend = (void *) 0xFF23BC60;
            CartridgeCancel = (void *) 0xFF17FD68;
    }

Are these addresses function pointers?  If so, you shouldn't really cast to void *.  This will hide the compiler warning without fixing the problem.

How many params do these functions take?  Do they return a value?  If they don't meet standard ARM calling conventions this could corrupt program state and maybe cause crashes.

These would likely be better as stubs.  That way you could remove the per cam code from the module, and just call the stubs - same name for all cams.

It would be cool if someone can give it a try and share his results, but it definitely worth using openmp. Here my WIP changes: https://github.com/anibarro/MLV-App
Just read your post, you are right, I thought it was going to be more complicated!

I can run some comparisons, is there some CLI way to start MLVApp and run a task?  Would be nice for doing a reproducible test.  If not, please describe how to run a test (what options to enable, sample file to use, etc).  I guess I want to compare your 8e1b6d89 against 95d8d20a?

I think CUDA is no option at all. In comparison there are just a few computers able to run this, while e.g. OpenCL works on nearly all computers out of the box. So if we would implement GPU features, I'd try again with OpenCL. And there is already a CUDA based MLV tool: fastcinemadng. I was never able to try it - no CUDA GPU in my near.

Right, if you did want to use it you'd certainly need it to detect at runtime and be optional.  Which is more complexity.  OpenMP is nice and general.

@iaburn: if you know how to use the GPU... I have absolutely no idea here. I just heard it is impossible with C/C++, because it has its own kernel language. So this means mostly rewriting everything.
Yes, in the past we tried to use the GPU for debayering. In the end we had success with it technically, but it was slower as with CPU, because the copy actions between graphic RAM and main RAM needed nearly the same time as if the CPU debayers. So GPU might be cool if you process all on it - single tasks are probably for nothing.

I've only tinkered with GPU programming a little, so don't take my advice very seriously here.  You're right that GPU programming is a different way of thinking, but, if you work in CUDA, the language you use is very C-like:
https://cuda-tutorial.readthedocs.io/en/latest/tutorials/tutorial01/

Note that CUDA is Nvidia only.  But it's a good interface and is a large part of why Nvidia dominate GPGPU.

Yes, bandwidth to the GPU is often a bottleneck.  But, you only need to copy once for all GPU cores to see the data.  So, you want to divide the problem up, have each core debayer a portion of the image, then combine.  Copy in once, process in parallel,  copy out once.

Debayer should be an easy algorithm to split up like this, and indeed there are free libraries available to do GPU debayer, e.g.: https://github.com/avionic-design/cuda-debayer

[0xff35654c]

Yes, a while ago I did some tests, and results were bad, *I could control tasks priority using this function on 700D 0xff35654c:
Lowering priority to a medium amount resulted in worse write speed and probably corrupted frames in some tasks, lowering priority to maximum or very high values --> Camera crash

Ah, good, you tried it   A shame it didn't work properly.  Your guess that other things are waiting on the results makes sense to me from those symptoms.  Maybe improvements could be made, but they don't sound the easy kind.

I forget, Bilal - did you try lowering priority on these tasks, rather than disabling them?  That might allow them to work, presumably at a lower frequency, while still saving quite a lot of CPU.  Might be a useful tradeoff; I'd guess things like shutter fine tuning would update less frequently but would still work.  Presumably you'd gain less write speed, but hopefully still some?

That's crazy. 22fps on intel processor.

I get about 25fps on my linux desktop.  With gcc, qt5, 1.13.  AMD 3800.  I think we're learning that old Macs are slow, not new Macs are fast

We don't know the exact issue.  We know it crashes during early boot.  We don't have good logs.  If you're happy messing about with a serial port and working out how to connect it as described in my prior post, I can talk you through the details.

There are approximately three devs.  I don't believe any are in NC

The file numbering doesn't show the shutter count.

From the thread linked earlier, RP seems to have some large offset applied to the TotalShutter number, we don't know why.  Subtracting this, yours is probably 2297.  We suspect TotalShutter is shutter actuations, and TotalShoot is pictures taken, including both physical and electronic shutter.

If the count had been reset, you would have no way of knowing.  Shutter could really have 1,000,000 activations, then the count was reset, then 2297 taken.

I'm the most active (only?) dev for Digic 7 cams, and I don't have a 6D2.  I did a blind port, but don't have a way to test it.  It has no features.

There won't be any progress on 6D2 until we get a dev that wants to work on it, or I get a 6D2 and a lot more spare time.

https://builds.magiclantern.fm/features.html

Am I right in thinking you have to poll with PTP?  If so latency might be a factor for use as a controller.

UART is simpler to code and doesn't require polling, but does require minor messing about to connect up (details already given on Discord).

I understand now, thanks - accidentally trying to build the old zip copy because the names were the same but different places.  Thanks for explaining, it's useful if I understand common problems with the official repo.