Will the real SoundDevShutDownIn please stand up

[dfort]

@andy_kh

Ok--got the test for the 70D ready. @DeafEyeJedi is also running this but it would be good to make sure the results are consistent.

The way to run the test is fairly simple. Turn on both the mlv_rec and mlv_snd modules and activate those modules. I prefer to reduce the frame to the smallest size because we're only checking for an audio issue. The first time you record an MLV clip the audio meters should work normally. What I'm interested in is what happens on the second through the forth clip. Make sure not to restart the camera between takes. Do the audio meters continue to operate normally?

Now go to my bitbucket download page and grab the four 70D test builds at the top of the list. Then go through them in order starting with 70D_merge_fw112.2017Oct04.70D112 and run the test. That first one is just a build that has no modifications so it should behave the same as the build you got from the 70D topic. If it doesn't work the same--stop, either I messed up or the branch in the main repository is broken.

Next run the test on each of the A/B/C builds and report back.

A has the new mlv_snd changes with SoundDevShutDownIn commented out in stubs.S. This should work the same as the 70D_merge_fw112 build.
B uses both StopASIFDMAADC and SoundDevShutDownIn.
C uses neither StopASIFDMAADC or SoundDevShutDownIn.

The A build should work. It will be interesting to see what is happening with the B and C builds.

[andy kh]

audio meters continue to work normally for all the builds however i get this messsage with the c build "audio fail to stop, state 4" when i stop the recording

[Danne]

Code: [Select]

audio fail to stop, state 4Happens on all builds?

[andy kh]

Code: [Select]

audio fail to stop, state 4Happens on all builds?

not all. i get this message only on C build

[Danne]

ah, missed that part of the information, sorry

[dfort]

Excellent report. So we can use StopASIFDMAADC like the EOSM or both StopASIFDMAADC and SoundDevShutDownIn in tandem like the 100D on the 70D

Still waiting for a report from a 6D user. Once those results are in we can look at fixing this issue.

[DeafEyeJedi]

Ok--got the test for the 70D ready. @DeafEyeJedi is also running this but it would be good to make sure the results are consistent.

The A build should work. It will be interesting to see what is happening with the B and C builds.

Here are the results from this morning...

•70D_merge_fw112.2017Oct04.70D112 -- Ran 5 takes in total and all audio meters looked normal and worked without the audio failed messages.

•A build -- Ran 5 takes in total and all audio meters looked normal and worked without the audio failed messages.

•B build -- Ran 5 takes in total and all audio meters looked normal and worked without the audio failed messages.

•C build -- This one was rather interesting. Notice the start up into ML manu took longer. Orange LED blinking for about 5-6 seconds upon turn on of camera which isn't normal. Also notice when pressing record (takes about 2-3 seconds sometimes more to finally start recording. I do get the 'Audio failed to stop, state 4' message at the end of 2nd recording AFTER the first take on this particular build. Meaning Audio works on the first take w no message. 2nd take (without camera restart) which prompt failed message. If restarted, then first take will use audio but then vice versa afterwards as you already know.

Thanks for these test builds, Dan and please let me know if there are anything else I can do to help you move forward with anything.

[dfort]

Thanks! Good to verify that the results are reproducible.

...please let me know if there are anything else I can do to help you move forward with anything.

Figure out how to get a 6D and run the test on that camera.

[dfort]

All the tests are in!

Now, how to straighten out the StopASIFDMAADC / SoundDevShutDownIn stubs issue once and for all?

I'd like to get this fixed on the unified branch so that the changes will eventually get into the various experimental branches instead of doing the fixes only in crop_rec_4k where several of the affected platforms don't even compile. Note that as a result of this seemingly petty topic we discovered and fixed a bug in the 700D and 650D.

So far the "fix" was to make sure we have the right stub address for SoundDevShutDownIn and comment out StopASIFDMAADC and/or SoundDevShutDownIn depending on the camera. I would prefer to do the fix in mlv_snd by checking for the camera and using the correct combination of those stubs like this pull request that I submitted but later changed. I think that having the stubs in order is worth adding a few lines in mlv_snd.c and documenting the issues with the various cameras right in the code will help future development.

Anyway, thought I'd put this out there before submitting the pull request.

[dfort]

Well I thought I was all set to get this straightened out but it looks like there's a few details still left to do. Since we're only dealing with SoundDevShutDownIn and StopASIFDMAADC I took a closer look and it seems that there are some more cameras to check out.

...
SoundDevActiveIn: 5D2, 5D3, 5DSM, 6D, 7D, 7D2M, 7DM, 50D, 60D, 70D, 80D, 100D, 500D, 550D, 600D, 650D, 700D, 750D, 760D, 1100D, 1200D, 1300D, EOSM, EOSM2
SoundDevShutDownIn: 5D3, 5DSM, 6D, 7D, 7D2M, 7DM, 70D, 80D, 100D, 650D, 700D, 750D, 760D, 1200D, 1300D, EOSM, EOSM2.
StartASIFDMAADC: 5D2, 5D3, 6D, 7D, 7DM, 50D, 60D, 70D, 100D, 500D, 550D, 600D, 650D, 700D, 1100D, 1200D, 1300D, EOSM, EOSM2.
StopASIFDMAADC: 6D, 70D, 100D, 500D, 1200D, 1300D, EOSM, EOSM2.

Would be great if you can get a dm-spy log with these functions, to see when/how Canon code calls them (as it's not obvious for me from the disassembly).

Please note I don't know what I'm doing - this part of the code is not mine.

I don't know what I'm doing either but looking at the 5D3 stubs:

5D3.113

Code: [Select]

// NSTUB(    ???,  SoundDevShutDownIn)                      /* present on 7D.203, 6D.113, 650D.104, EOSM.202, 700D.113 */
// NSTUB(    ???,  StopASIFDMAADC)                          /* present on 7D.203, 6D.113, EOSM.202 */

5D3.123

Code: [Select]

NSTUB(0xFF10E7B0,  StopASIFDMAADC)
...
// NSTUB(    ???,  SoundDevShutDownIn)                      /* present on 7D.203, 6D.113, 700D.111, 650D.104, EOSM.202 */

So StopASIFDMAADC showed up after the firmware update but not SoundDevShutDownIn or maybe since it was commented out the address wasn't saved either? Did someone throw out the baby with the bath water or is it really missing? More testing to do see what happens when using StopASIFDMAADC on the 5D3.123.

Another test is to run dm-spy and look at what the logs reveal about SoundDevShutDownIn and StopASIFDMAADC on various cameras.

This is what we learned about the cameras tested so far:

• Cameras that can use both SoundDevShutDownIn and StopASIFDMAADC or just StopASIFDMAADC to stop the audio: 6D, 70D, 100D
• Cameras that use neither SoundDevShutDownIn or StopASIFDMAADC to stop the audio: 7D, 650D, 700D
• Camera that can only use StopASIFDMAADC to stop the audio: EOSM

The 7D is a special case because it also works with SoundDevShutDownIn which is the way mlv_snd is currently coded. In fact it is the only camera that works with the mlv_snd audio shut down code but since it doesn't seem to make a difference whether or not that stub is active we could assume that it isn't necessary.

So no models that we tested so far really need this:

Code: [Select]

    /* some models may need this */
    SoundDevShutDownIn();
    audio_configure(1);

However, the 6D, 70D, 100D and EOSM need this:

Code: [Select]

    StopASIFDMAADC();
    audio_configure(1);


[dfort]

...More testing to do see what happens when using StopASIFDMAADC on the 5D3.123...

Whoa--it crashes. Here are the crash logs.

Code: [Select]

ASSERT: hLvJob->hJpegMemSuite
at ./Epp/Vram/VramStage.c:891, Epp:ff1859cc
lv:1 mode:3

Epp stack: 17dbd0 [17dca8-17d0a8]
0xUNKNOWN  @ de48:17dca0
0xUNKNOWN  @ 17bbc:17dc78
0x000178B4 @ ff0de67c:17dc58
0xUNKNOWN  @ 178e4:17dc48
0xUNKNOWN  @ 1796c:17dc28
0x00001900 @ ff1859c8:17dc08
0x00069868 @ 69c1c:17dbd0

Magic Lantern version : Nightly.2017Oct10.5D3123
Mercurial changeset   : 5c5211d7eb74+ (crop_rec_4k_mlv_snd_audio_issue) tip
Built on 2017-10-10 16:55:36 UTC by rosiefort@RosieFoComputer.
Free Memory  : 129K + 3836K


Code: [Select]

ASSERT: IsSuiteSignature( hSuite )
at ./PackMemory/PackMem.c:599, Epp:aefc
lv:0 mode:3

Epp stack: 17dbc0 [17dca8-17d0a8]
0xUNKNOWN  @ de48:17dca0
0xUNKNOWN  @ 17bbc:17dc78
0x000178B4 @ ff0de67c:17dc58
0xUNKNOWN  @ 178e4:17dc48
0xUNKNOWN  @ 1796c:17dc28
0x0000AEB0 @ ff185a48:17dc08
0x00001900 @ aef8:17dbf8
0x00069868 @ 69c1c:17dbc0

Magic Lantern version : Nightly.2017Oct10.5D3123
Mercurial changeset   : 5c5211d7eb74+ (crop_rec_4k_mlv_snd_audio_issue) tip
Built on 2017-10-10 16:55:36 UTC by rosiefort@RosieFoComputer.
Free Memory  : 130K + 3835K

Uh oh -- pull request to fix this and all the other issues covered in this topic submitted to the crop_rec_4k branch:

https://bitbucket.org/hudson/magic-lantern/pull-requests/871/fix-for-sounddevshutdownin-and/diff

Yeah, not all models that are affected currently work in the crop_rec_4k branch but at least it is now documented in case we want to do similar changes in the unified branch or maybe eventually implement a new sound system.

[EDIT] Posted test builds for the pull request (mlv_snd_fix.2017Oct10...). Note that even though this issue affects the 6D, 7D, 70D, 700D, 650D, 100D, EOSM and 5D3.123, only the 700D, 100D, EOSM and 5D3.123 compile in the crop_rec_4k branch so those are the ones I posted.

[dfort]

Pull request merged. Case closed. Moving on to more interesting projects.

[Lars Steenhoff]

Nice work!

[bitframe]

[EDIT] Posted test builds for the pull request (mlv_snd_fix.2017Oct10...). Note that even though this issue affects the 6D, 7D, 70D, 700D, 650D, 100D, EOSM and 5D3.123, only the 700D, 100D, EOSM and 5D3.123 compile in the crop_rec_4k branch so those are the ones I posted.

@dfort: is there a fix for the 100D yet, I was not able to find a working file for mlv_rec, mlv_snd and 10 bit recording working.

[dfort]

The 100D build with these changes should be in the Experiments download page. Look for it in the crop_rec_4k branch, top of that page. I took it off my page after it was merged to avoid confusion--of course I forgot to mention that.