Adding new cameras, applying patches into trunk (with source code prepared) - page 163 - General Discussion and Assistance - CHDK Forum
« previous next »
Pages: 1 ... 158 159 160 161 162 [163] 164 165 166 167 168 169   Go Down

Adding new cameras, applying patches into trunk (with source code prepared)

  • 1685 Replies
  • 886266 Views
*

Offline reyalp

  • ******
  • 14134
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1620 on: 20 / November / 2019, 22:25:03 »
Advertisements
Quote from: philmoz on 20 / November / 2019, 19:10:21
Alternate patch attached.
- Adds a dummy handler for the initial value of gui_mode.
- Remove the gui_get_mode() function in favour of camera_info.state.gui_mode
That seems OK to me. Tested booting to play and rec without issue on digic 4 (elph180) and digic 6 (sx710)

One thing I wondered is if there is code that relies on gui_mode == 0 being "none". The closest I found was script, which uses
Code: [Select]
camera_info.state.gui_mode != 0
for get_alt_mode(). This should not have any issue with the patch, since script shouldn't be able to run before gui_init. (I assume it doesn't use camera_info.state.gui_mode_alt because script is technically a different gui_mode, even though it has alt and non-alt states)
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1621 on: 20 / November / 2019, 23:16:47 »
Quote from: reyalp on 20 / November / 2019, 22:25:03
Quote from: philmoz on 20 / November / 2019, 19:10:21
Alternate patch attached.
- Adds a dummy handler for the initial value of gui_mode.
- Remove the gui_get_mode() function in favour of camera_info.state.gui_mode
That seems OK to me. Tested booting to play and rec without issue on digic 4 (elph180) and digic 6 (sx710)

One thing I wondered is if there is code that relies on gui_mode == 0 being "none". The closest I found was script, which uses
Code: [Select]
camera_info.state.gui_mode != 0
for get_alt_mode(). This should not have any issue with the patch, since script shouldn't be able to run before gui_init. (I assume it doesn't use camera_info.state.gui_mode_alt because script is technically a different gui_mode, even though it has alt and non-alt states)


The default value for camera_info.state.gui_mode is 0.
The new value I added is only used in the dummy handler for the init code that checks for mode change.
Anything that might use camera_info.state.gui_mode before gui_init is called should be unaffected (I think).

Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline reyalp

  • ******
  • 14134
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1622 on: 20 / November / 2019, 23:54:48 »
Quote from: philmoz on 20 / November / 2019, 23:16:47
Anything that might use camera_info.state.gui_mode before gui_init is called should be unaffected (I think).
Yes, my mistake, I was thinking GUI_MODE_STARTUP would appear in camera_info.state.gui_mode before init, but of course it won't.
Logged
Don't forget what the H stands for.
*

Offline srsa_4c

  • ******
  • 4451
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1623 on: 21 / November / 2019, 16:08:27 »
Quote from: philmoz on 20 / November / 2019, 19:10:21
Alternate patch attached.
Seems to work OK for me too.
Logged
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1624 on: 22 / November / 2019, 23:53:27 »
Quote from: philmoz on 20 / November / 2019, 19:10:21
Quote from: philmoz on 20 / November / 2019, 16:04:19
Quote from: reyalp on 20 / November / 2019, 15:23:42
Quote from: philmoz on 20 / November / 2019, 15:03:07
I can't see any other cases - am I missing anything?
Perhaps stating the obvious, but I assume the ones srsa_4c is hitting is the ones gui_set_mode where it references old_mode->... on 2474 and 2477
It seems like those could just be skipped if old_mode is null, though I'm not immediately sure whether redraw / restore stuff should be called on the first call.

gui_set_mode also returns old_mode, which would be NULL in this case, but the value isn't used in gui_init.


Good point, missed that case.


Setting a default for gui_mode makes more sense now - I'm not fully comfortable with the idea of using the alt mode handler though.
Perhaps a dummy handler struct for startup until gui_init is called might work.


Phil.


Alternate patch attached.
- Adds a dummy handler for the initial value of gui_mode.
- Remove the gui_get_mode() function in favour of camera_info.state.gui_mode


Phil.


Added to trunk in revision 5296.


Is it worth adding to the release-1_4 branch?

Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline reyalp

  • ******
  • 14134
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1625 on: 23 / November / 2019, 01:08:11 »
Quote from: philmoz on 22 / November / 2019, 23:53:27
Is it worth adding to the release-1_4 branch?
AFAIK was only needed because digic 7 crashes on NULL pointer reads, so shouldn't be required.

(I do hope to make 1.5 the stable soon. For reals...  :-[)
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1626 on: 11 / December / 2019, 22:52:33 »
Question on revision 5294 for Ixus 220.


This changed both code_gen.txt and boot.c for 1.00c, 1.01a and 1.01g; but only changed code_gen.txt for version 1.01c.


Noticed when doing a batch-rebuild-code-gen - boot.c for 1.01c gets changed.


Not sure if this was an oversight, or the boot.c code was correct and code_gen.txt should not have been updated.

Logged
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)
*

Offline reyalp

  • ******
  • 14134
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1627 on: 11 / December / 2019, 22:59:52 »
Quote from: philmoz on 11 / December / 2019, 22:52:33
Question on revision 5294 for Ixus 220.
This changed both code_gen.txt and boot.c for 1.00c, 1.01a and 1.01g; but only changed code_gen.txt for version 1.01c.
Thanks for catching that. I just missed running code gen on that sub, the MMIO should be the same for all. Checked in now.
Logged
Don't forget what the H stands for.
*

Offline srsa_4c

  • ******
  • 4451
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1628 on: 14 / December / 2019, 19:33:26 »
Found yet another case of a dereferenced null pointer.
Question is the usual, does the following fix look okay?
Code: [Select]
Index: core/shooting.c
===================================================================
--- core/shooting.c (revision 5331)
+++ core/shooting.c (working copy)
@@ -1053,7 +1053,7 @@
   int fl = get_focal_length(zoom_point);
   short f_focus_ok = shooting_get_focus_ok();
   short f_hyp_calc = 0, f_dist_calc = 0;
-  short min_av96_zoom_point = min_av96_zoom_point_tbl[zoom_point];
+  short min_av96_zoom_point = 0;
   short av96 = shooting_get_user_av96();
   short curr_av96 = shooting_get_current_av96();
   short prop_av96 = shooting_get_av96();
@@ -1063,8 +1063,10 @@
     min_av96_zoom_point_tbl = (short *) malloc(zoom_points * sizeof(short));
     if (min_av96_zoom_point_tbl) {
       memset(min_av96_zoom_point_tbl, 0, zoom_points * sizeof(short));
-      min_av96_zoom_point = 0;
     }
+    else {
+        return;
+    }
   } else min_av96_zoom_point = min_av96_zoom_point_tbl[zoom_point];
 
   if (min_av96_zoom_point==0 && shooting_in_progress()) {
Logged
*

Offline reyalp

  • ******
  • 14134
Re: Adding new cameras, applying patches into trunk (with source code prepared)
« Reply #1629 on: 14 / December / 2019, 20:26:21 »
Quote from: srsa_4c on 14 / December / 2019, 19:33:26
Found yet another case of a dereferenced null pointer.
Question is the usual, does the following fix look okay?
That existing code  :o :-X

Your fix looks OK to me.
Logged
Don't forget what the H stands for.
Pages: 1 ... 158 159 160 161 162 [163] 164 165 166 167 168 169   Go Up
« previous next »
 

Related Topics


SimplePortal © 2008-2014, SimplePortal