Will the real SoundDevShutDownIn please stand up

Started by dfort, September 27, 2017, 07:02:45 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

dfort

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

a1ex

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.

dfort

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

dfort

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

canneloni

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?
100D.100B ; Canon 18-55 STM ; Canon 50 1,8 II ; Canon 75-300 4,0 - 5,6 III ; Sigma 17-50 2,8

Danne

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?

dfort

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.

dfort

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.

DeafEyeJedi

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!
5D3.113 | 5D3.123 | EOSM.203 | 7D.203 | 70D.112 | 100D.101 | EOSM2.* | 50D.109

Danne

Short report. Audio meters works fine also when recording multiple files without the need to change in mlv_snd.c on eos 100D.

IDA_ML

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.

dfort

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.

dfort

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.

dfort

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

g3gg0

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.
Help us with datasheets - Help us with register dumps
magic lantern: 1Magic9991E1eWbGvrsx186GovYCXFbppY, server expenses: [email protected]
ONLY donate for things we have done, not for things you expect!

dfort

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!

g3gg0

well. that code was made ages ago. but probably it is helpful.
Help us with datasheets - Help us with register dumps
magic lantern: 1Magic9991E1eWbGvrsx186GovYCXFbppY, server expenses: [email protected]
ONLY donate for things we have done, not for things you expect!

dfort

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.

nikfreak

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.
[size=8pt]70D.112 & 100D.101[/size]

andy kh

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
5D Mark III - 70D

dfort

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:





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

dfort

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.

dfort

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.

andy kh

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. 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
5D Mark III - 70D

dfort

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.