Trying to understand Movie Restart (so I can fix it...)

[dpjpandone]

Hey guys,

I want to solve issue #2065:  https://bitbucket.org/hudson/magic-lantern/issue/2065/movie-restart-7d-stuck-in-loop-cannot-stop

I have been looking at the following lines for movtweaks.c:

Code: [Select]

#ifdef FEATURE_MOVIE_RESTART
        static int recording_prev = 0;
       
        #if defined(CONFIG_5D2) || defined(CONFIG_50D) || defined(CONFIG_7D)
        if(!RECORDING_H264 && recording_prev && !movie_was_stopped_by_set) // see also gui.c
        #else
        if(!RECORDING_H264 && recording_prev && wait_for_lv_err_msg(0))
        #endif
        {
            if (movie_restart)
            {
                msleep(500);
                movie_start();
            }
        }
        recording_prev = RECORDING_H264;

        if(!RECORDING_H264)
        {
            movie_was_stopped_by_set = 0;
        }
    #endif
I'm trying to understand how it works, and why it is broken on 7D. One thing that stands out to me right away is that 7D does not use the SET button to start/stop recording (unlike 5D2 and 50D, it has a dedicated start/stop button)

any additional information you can provide would be greatly appreciated. I tried to compare to a build where it is working properly, but there are so many changes by now that it's hard for me to track it down. If you can just point me to the parts that are relevant, that would be awesome...

Thanks!

[ayshih]

Hmm, curious.  It looks like the 7D was lumped in with the 50D and the 5D2 because movie restart wasn't working the other way (see this commit from Jan 2013).  For the 7D, movie_was_stopped_by_set was set by BGMT_LV rather than BGMT_PRESS_SET, despite the name.

Then, it looks like movie restart broke recently because the 7D-specific gui.c was considered obsolete and removed (see this commit from two months ago).  It's possible that a1ex simply didn't notice that there was a custom button handler in there.

[a1ex]

Correct; that custom handler should be moved in the same file as the rest of implementation.

I wouldn't mind using the old-style method on all cameras (that would be the most portable way, and will get rid of some stubs). Also, g3gg0 has some nice ideas for improving movie restart here: http://www.magiclantern.fm/forum/index.php?topic=9625.msg99329#msg99329

[dpjpandone]

I took a stab at this over the weekend, but clearly I'm doing it wrong..... The worst part is I have to wait until 4GB are recorded every time I want to see if my changes had the desired effect.... Does it make sense to raise the bitrate and record a bunch of high-iso noise to hit this 4GB limit faster? or is there a better way?

[Walter Schulz]

On 7D with a fast card you have just to wait for about a minute.

[dpjpandone]

when you say:

"that custom handler should be moved in the same file as the rest of implementation"

should it be moved to movtweaks, or gui-common?

[dpjpandone]

when I put it in gui-common.c,

I get this:

Code: [Select]

$ make
[ VERSION  ]   ../../platform/7D.203/version.bin
[ VERSION  ]   ../../platform/7D.203/version.c
[ CC       ]   version.o
[ CC       ]   gui-common.o
../../src/gui-common.c: In function 'handle_common_events_by_feature':
../../src/gui-common.c:443:32: error: 'recording' undeclared (first use in this function)
 if (event->param == BGMT_LV && recording)
                                ^
../../src/gui-common.c:443:32: note: each undeclared identifier is reported only once for each function it appears in
../../Makefile.filerules:23: recipe for target 'gui-common.o' failed
make: *** [gui-common.o] Error 1

so I know I need to: int recording, but how should I do it, like this:

Code: [Select]

#if defined(CONFIG_7D)
extern int recording;
if (event->param == BGMT_LV && recording)
{
 extern int movie_was_stopped_by_set;
movie_was_stopped_by_set = 1;
}
#endif

I tells me "recording" is undefined? how should gui-common know that the camera is recording?

Am I even close? I'm starting to realize that my lack of understanding how each file/part of ML talks to eachother and passes variables/ints is seriously handicapping me. Albert shih found/showed me everything I needed to get this working, but I can't seem to figure out where to put it.

Could someone point me to an example of where a custom button handler was moved from a camera specific gui.c to gui-common.c? Maybe if I can see something similar (perhaps for a different camera, or different button) then I might be able to understand and figure it out on my own?

[dmilligan]

there are macros for recording state now (RECORDING_H264, RECORDING_RAW, etc.) and 'recording' was made private

[dpjpandone]

Thanks Dmilligan!

I'm pretty sure I got it, can someone confirm that it's ok to put this code after:

Code: [Select]

int handle_common_events_by_feature(struct event * event)
{
    // common to most cameras
    // there may be exceptions

My first inclination is to put it here:

Code: [Select]

// If we're here, we're dealing with a button press.  Record the timestamp
    // as a record of when the user was last actively pushing buttons.
    if (event->param != GMT_OLC_INFO_CHANGED)
        last_time_active = get_seconds_clock();

    #if defined(CONFIG_7D)
    if (event->param == BGMT_LV && RECORDING_H264)
    {
    extern int movie_was_stopped_by_set;
    movie_was_stopped_by_set = 1;
    }
    #endif

    #ifdef CONFIG_MENU_WITH_AV
    if (handle_av_short_for_menu(event) == 0) return 0;
    #endif

    if (handle_module_keys(event) == 0) return 0;
    if (handle_flexinfo_keys(event) == 0) return 0;

    #ifdef FEATURE_DIGITAL_ZOOM_SHORTCUT
    if (handle_digital_zoom_shortcut(event) == 0) return 0;
    #endif

So that it looks pretty and it's near the other features....

Is this correct? here is my gui-common.c: http://www.filedropper.com/gui-common

Thanks for all your help with this. It's really exciting to be able to change the behavior of my favorite camera!

[mrtorrent]

Ping pong. Is dpjpandone's solution correct? If so, can we get it committed and close out #2065? This is a pretty killer bug for movie restart.

Let me know if there's anything I can do to help.

Thanks!

[arrinkiiii]

WOW  !!!  Thanks dpjpandone  =)

[mrtorrent]

It's been a couple months, so just following up on this again - any progress? Anything else we can do? Thanks.

[ClarkLZeuss]

Checking in on this since it's been more than 2 months. Anything new? Wanted to mention something, if this helps at all: on my 7D, this bug with Movie Restart cannot be overridden by pressing the Play button while recording. I have to shut off the camera, and restart. At times, I've had to remove the battery when it didn't start back up. Thanks for your work everyone!