LiveView hacks (write speed improvement)

[ML700D]

Write speed is affected by many factors like resolution and FPS, it varies from video mode to another, also from crop preset to another.
Higher resolution and FPS means reducded write speed compared to less resolution preset or lower FPS. Anyway the hacks help in all kind of settings.

You shouldn't compare write speed in a preset to another, but compare write speed in one preset like this: once with hacks disabled and once with hacks enabled.
Also, all hacks got disabled if you are not recording, benchmarks in video mode will always show write speed without the hacks.

thanks bilal.
I use play mode and the test look stable now
with all hack enable I can record 1440 1:1 preset with longer duration

[dpjpandone]

Nope, it does make a difference . . at least my benchmarks prove that.

How did you make your calculations?

5D3 Sandisk Extreme Pro 170MB/s SD card @240 MHz

Real world RECORDING (not benchmark) speeds (global draw enabled, but no histogram/zebras or other performance sucking overlays)

Small hacks = 81MB/s
Small hacks +LVFACE/AEWB = 88MB/s
Small hacks +LVFACE/AEWB + Cartridge cancel = 88MB/s

what are your actual sustained RECORDING speeds?

[Grognard]

Small hacks = 81MB/s
Small hacks +LVFACE/AEWB = 88MB/s
Small hacks +LVFACE/AEWB + Cartridge cancel = 88MB/s

And what about. Small hacks + LVFACE (without AEWB)?

[dpjpandone]

And what about. Small hacks + LVFACE (without AEWB)?

it's less without AEWB. Pretty close to just small hacks. AEWB nets the massive 7 MB/s improvement

It's a pretty huge sacrifice to lose shutter fine tuning, but this hack makes it possible to record continuous 12-bit lossless 3k 2.39 to SD card at very high ISO's which is the most I will ever ask the camera to do in real life. I have no use for broken preview modes. 3.5k with a realtime 5x preview is perfect for extreme closeups or macro shots, I do the rest of my work in 1080p and the 5D3 is DAMN SHARP and rolling shutter is acceptable.

[theBilalFakhouri]

@dpjpandone

Your test isn't accurate, beside it misses some infos:

1. Run benchmarks in Play mode at 240 MHz, reported write speed? if SD card already reached its maximum write speed in LiveView (with small and LVFACE/AEWB hacks), you won't gain anything more in LiveView.
2. Your camera and ML settings?
3. Try this test: turn on Small hacks, turn off "LVFACE/AEWB" then hit recording button, reported write speed?, now turn on "Cartridge cancel" (keep LVFACE/AEWB off, and small hacks on), reported write speed?
4. Reported write speed in LiveView (from RAW video) isn't always accurate, buffer size will have an impact too, especially on 5D3, you need to record long clips at same settings (FPS, resolution, exposure and a static scene and s if you are using lossless, and it would be better to use uncompressed RAW), also record a few clips each time, make the calculations then average the results.

Real world RECORDING (not benchmark) speeds ..

Yes, I said "at least" benchmarks prove that, but in reality I can see an improvement in recording times and write speed also in actual recording, especially on 700D. 5D3 does have an improvement too.

Quick tests on 700D, 240 MHz, 1736x1160, 14-bit uncompressed, 23.976 FPS, Global Draw off, mlv_snd on, "lvface and aewb" are disabled reported required write speed 80.5 MB/s:

-Small hacks enabled:

Clip 1: 177 Frames
Clip 2: 198 Frames
Clip 3: 189 Frames
Clip 4: 185 Frames
Clip 5: 191 Frames

Average: 188 Frames, 7.8 Seconds, reported write speed up to ~67 MB/s

-Small hacks enabled + CartridgeCancel:

Clip 1: 211 Frames
Clip 2: 378 Frames
Clip 3: 377 Frames
Clip 4: 243 Frames
Clip 5: 187 Frames
Clip 6: 375 Frames

Average: 295 Frames, 12.2 Seconds, reported write speed up to ~74 MB/s
5D3 should give similar results (I did some tests in past).

-Last note:
You can see the most gain for each hack once that hack is enabled alone, e.g. if you enable small hacks you will see *large* write speed improvement compared to if small hacks disabled, if you turned off small hacks, and only enable lvface and aewb, you will see the most gain for these two hacks, but if you enable small hacks + lvface and aewb, the total gain will be reduced:

for example:
if you gained 7 MB/s with small hacks alone
if you gained 7 MB/s with more hacks alone (lvface and aewb)
the total gain won't be 14 MB/s if both were enabled, but something less than that, like 11-12 MB/s . . this applies to "CartridgeCancel" too. There is some kind of curve here.

[theBilalFakhouri]

And what about. Small hacks + LVFACE (without AEWB)?

aewb task is the most CPU hungry task on 700D, on 5D3 it takes good amount of CPU too, suspending aweb has nice positive impact on write speed.

AFAIK, currently Shutter fine-tuning is depending on aewb task (when aewb task get disabled, it break this feature), more likely it can be fixed if we override shutter speed registers directly from ADTG patch . .  I didn't test this theroy yet.

[names_are_hard]

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?

[theBilalFakhouri]

@names_are_hard

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

My theory is other tasks like LiveView ones will still wait for other tasks to do its job, resulting in delays (which mean lower write speed, corrupted frames), if priority was too low, other tasks never get response from the lowered priority task, so it crashes the camera.

..rather than disabling them?

The tasks themselves never get disabled on Canon task manager, it got disabled internally, maybe it does something like "hey other tasks, work without me", it's probably set a flag, or return a response sayes everything work fine to other tasks/function, beside it stop doing its job.

*I couldn't change priorities for all tasks on real hardware using that function (well, I could change priorities for the tasks I needed), on QEMU the same function could change priorities for all tasks.

[dpjpandone]

-Last note:
You can see the most gain for each hack once that hack is enabled alone, e.g. if you enable small hacks you will see *large* write speed improvement compared to if small hacks disabled, if you turned off small hacks, and only enable lvface and aewb, you will see the most gain for these two hacks, but if you enable small hacks + lvface and aewb, the total gain will be reduced:

this makes sense, I'll try it without the other hacks and let you know

[names_are_hard]

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.

[Danne]

Good thing bilal also solved 240Mhz sd patch solving most overhead issues even without the aewb hack .

[dpjpandone]

Good thing bilal also solved 240Mhz sd patch solving most overhead issues even without the aewb hack .

Truly amazing

[dpjpandone]

I was putting it back to test again. why is it complaining about this:

Code: [Select]

mlv_lite.c:2170:9: warning: initialization of 'void (*)()' from 'unsigned int' makes pointer from integer without a cast [-Wint-conversion]
 2170 |         cam_eos_m ? 0xff2606f4 :
      |         ^~~~~~~~~
mlv_lite.c:2176:9: warning: initialization of 'void (*)()' from 'unsigned int' makes pointer from integer without a cast [-Wint-conversion]
 2176 |         cam_eos_m ? 0xff177ff8 :
      |         ^~~~~~~~~
mlv_lite.c:2196:9: warning: initialization of 'void (*)()' from 'unsigned int' makes pointer from integer without a cast [-Wint-conversion]
 2196 |         cam_eos_m ? 0xffa7e7d8 :
      |         ^~~~~~~~~

Code: [Select]

void hack_liveview_more()
{
    if (more_hacks && shamem_read(0xc0f383d4) != 0x4f0010) /* excludes mcm mode on eosm */
    {
        void (*aewbSuspend)() =
        cam_eos_m ? 0xff2606f4 :
        cam_5d3_113 ? 0xff23bc60 :
        cam_5d3_123 ? 0xff23ff10 :
        0;
       
        void (*lvfaceEnd)() =
        cam_eos_m ? 0xff177ff8 :
        cam_5d3_113 ? 0xff16d77c :
        cam_5d3_123 ? 0xff16e318 :
        0;
       
        lvfaceEnd();
       
        if (more_hacks == 2)
        {
            aewbSuspend();
        }
    }
   




    if (one_more_hack)
    {
        void (*CartridgeCancel)() =
        cam_eos_m ? 0xffa7e7d8 :
        cam_5d3_113 ? 0xff17fd68 :
        cam_5d3_123 ? 0xff181340 :
        0;
        CartridgeCancel();
        msleep(40);
    }
}
is this compiler just being picky or is there a problem?

[Danne]

I have the same. Some unclean code which needs some polishing I guess.

[dpjpandone]

I have the same. Some unclean code which needs some polishing I guess.

should we put those lines in the stubs file instead?

[theBilalFakhouri]

We can add the addresses like this commit, under:

Code: [Select]

static unsigned int raw_rec_init()

Code: [Select]

    if (is_camera("5D3", "1.1.3"))
    {
            lvfaceEnd  = (void *) 0xFF16D77C;
            aewbSuspend = (void *) 0xFF23BC60;
            CartridgeCancel = (void *) 0xFF17FD68;
    }
That commit is cleaner implementation than the one in our current builds.

Edit: that commit has some "spacing" issue, a coder error , it doesn't affect the code of course, but how it looks like. Will fix it later.
Thanks names_are_hard for mentioned this

[Danne]

Nice to see your code up there bilal online .

[names_are_hard]

We can add the addresses like this commit, under:

Code: [Select]

    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.

[Danne]

Is this source tree from your gitfix @names_are_hard? I notice commits are directly out into the original branch. Should we continue forking code instead? Are these cvamges to be pushed to your source again or is everyone keeps his own codebase?

[names_are_hard]

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.

[Danne]

Thanks for info here.
Maybe ironing out 10 most used branches personally and start building git tree from there.
It is still possible to keep all hg style but more for source access I guess.

[names_are_hard]

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.

[dpjpandone]

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.

I'm trying to learn how to do this properly. Can you please check my work?

The code from TheBilal was:

Code: [Select]

if (is_camera("5D3", "1.2.3"))
    {
        lvfaceEnd  = (void *) 0xFF16E318;
aewbSuspend = (void *) 0xFF23FF10;
CartridgeCancel = (void *) 0xFF181340;
more_hacks_are_supported = 1;
CartridgeCancel_works = 1;
The first step I think is to add this to Stubs.s in 5D3.123:

Code: [Select]

/** LiveView Hacks For Write Speed Improvement **/
///NSTUB(0xFF16E318,  lvfaceEnd)
///NSTUB(0xFF23FF10,  aewbSuspend)
///NSTUB(0xFF181340, CartridgeCancel)
now when we get to:

Code: [Select]

CartridgeCancel_works = 1; should this go in Features.h ?

like this?:

Code: [Select]

#define FEATURE_CARTRIDGE_CANCEL// write speed improvement in LiveView
#define FEATURE_MORE_HACKS// write speed improvement in LiveView 
Let me stop here because I dont know if it's ok to define a feature that is dependent on a module?

[names_are_hard]

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.

[dpjpandone]

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.

Not asking you to test it, just to tell me if I'm doing stubs wrong.

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.

This is helpful. On second glance I can't find an example of a module calling ifdef(feature xyz)

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.

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?