Hiding a menu entry based on another meny entry

[Audionut]

Card spanning in mlv_rec, somewhat depends on "Files > 4GiB (exFAT)".  Without exFAT enabled, spanned files contain the same filename as files created when 4GiB is reached.  See here.

There is relevant code already present in mlv_rec.

Code: [Select]

    for (struct menu_entry * e = raw_video_menu[0].children; !MENU_IS_EOL(e); e++)
    {
        /* customize menus for each camera here (e.g. hide what doesn't work) */
        if (!cam_eos_m && streq(e->name, "Rec Key") )
            e->shidden = 1;
        if (cam_eos_m && streq(e->name, "Digital dolly") )
            e->shidden = 1;

        if (!cam_5d3 && streq(e->name, "CF-only buffers") )
            e->shidden = 1;
        if (!cam_5d3 && streq(e->name, "Card spanning") )
            e->shidden = 1;
        if (!exFAT && streq(e->name, "Files > 4GiB (exFAT)") )
            e->shidden = 1;

        /* Memory hack confirmed to work only on 5D3 and 6D */
        if (streq(e->name, "Memory hack") && !(cam_5d3 || cam_6d))
        {
            e->shidden = 1;
            memory_hack = 0;
        }
    }
I can see how this code is working, but I don't understand how to create the same based from the status of another menu entry.  And I can't think of any other examples in the source to check.

[a1ex]

Hiding menus is only for those cases where the menu has no chance to work with your camera.

When something doesn't work because something else has to be configured in a certain way, I recommend using a MENU_WARNING_NOT_WORKING to gray out the menu item (or a .depends_on if available). There you can explain what else the user has to configure.

Note that graying out is done only on the user interface; it has no effect of the behavior of that code. So, right now, the checks have to be done both in menu (to gray out the option) and in the worker code (usually to make sure that option has no effect, or to print an error message).

When you gray out some setting because it depends on some other settings, I suggest doing so regardless of the current value of that setting. This will allow the user skip that menu entry quickly, without having to think about whether he wants it on or off (think of movie options disabled in photo mode).

Related: www.magiclantern.fm/forum/index.php?topic=10422

[Audionut]

Thanks a1ex.  I'll attempt to accomplish this for piece of mind (practice).

However, it sounds quirky, and a cleaner solution IMO, is to simply ensure that spanned filenames are different.  I'm browsing the source, but as with most things that seem easy, it isn't.

[a1ex]

I'd say the simplest thing would be to change the first letter from the file name (the M). Maybe 'S' for the files on the SD card?

[Audionut]

I think I am on the right track, but I have a funny feeling that this isn't being specific enough. 
It's simply saying, if card spanning, use this filename instead.  Rather then, use this filename for spanned files.

Code: [Select]

diff -r f3b2d51d430c modules/mlv_rec/mlv_rec.c
--- a/modules/mlv_rec/mlv_rec.c Wed May 28 17:50:04 2014 +0300
+++ b/modules/mlv_rec/mlv_rec.c Wed May 28 19:47:58 2014 +0300
@@ -2150,6 +2150,11 @@
          * last field gets incremented if there's another video with the same name
          */
         snprintf(videoname, sizeof(videoname), "M%02d-%02d%02d", now.tm_mday, now.tm_hour, COERCE(now.tm_min + number, 0, 99));
+
+        else if(card_spanning)
+        {
+            snprintf(videoname, sizeof(videoname), "S%02d-%02d%02d", now.tm_mday, now.tm_hour, COERCE(now.tm_min + number, 0, 99));
+        }
         
         if(create_dirs)
         {


What about this?

Code: [Select]

--- a/modules/mlv_rec/mlv_rec.c Wed May 28 17:50:04 2014 +0300
+++ b/modules/mlv_rec/mlv_rec.c Wed May 28 19:47:58 2014 +0300
@@ -2150,6 +2150,11 @@
          * last field gets incremented if there's another video with the same name
          */
         snprintf(videoname, sizeof(videoname), "M%02d-%02d%02d", now.tm_mday, now.tm_hour, COERCE(now.tm_min + number, 0, 99));
+
+        else if(writer == 1)
+        {
+            snprintf(videoname, sizeof(videoname), "S%02d-%02d%02d", now.tm_mday, now.tm_hour, COERCE(now.tm_min + number, 0, 99));
+        }
         
         if(create_dirs)
         {
edit:  nope, the bottom example doesn't compile, and I need to remove the else.

[a1ex]

From what I understand, get_next_raw_movie_file_name() will return the base name for all the linked streams.

It looks like the real action is happening in mlv_rec_precreate_files. That is, from the base name, it will create:

A:/Mdd-hhmm.MLV, A:/Mdd-hhmm.M00, A:/Mdd-hhmm.M01, in total, "count" files
B:/Mdd-hhmm.M00, B:/Mdd-hhmm.M01, in total, "count"-1 files

Now, if you rename the SD ones as "Sdd-hhmm" (or maybe Mdd-hhmm.S00), you will also need to update the converters, so they will know how to match the files. With the current scheme, the trick is that you don't know in advance how many files you are going to be saved, so you can't get a continuous numbering for easy identification.

Another possibility would be to keep the old naming, but rename all the files at the end (after recording stopped). This way, you could just copy-paste the files in the same folder, and the existing converters would recognize them (I guess, didn't try).

Better ask g3gg0 for this one.

[Marsu42]

Note that graying out is done only on the user interface; it has no effect of the behavior of that code. So, right now, the checks have to be done both in menu (to gray out the option) and in the worker code (usually to make sure that option has no effect, or to print an error message).

+1 for checking the latter, it often happens to me that a feature or part of it still does something even if it has been deactivated in the menu, so don't forget to double-check if individual menu options are on/off and then if the whole main menu item has been disabled :-p

[a1ex]

I'm thinking at some backend where you would write the dependency checks only once, so you would have a variable to display in the menu (that you can toggle), and another variable to use in the worker code.

One example is global draw: you turn it on, but it has a bunch of prerequisites: basically it's only active when you are in regular LiveView mode, without other Canon dialogs. So, in the config entry you have just global_draw, and in the overlay code that checks whether it should draw, it uses get_global_draw(), which checks the menu setting and the internal conditions.

However, having a getter for every single setting is a bit overkill and you will be tempted to use the variable directly (bypassing the checks). This could maybe work if menus would be linked directly to config entries (structures), instead of variables, and the config API would prevent you from accessing the variable directly in user code, forcing you to use getters/setters.

[Marsu42]

I'm thinking at some backend where you would write the dependency checks only once, so you would have a variable to display in the menu (that you can toggle), and another variable to use in the worker code.

Well, at the end of the day imho the current way is working rather fine, it's not like ML is the firmware for the next mission to Mars. There are not that many inter-dependencies (that would come to my mind), and you just have to be sure to check for enabled/disabled features in the code. Much more important to get shiny new features that actually improve the photography result. Or save the camera's shutter. Like full res silent dng :->