Magic Lantern Forum

Developing Magic Lantern => General Development => Topic started by: dfort on September 27, 2017, 07:02:45 AM

Title: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 27, 2017, 07:02:45 AM
Maybe I'm making a mountain out of a mole hill but a few cameras have the same address for two different stubs, SoundDevShutDownIn and StopASIFDMAADC.

I made a list to illustrate the problem--and I do think this is a problem.

Cameras that don't have either the SoundDevShutDownIn or StopASIFDMAADC stub

5D2
5D3
60D
500D
550D
600D
1100D

Cameras that have both the SoundDevShutDownIn and StopASIFDMAADC stubs

7D:
NSTUB(0xFF064304,  SoundDevShutDownIn)
...
NSTUB(0xFF061C08,  StopASIFDMAADC)                          // Called by SoundDevStopIn

650D:
NSTUB(0xFF10B0D8,  SoundDevShutDownIn)
...
NSTUB(0xFF1088E0,  StopASIFDMAADC)  <-- Not Listed in stubs.S

700D:
NSTUB(0xFF10B930,  SoundDevShutDownIn)
...
NSTUB(0xFF109138,  StopASIFDMAADC)                          /* present on 7D.203, 6D.113, EOSM.202 */

Cameras that use the same address for both the SoundDevShutDownIn and StopASIFDMAADC stubs

6D:
NSTUB(0xFF11C874,  SoundDevShutDownIn)
...
NSTUB(0xFF11C874,  StopASIFDMAADC)

EOSM:
NSTUB(0xFF10A920,  SoundDevShutDownIn)                      // Temporarily using address for StopASIFDMAADC to resolve MLV_SND issue
...
NSTUB(0xFF10A920,  StopASIFDMAADC)                          // Regular -- Stop ASIF ADC - needed for future changes to mlv_snd.c

100D:
NSTUB(0xFF112680,  StopASIFDMAADC)
...
NSTUB(0xFF112680,  SoundDevShutDownIn)

70D:
NSTUB(0xFF11758C,  StopASIFDMAADC)
...
NSTUB(0xFF11758C,  SoundDevShutDownIn)


Differentiating between these two stubs becomes important in this bit of code in the mlv_snd_stop() function:

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


Now those platforms that have duplicate addresses are using only StopASIFDMAADC, not the SoundDevShutDownIn. So what happens if we simply do this:

/* of course we need to initialize it */
extern WEAK_FUNC(ret_0) int StopASIFDMAADC();
...
    /* some models may need this */
    StopASIFDMAADC();
    audio_configure(1);


That way we could put the real SoundDevShutDownIn addresses in the stubs.S for the 6D, EOSM, 100D and 70D. The question is, what will that do to the 7D, 650D and 700D? Also, where else is SoundDevShutDownIn used?

beep.c
#if defined(CONFIG_7D) || defined(CONFIG_6D) || defined(CONFIG_70D) || defined(CONFIG_EOSM)
    /* experimental for 7D now, has to be made generic */
/* Disable Audio */
    void SoundDevShutDownIn();
    SoundDevShutDownIn();
#endif


It is also used in bitrate-6d.c but that section of code is commented out so it doesn't apply.

Seems that now that experimental builds are being compiled and published for anyone willing to test, maybe the crop_rec_4k branch would be a good place to try out a fix? That branch includes the 100D and 70D so it is probably the best place to run an experiment to put the right SoundDevShutDownIn address in the 6D, EOSM, 100D and 70D, modify mlv_snd.c and beep.c to use StopASIFDMAADC and ask testers to report back.

Why fix it if it is working? Because duplicate stub addresses is a rather ugly hack.

Here are a few links to previous discussions of this issue:

http://www.magiclantern.fm/forum/index.php?topic=16040.msg190519#msg190519
https://bitbucket.org/hudson/magic-lantern/pull-requests/863/fix-for-audio-issues-on-eos-100d-possibly/diff
https://bitbucket.org/hudson/magic-lantern/pull-requests/657/eosm-code-cleanup/diff#comment-9556325

And here's a bug report that is affected if you use the "right" SoundDevShutDownIn address in the current mlv_snd code:

https://bitbucket.org/hudson/magic-lantern/issues/2255/mlv_sound-working-only-on-the-first-video

By the way, for those of you that didn't understand my subject line, it was taken from a popular TV show named To Tell The Truth. The show first aired in the 1950's where a panel of four celebrities had to correctly identify the contestant (usually with an unusual occupation or experience) from the two imposters.

Then again maybe I chose the imposter and the real SoundDevShutDownIn really is StopASIFDMAADC? That stub is used only once:

#ifdef CONFIG_6D
void StopASIFDMAADC();
StopASIFDMAADC(asif_rec_stop_cbr, 0);
#endif
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: a1ex on September 27, 2017, 11:22:12 PM
That's a good cleanup project; figured out I should apply this patch:


diff -r e90a5d0164ee -r bb2030306d4a platform/100D.101/stubs.S
--- a/platform/100D.101/stubs.S
+++ b/platform/100D.101/stubs.S
@@ -78,1 +78,1 @@
-NSTUB(0xFF112680,  SoundDevShutDownIn)
+NSTUB(0xFF115108,  SoundDevShutDownIn)
diff -r e90a5d0164ee -r bb2030306d4a platform/6D.116/stubs.S
--- a/platform/6D.116/stubs.S
+++ b/platform/6D.116/stubs.S
@@ -85,1 +85,1 @@
-NSTUB(0xFF11C874,  SoundDevShutDownIn)
+NSTUB(0xFF11E8A4,  SoundDevShutDownIn)
diff -r e90a5d0164ee -r bb2030306d4a platform/70D.112/stubs.S
--- a/platform/70D.112/stubs.S
+++ b/platform/70D.112/stubs.S
@@ -80,1 +80,1 @@
-NSTUB(0xFF11758C,  SoundDevShutDownIn)
+NSTUB(0xFF1195C4,  SoundDevShutDownIn)
diff -r e90a5d0164ee -r bb2030306d4a platform/EOSM.202/stubs.S
--- a/platform/EOSM.202/stubs.S
+++ b/platform/EOSM.202/stubs.S
@@ -77,1 +77,1 @@
-NSTUB(0xFF10A920,  SoundDevShutDownIn)                      // Temporarily using address for StopASIFDMAADC to resolve MLV_SND issue
+NSTUB(0xFF10D3A0,  SoundDevShutDownIn)


but not sure yet what to do from here.

In beep.c, in WAV recording code (commented out for reasons other than these stubs), we have:

SoundDevActiveIn(0); /* optional for some models */
/* wav recording */
SoundDevShutDownIn(); /* optional for some models */


So far, so good - something that probably activates the sound device, and something that probably turns it off.

Same in mlv_snd:

SoundDevActiveIn(0); /* in mlv_snd_prepare_audio, for all models that have it defined */
/* sound recording between these */
SoundDevShutDownIn(); /* in mlv_snd_stop, for all models that have it defined */


Turning something on with SoundDevActiveIn and off with StopASIFDMAADC looks a bit weird to me.

In shoot.c:

                    //Enable Audio IC In Photo Mode if off
                    if (!is_movie_mode())
                    {
                        SoundDevActiveIn(0);
                    }


but nobody turns it off.

The best hint is in beep.c for 6D:


SoundDevActiveIn(0); /* optional for some models */
StartASIFDMAADC(..., asif_rec_continue_cbr, ...) /* all models */
/* wav recording - waits until asif_rec_continue_cbr decides to finish */
/* asif_rec_continue_cbr calls StopASIFDMAADC on its last call (6D only) */
SoundDevShutDownIn(); /* optional for some models */


That makes some sense to me, so I assume all models require StopASIFDMAADC, and some require SoundDevShutDownIn as well.

Nope - older models don't even have the StopASIFDMAADC string in the firmware. It's probably required for all models that have it, and SoundDevShutDownIn is probably nice to have*) for all models that have it.

*) needed, but with minor side effects it it's not there; if it does what I think it does, you will probably see the difference between just StopASIFDMAADC and StopASIFDMAADC+SoundDevShutDownIn with a... multimeter.

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.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 28, 2017, 12:06:43 AM
with a... multimeter???

Quote from: a1ex on September 27, 2017, 11:22:12 PM
Turning something on with SoundDevActiveIn and off with StopASIFDMAADC looks a bit weird to me.

Exactly my thoughts, yet as you know that's what is happening by giving SoundDevShutDownIn the address for StopASIFDMAADC.

I was working with Danne on his recent 100D experimenting and he came up with something rather clever on his original pull request to get audio working on that camera (https://bitbucket.org/hudson/magic-lantern/commits/d1f9fdeb3bd60f327b78fede10f6b76360b37c06?at=100D_merge_fw101). Basically, StopASIFDMAADC+SoundDevShutDownIn instead of just StopASIFDMAADC (though naming it SoundDevShutDownIn)--ugh!

-extern WEAK_FUNC(ret_0) int SoundDevShutDownIn();
+extern WEAK_FUNC(ret_0) int SoundDevShutDownIn();
+extern WEAK_FUNC(ret_0) int StopASIFDMAADC();
...
     /* some models may need this */
-    SoundDevShutDownIn();
+    SoundDevShutDownIn();
+    StopASIFDMAADC();
     audio_configure(1);


What do you think this does?
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 28, 2017, 07:24:24 AM
Looks like the test builds are up--let's start testing!

Calling volunteer 6D, EOSM, 100D, 70D users to try out the latest crop_rec_4k experimental builds and report back.

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

Note that the 6D and 70D aren't on that page--I compiled test builds for those cameras and posted them in my Bitbucket download page (https://bitbucket.org/daniel_fort/magic-lantern/downloads/). Don't get too excited, some of the cool features from the crop_rec_4k branch probably aren't working on those cameras. What we're looking for is the following:

Are you able to record multiple clips with audio using mlv_rec with mlv_snd? Are the beeps still working? If you discover any strange behavior, regress to a previous build and test again.

If possible:
Quote from: a1ex on September 27, 2017, 11:22:12 PM
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).

[EDIT] I'll be the first crash test dummy -- On the EOSM sound recording using mlv_rec/mlv_snd works though the audio meters stop functioning after recording a clip. Restarting the camera or taking a still photo restores the audio meters. Regressing one step back (commit e90a5d0) the audio meters work fine. Another issue is that when running a fresh install and using mlv_rec/mlv_snd for the first time, it doesn't seem to go into the normal record mode (no recording icon on the LV screen) and it only saves a MLV_REC.TMP file on the card. Restarting the camera "fixes" this and that issue seems to go away. This also happened in the regression test so I tried out the unified branch and it worked fine. More testing to come.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: canneloni on September 28, 2017, 08:00:12 AM
Just downloaded the build and tried mlv_rec with mlv_sound and had no problem at all. Was able to record multiple clips with sound turned on. Play them with latest MLV Producer and got good video and good sound!

I also tried mlv_lite without sound and without crop_rec and was also able to record some seconds before recording was stopped automatically.

Edit: When i try mlv_lite with crop_rec my bottom half of the screen goes white and when i hit record i get the message "Raw detect error"

What exactly do you mean with beep sound?
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: Danne on September 28, 2017, 08:19:47 AM
QuoteEdit: When i try mlv_lite with crop_rec my bottom half of the screen goes white and when i hit record i get the message "Raw detect error"
Your camera has to be set to 720p in menu. You are testing 100D obviously since eosm already is in mv720p mode when selecting crop_rec. Then again I´,m not sure this behaviour is expected since 5D mark III doesn´t need to manually set mv720p to go into crop_rec 3x3.
I don´t think there is a beep option on the 100D. Is there one on the eosm?
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 28, 2017, 08:34:50 AM
I made some additions to the post with the EOSM test. So you're not experiencing those audio meter issues?

Quote from: Danne on September 28, 2017, 08:19:47 AM
I don´t think there is a beep option on the 100D. Is there one on the eosm?

I'm not sure how to test for beeps. Maybe play Arkanoid? The EOSM doesn't make any sounds while the 5D3 does.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 28, 2017, 08:47:26 AM
Did a followup test on the EOSM. Removing SoundDevShutDownIn fixed the audio meters issue:

mlv_snd.c
    /* some models may need this */
    StopASIFDMAADC();
-   SoundDevShutDownIn();
    audio_configure(1);


Sort of expected that because it puts us back where we were before commit f9c3d6e. This should also work for the 100D.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: DeafEyeJedi on September 28, 2017, 08:50:32 AM
Quote from: dfort on September 28, 2017, 08:47:26 AM
...Sort of expected that because it puts us back where we were before commit f9c3d6e. This should also work for the 100D.

Great find, @dfort!
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: Danne on September 28, 2017, 09:00:20 AM
Short report. Audio meters works fine also when recording multiple files without the need to change in mlv_snd.c on eos 100D.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: IDA_ML on September 28, 2017, 04:09:05 PM
I can also confirm that MLV sound works fine when recording with the MLV_rec module at 1728x972 resolution on the 100D.  Sound also gets recorded when I press the 5x magnification button in that mode.  I get the following record times when recording 10-bit MLV video WITH sound:

1) Normal mode at 1728x972 resolution: 15 s. with sound and 16 s. without sound;

2) 5x magnification active at 1920x1080 resolution: slightly more than 7 s. with sound

3) 5x magnification active at 2492x1080 resolution: slightly more than 4 s. with sound.

I preformed several consecutive recordings without turning camera off, by randomly switching between normal and 5x magnification modes as I often do when shooting MLV video with my EOS 35/F2 IS lens.  Every time sound was recorded properly.  I still have to check if sound is recorded synchronously with video.  Latest (today's) version of MLV Producer opens the files almost instantly by dragging and dropping from Explorer, kills the focus points automatically producing a very clean and vivid image, plays the files with sound and exports them with sound into the MPEG2 format.  If the MPEG container is used for export, a nice thumbnail of the exported file appears in Explorer - very useful, gives you a hint what the clip is all about.

I used a 64 GB SanDisk Expreme SD-card with about 60 MB/s write speed for the above experiments.  The ML benchmark test for that card indicated 41,6 MB/s write speed in the camera.  I didn't test 12 and 14 bits since record times are too short in those modes - just about 4 s. recording at 14 bit and 1728x972 resolution.

Very exciting development for the 100D and MLV Producer!  Thank you all, guys, for your enormous efforts and the expert work.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 28, 2017, 05:24:26 PM
So using SoundDevShutDownIn with StopASIFDMAADC in mlv_snd is also affecting the 700D. It shows an "audio failed to stop. state 4" error message. Using SoundDevShutDownIn works but the 700D also has the audio meter issue I noted on the EOSM. Removing both SoundDevShutDownIn and StopASIFDMAADC and the 700D works fine--no audio metering issues or audio error messages.

If it is good for the 700D is it good for the EOSM? Nope. It cleared up the audio meter issue but on the second and third clip the audio error message appeared and on the forth clip a "Threads failed to start" message appeared. So the EOSM does need the StopASIFDMAADC only.

Important finding -- the 700D on the unified branch also exhibits the audio meters issue. Meters stop working after recording the first clip. Interesting thing is that the EOSM is fine on the unified branch but has the audio meters issue ( [EDIT] in the crop_rec_4k branch) not matter what combination of SoundDevShutDownIn / StopASIFDMAADC I tried in mlv_snd.

I've been concentrating on changes to this:

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


but maybe this has to be changed too? [EDIT] Tried removing the following and the 700D froze when recording. All hell broke loose on the EOSM so it is probably best not to mess around with this.

    /* some models may need this */
    SoundDevActiveIn(0);

Testing continues.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 28, 2017, 09:24:35 PM
So what's the difference between the mlv_rec in the crop_rec_4k branch and the mlv_rec in unified? Ah ha--10/12bit. So I jumped to that branch and was able to reproduce the 700D issues on the raw_video_10bit_12bit branch. The EOSM seemed to be fine though.

Another problem is that while I am able to fix the issues with the 700D in mv1080 mode, it doesn't work at all in movie crop mode (the 600D mode).

Ok--so since mlv_rec wasn't even compiling in the crop_rec_4k branch until recently we aren't in too bad shape.

Danne reported to me that removing:

extern WEAK_FUNC(ret_0) int SoundDevShutDownIn();
...
SoundDevShutDownIn();


Doesn't work on the 100D so it looks like that camera is working fine with the latest changes but the 700D and EOSM need some work. I haven't heard back from any 6D or 70D users. Even though those cameras aren't really ready for crop_rec_4k, we could try to work out the issue with the SoundDevShutDownIn using the address for StopASIFDMAADC.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 29, 2017, 02:32:44 AM
Just ran an interesting regression test on the 700D. Trying to track down when that bug with the audio meters stopping after recording the first clip. I went all the way to the first 700D.114 of the Jenkins builds -- June 20, 2015 and it was still there. Glad it wasn't my 700D.115 update that broke it but am perplexed why such an old and obvious issue was never reported, or maybe it was and I missed it? Maybe I should keep going back to see if it is also in the 700D.113 firmware versions?

[EDIT] Went way back into the 700D.113 builds and the audio meters issue still showed up so it looks like it is about time to fix it?

[EDIT 2] https://bitbucket.org/hudson/magic-lantern/pull-requests/866/audio-meters-fix-for-700d/diff
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: g3gg0 on September 30, 2017, 12:25:13 PM
maybe there is some useful info in https://bitbucket.org/hudson/magic-lantern/branch/new-sound-system

rewrote the magic lantern sound code to allow full control and proper playback and recording.
even added a mp3 player and easy sound recording.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on September 30, 2017, 03:29:17 PM
Maybe--saw that SoundDevShutDownIn is also used in mlv_snd in the new-sound-system branch but it doesn't use the address for StopASIFDMAADC, at least not on the EOSM I'll need to check the others. Thanks for the tip--more to study!
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: g3gg0 on October 01, 2017, 10:20:55 AM
well. that code was made ages ago. but probably it is helpful.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 01, 2017, 04:04:17 PM
Just a quick update on the progress to get the SoundDevShutDownIn in order. We're running an experiment in the crop_rec_4k branch and so far have the EOSM, 700D and 100D are working properly with the correct SoundDevShutDownIn stub. Still need to check up on the 6D, 7D, 70D and 650D. I don't have access to those cameras and there aren't experimental crop_rec_4k builds for these cameras so I'll need some help with these.

@nikfreak -- How is the 70D doing on the crop_rec_4k branch? Looks like it is merged but not compiling yet. I was able to build it by guessing a few addresses but don't have any way of testing it.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: nikfreak on October 01, 2017, 04:23:00 PM
recording lossless works though there's some strange things happening on the lcd (like some sort of fps override kicking in and reducing fps to a felt 10fps), crop_rec 3x3 doesn't work at all. Regarding sound didn't try yet myself but also got no complaints till now that it could be broken - at least I don't remember any reports on 70D thread.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: andy kh on October 01, 2017, 05:24:48 PM
Quote from: nikfreak on October 01, 2017, 04:23:00 PM
recording lossless works though there's some strange things happening on the lcd (like some sort of fps override kicking in and reducing fps to a felt 10fps)

recording in raw get that strange thing on lcd with the latest nightly build(25 Sep 17) mvl recording works fine.

Quote from: nikfreak on October 01, 2017, 04:23:00 PM
crop_rec 3x3 doesn't work at all.

oh i have been waiting for this as i shoot slowmotion in all my videos
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 02, 2017, 03:00:02 AM
Quote from: nikfreak on October 01, 2017, 04:23:00 PM
recording lossless works though there's some strange things happening on the lcd (like some sort of fps override kicking in and reducing fps to a felt 10fps)

Yes, that happens on several of the lossless compression settings. The LiveView may turn black and white at a reduced refresh rate and it looks rather terrible but you change it. The setting is in mlv_lite:

(https://farm5.staticflickr.com/4422/37410702382_4d93491505.jpg) (https://flic.kr/p/YZRCs3)

(https://farm5.staticflickr.com/4474/37441663311_90a50a6bf2.jpg) (https://flic.kr/p/Z3Aj4a)

Quote from: nikfreak on October 01, 2017, 04:23:00 PM
crop_rec 3x3 doesn't work at all.

That could be a tricky one to get working properly. Most "bug" reports I've seen about crop_rec 3x3 on the 700D is because the user left the Canon settings at 1920x1080. It must be set to 1280x720. Another gotcha is that the LiveView is stretched vertically when it is working properly.

Quote from: nikfreak on October 01, 2017, 04:23:00 PM
Regarding sound didn't try yet myself but also got no complaints till now that it could be broken - at least I don't remember any reports on 70D thread.

The whole point of this topic stems from a conversation we had a couple of years ago.

https://bitbucket.org/hudson/magic-lantern/pull-requests/657/eosm-code-cleanup/activity#comment-9556325
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 03, 2017, 07:01:18 AM
Looked a little closer at the 650D and found this:

650D:
NSTUB(0xFF10B0D8,  SoundDevShutDownIn)
...
NSTUB(0xFF1088E0,  StopASIFDMAADC)  <-- Not Listed in stubs.S

That's right, StopASIFDMAADC is blank and commented out.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 04, 2017, 06:11:11 AM
Update

Fixed on the unified branch: 650D, 700D

Fixed on the crop_rec_4k branch: 100D, EOSM

Still need to run tests: 6D, 7D, 70D

Ok, it depends what "fixed" means to you. Originally I just wanted to straighten out the issue with the mixed up stub addresses but it turns out that we discovered an unreported issue with the audio meters on the 650D and 700D. The fix turned out to be commenting out the StopASIFDMAADC and SoundDevShutDownIn stubs. The 7D is set up the same way so I posted a message to see if someone could test that camera.

The 6D and 70D could probably get the same treatment as the EOSM which needs StopASIFDMAADC to shut down the audio--or maybe they need a third method?

Upon further testing with the 100D we discovered that it did better when both StopASIFDMAADC and SoundDevShutDownIn are used in tandem. So now we have a situation where the crop_rec_4k branch has the correct stub addresses and the 100D_merge_fw101 branch has the address for StopASIFDMAADC copied over to the SoundDevShutDownIn stub--which is something that should be corrected.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: andy kh on October 04, 2017, 04:22:30 PM
Quote from: dfort on September 28, 2017, 07:24:24 AM
Looks like the test builds are up--let's start testing!

Calling volunteer 6D, EOSM, 100D, 70D users to try out the latest crop_rec_4k experimental builds and report back.

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

Note that the 6D and 70D aren't on that page--I compiled test builds for those cameras and posted them in my Bitbucket download page (https://bitbucket.org/daniel_fort/magic-lantern/downloads/). Don't get too excited, some of the cool features from the crop_rec_4k branch probably aren't working on those cameras. What we're looking for is the following:


i missed this update. i came to know about this build today only. test the 70D. my camera live view dont turn on
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 04, 2017, 09:20:00 PM
Quote from: andy kh on October 04, 2017, 04:22:30 PM
test the 70D. my camera live view dont turn on

Not surprising. Though the 70D is in the crop_rec_4k branch it doesn't build. I had to make some changes for it to compile but perhaps it is better if I start with the 70D pull request.

Deleted both the 70D and 6D tests from my download page. I'll update this topic when there's something new to report.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 04, 2017, 11:02:43 PM
@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 (https://bitbucket.org/daniel_fort/magic-lantern/downloads/) 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.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: andy kh on October 05, 2017, 12:36:17 PM
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
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: Danne on October 05, 2017, 01:31:46 PM
audio fail to stop, state 4
Happens on all builds?
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: andy kh on October 05, 2017, 01:40:38 PM
Quote from: Danne on October 05, 2017, 01:31:46 PM
audio fail to stop, state 4
Happens on all builds?
not all. i get this message only on C build
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: Danne on October 05, 2017, 02:37:35 PM
ah, missed that part of the information, sorry
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 05, 2017, 04:08:09 PM
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 (http://www.magiclantern.fm/forum/index.php?topic=15088.msg191140;topicseen#msg191140) user. Once those results are in we can look at fixing this issue.
Title: Will the real SoundDevShutDownIn please stand up
Post by: DeafEyeJedi on October 05, 2017, 06:49:23 PM
Quote from: dfort on October 04, 2017, 11:02:43 PM
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.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 05, 2017, 07:12:18 PM
Thanks! Good to verify that the results are reproducible.

Quote from: DeafEyeJedi on October 05, 2017, 06:49:23 PM
...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.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 08, 2017, 10:47:47 PM
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 (https://bitbucket.org/daniel_fort/magic-lantern/commits/13a5bcbc0d70cb104258c30c508657e2831d14da?at=unified) 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.


Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 10, 2017, 08:03:28 AM
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.

Quote from: a1ex on September 27, 2017, 11:22:12 PM
...
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
// 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
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:
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:
    /* some models may need this */
    SoundDevShutDownIn();
    audio_configure(1);


However, the 6D, 70D, 100D and EOSM need this:
    StopASIFDMAADC();
    audio_configure(1);

Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 10, 2017, 07:42:12 PM
Quote from: dfort on October 10, 2017, 08:03:28 AM
...More testing to do see what happens when using StopASIFDMAADC on the 5D3.123...

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

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


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 (https://bitbucket.org/hudson/magic-lantern/branch/new-sound-system).

[EDIT] Posted test builds for the pull request (https://bitbucket.org/daniel_fort/magic-lantern/downloads/) (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.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 11, 2017, 12:54:39 AM
Pull request merged. Case closed. Moving on to more interesting projects.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: Lars Steenhoff on October 11, 2017, 02:05:05 AM
Nice work!
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: bitframe on October 17, 2017, 11:55:39 PM
Quote[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.
Title: Re: Will the real SoundDevShutDownIn please stand up
Post by: dfort on October 20, 2017, 05:33:01 PM
The 100D build with these changes should be in the Experiments download page (https://builds.magiclantern.fm/experiments.html). 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.