diff highlight.c @ 114:b6834e6359cf src

big libdvdnav cleanup, quoting the ChangeLog: * some bugfixes * code cleanup * build process polishing * more sensible event order in get_next_block to ensure useful event delivery * VOBU level resume * fixed: seeking in a multiangle feature briefly showed the wrong angle

--- a/highlight.c	Mon Jan 13 13:33:45 2003 +0000
+++ b/highlight.c	Thu Feb 20 15:32:21 2003 +0000
@@ -25,23 +25,25 @@
 #include "config.h"
 #endif
 
-
-#define BUTTON_TESTING
-
 #include <assert.h>
 
-#include <dvdnav.h>
 #include "dvdnav_internal.h"
 
 #include "vm.h"
 #include <dvdread/nav_types.h>
 
+/*
+#define BUTTON_TESTING
+*/
+
 #ifdef BUTTON_TESTING
+
 #include <dvdread/nav_print.h>
 #include "vmcmd.h"
 
 static void print_time(dvd_time_t *dtime) {
   const char *rate;
+
   assert((dtime->hour>>4) < 0xa && (dtime->hour&0xf) < 0xa);
   assert((dtime->minute>>4) < 0x7 && (dtime->minute&0xf) < 0xa);
   assert((dtime->second>>4) < 0x7 && (dtime->second&0xf) < 0xa);
@@ -69,18 +71,18 @@
 static void nav_print_PCI_GI(pci_gi_t *pci_gi) {
   int i;
 
-  fprintf(MSG_OUT,"pci_gi:\n");
-  fprintf(MSG_OUT,"nv_pck_lbn    0x%08x\n", pci_gi->nv_pck_lbn);
-  fprintf(MSG_OUT,"vobu_cat      0x%04x\n", pci_gi->vobu_cat);
-  fprintf(MSG_OUT,"vobu_uop_ctl  0x%08x\n", *(uint32_t*)&pci_gi->vobu_uop_ctl);
-  fprintf(MSG_OUT,"vobu_s_ptm    0x%08x\n", pci_gi->vobu_s_ptm);
-  fprintf(MSG_OUT,"vobu_e_ptm    0x%08x\n", pci_gi->vobu_e_ptm);
-  fprintf(MSG_OUT,"vobu_se_e_ptm 0x%08x\n", pci_gi->vobu_se_e_ptm);
-  fprintf(MSG_OUT,"e_eltm        ");
+  fprintf(MSG_OUT,"libdvdnav: pci_gi:\n");
+  fprintf(MSG_OUT,"libdvdnav: nv_pck_lbn    0x%08x\n", pci_gi->nv_pck_lbn);
+  fprintf(MSG_OUT,"libdvdnav: vobu_cat      0x%04x\n", pci_gi->vobu_cat);
+  fprintf(MSG_OUT,"libdvdnav: vobu_uop_ctl  0x%08x\n", *(uint32_t*)&pci_gi->vobu_uop_ctl);
+  fprintf(MSG_OUT,"libdvdnav: vobu_s_ptm    0x%08x\n", pci_gi->vobu_s_ptm);
+  fprintf(MSG_OUT,"libdvdnav: vobu_e_ptm    0x%08x\n", pci_gi->vobu_e_ptm);
+  fprintf(MSG_OUT,"libdvdnav: vobu_se_e_ptm 0x%08x\n", pci_gi->vobu_se_e_ptm);
+  fprintf(MSG_OUT,"libdvdnav: e_eltm        ");
   print_time(&pci_gi->e_eltm);
   fprintf(MSG_OUT,"\n");
 
-  fprintf(MSG_OUT,"vobu_isrc     \"");
+  fprintf(MSG_OUT,"libdvdnav: vobu_isrc     \"");
   for(i = 0; i < 32; i++) {
     char c = pci_gi->vobu_isrc[i];
     if((c >= ' ') && (c <= '~'))
@@ -99,10 +101,10 @@
   if(j == 0)
     return;
 
-  fprintf(MSG_OUT,"nsml_agli:\n");
+  fprintf(MSG_OUT,"libdvdnav: nsml_agli:\n");
   for(i = 0; i < 9; i++)
     if(nsml_agli->nsml_agl_dsta[i])
-      fprintf(MSG_OUT,"nsml_agl_c%d_dsta  0x%08x\n", i + 1,
+      fprintf(MSG_OUT,"libdvdnav: nsml_agl_c%d_dsta  0x%08x\n", i + 1,
              nsml_agli->nsml_agl_dsta[i]);
 }
 
@@ -111,24 +113,24 @@
   if((hl_gi->hli_ss & 0x03) == 0)
     return;
 
-  fprintf(MSG_OUT,"hl_gi:\n");
-  fprintf(MSG_OUT,"hli_ss        0x%01x\n", hl_gi->hli_ss & 0x03);
-  fprintf(MSG_OUT,"hli_s_ptm     0x%08x\n", hl_gi->hli_s_ptm);
-  fprintf(MSG_OUT,"hli_e_ptm     0x%08x\n", hl_gi->hli_e_ptm);
-  fprintf(MSG_OUT,"btn_se_e_ptm  0x%08x\n", hl_gi->btn_se_e_ptm);
+  fprintf(MSG_OUT,"libdvdnav: hl_gi:\n");
+  fprintf(MSG_OUT,"libdvdnav: hli_ss        0x%01x\n", hl_gi->hli_ss & 0x03);
+  fprintf(MSG_OUT,"libdvdnav: hli_s_ptm     0x%08x\n", hl_gi->hli_s_ptm);
+  fprintf(MSG_OUT,"libdvdnav: hli_e_ptm     0x%08x\n", hl_gi->hli_e_ptm);
+  fprintf(MSG_OUT,"libdvdnav: btn_se_e_ptm  0x%08x\n", hl_gi->btn_se_e_ptm);
 
   *btngr_ns = hl_gi->btngr_ns;
-  fprintf(MSG_OUT,"btngr_ns      %d\n",  hl_gi->btngr_ns);
-  fprintf(MSG_OUT,"btngr%d_dsp_ty    0x%02x\n", 1, hl_gi->btngr1_dsp_ty);
-  fprintf(MSG_OUT,"btngr%d_dsp_ty    0x%02x\n", 2, hl_gi->btngr2_dsp_ty);
-  fprintf(MSG_OUT,"btngr%d_dsp_ty    0x%02x\n", 3, hl_gi->btngr3_dsp_ty);
+  fprintf(MSG_OUT,"libdvdnav: btngr_ns      %d\n",  hl_gi->btngr_ns);
+  fprintf(MSG_OUT,"libdvdnav: btngr%d_dsp_ty    0x%02x\n", 1, hl_gi->btngr1_dsp_ty);
+  fprintf(MSG_OUT,"libdvdnav: btngr%d_dsp_ty    0x%02x\n", 2, hl_gi->btngr2_dsp_ty);
+  fprintf(MSG_OUT,"libdvdnav: btngr%d_dsp_ty    0x%02x\n", 3, hl_gi->btngr3_dsp_ty);
 
-  fprintf(MSG_OUT,"btn_ofn       %d\n", hl_gi->btn_ofn);
+  fprintf(MSG_OUT,"libdvdnav: btn_ofn       %d\n", hl_gi->btn_ofn);
   *btn_ns = hl_gi->btn_ns;
-  fprintf(MSG_OUT,"btn_ns        %d\n", hl_gi->btn_ns);
-  fprintf(MSG_OUT,"nsl_btn_ns    %d\n", hl_gi->nsl_btn_ns);
-  fprintf(MSG_OUT,"fosl_btnn     %d\n", hl_gi->fosl_btnn);
-  fprintf(MSG_OUT,"foac_btnn     %d\n", hl_gi->foac_btnn);
+  fprintf(MSG_OUT,"libdvdnav: btn_ns        %d\n", hl_gi->btn_ns);
+  fprintf(MSG_OUT,"libdvdnav: nsl_btn_ns    %d\n", hl_gi->nsl_btn_ns);
+  fprintf(MSG_OUT,"libdvdnav: fosl_btnn     %d\n", hl_gi->fosl_btnn);
+  fprintf(MSG_OUT,"libdvdnav: foac_btnn     %d\n", hl_gi->foac_btnn);
 }
 
 static void nav_print_BTN_COLIT(btn_colit_t *btn_colit) {
@@ -140,10 +142,10 @@
   if(j == 0)
     return;
 
-  fprintf(MSG_OUT,"btn_colit:\n");
+  fprintf(MSG_OUT,"libdvdnav: btn_colit:\n");
   for(i = 0; i < 3; i++)
     for(j = 0; j < 2; j++)
-      fprintf(MSG_OUT,"btn_cqoli %d  %s_coli:  %08x\n",
+      fprintf(MSG_OUT,"libdvdnav: btn_cqoli %d  %s_coli:  %08x\n",
              i, (j == 0) ? "sl" : "ac",
              btn_colit->btn_coli[i][j]);
 }
@@ -151,9 +153,9 @@
 static void nav_print_BTNIT(btni_t *btni_table, int btngr_ns, int btn_ns) {
   int i, j, k;
 
-  fprintf(MSG_OUT,"btnit:\n");
-  fprintf(MSG_OUT,"btngr_ns: %i\n", btngr_ns);
-  fprintf(MSG_OUT,"btn_ns: %i\n", btn_ns);
+  fprintf(MSG_OUT,"libdvdnav: btnit:\n");
+  fprintf(MSG_OUT,"libdvdnav: btngr_ns: %i\n", btngr_ns);
+  fprintf(MSG_OUT,"libdvdnav: btn_ns: %i\n", btn_ns);
 
   if(btngr_ns == 0)
     return;
@@ -163,22 +165,22 @@
       if(j < btn_ns) {
         btni_t *btni = &btni_table[(36 / btngr_ns) * i + j];
 
-        fprintf(MSG_OUT,"group %d btni %d:  ", i+1, j+1);
+        fprintf(MSG_OUT,"libdvdnav: group %d btni %d:  ", i+1, j+1);
         fprintf(MSG_OUT,"btn_coln %d, auto_action_mode %d\n",
                btni->btn_coln, btni->auto_action_mode);
-        fprintf(MSG_OUT,"coords   (%d, %d) .. (%d, %d)\n",
+        fprintf(MSG_OUT,"libdvdnav: coords   (%d, %d) .. (%d, %d)\n",
                btni->x_start, btni->y_start, btni->x_end, btni->y_end);
 
-        fprintf(MSG_OUT,"up %d, ", btni->up);
+        fprintf(MSG_OUT,"libdvdnav: up %d, ", btni->up);
         fprintf(MSG_OUT,"down %d, ", btni->down);
         fprintf(MSG_OUT,"left %d, ", btni->left);
         fprintf(MSG_OUT,"right %d\n", btni->right);
         for(k = 0; k < 8; k++) {
-          fprintf(MSG_OUT, "%02x ", btni->cmd.bytes[k]);
+          fprintf(MSG_OUT, "libdvdnav: %02x ", btni->cmd.bytes[k]);
         }
         fprintf(MSG_OUT, "| ");
         vmPrint_mnemonic(&btni->cmd);
-        fprintf(MSG_OUT, "\n\n");
+        fprintf(MSG_OUT, "\n");
       }
     }
   }
@@ -187,29 +189,30 @@
 static void nav_print_HLI(hli_t *hli) {
   int btngr_ns = 0, btn_ns = 0;
 
-  fprintf(MSG_OUT,"hli:\n");
+  fprintf(MSG_OUT,"libdvdnav: hli:\n");
   nav_print_HL_GI(&hli->hl_gi, & btngr_ns, & btn_ns);
   nav_print_BTN_COLIT(&hli->btn_colit);
   nav_print_BTNIT(hli->btnit, btngr_ns, btn_ns);
 }
 
 void nav_print_PCI(pci_t *pci) {
-  fprintf(MSG_OUT,"pci packet:\n");
+  fprintf(MSG_OUT,"libdvdnav: pci packet:\n");
   nav_print_PCI_GI(&pci->pci_gi);
   nav_print_NSML_AGLI(&pci->nsml_agli);
   nav_print_HLI(&pci->hli);
 }
 
+#endif
 
-#endif
 
 /* Highlighting API calls */
 
-
-
-dvdnav_status_t dvdnav_get_current_highlight(dvdnav_t *this, int* button) {
-  if(!this)
-   return S_ERR;
+dvdnav_status_t dvdnav_get_current_highlight(dvdnav_t *this, int *button) {
+  
+  if(!this) {
+    printerr("Passed a NULL pointer.");
+    return S_ERR;
+  }
 
   /* Simply return the appropriate value based on the SPRM */
   (*button) = (this->vm->state.HL_BTNN_REG) >> 10;
@@ -217,7 +220,7 @@
   return S_OK;
 }
 
-btni_t *__get_current_button(dvdnav_t *this, pci_t *pci) {
+static btni_t *get_current_button(dvdnav_t *this, pci_t *pci) {
   int button = 0;
 
   if(dvdnav_get_current_highlight(this, &button) != S_OK) {
@@ -231,36 +234,24 @@
   return &(pci->hli.btnit[button-1]);
 }
 
-dvdnav_status_t dvdnav_button_auto_action(dvdnav_t *this, pci_t *pci) {
-  btni_t *button_ptr;
-  
-  if(!this)
-   return S_ERR;
-
-  if((button_ptr = __get_current_button(this, pci)) == NULL) {
-    return S_ERR;
-  }
-  if (button_ptr->auto_action_mode == 1) {
-    return S_OK;
-  }
-  return S_ERR;
+static dvdnav_status_t button_auto_action(dvdnav_t *this, pci_t *pci) {
+  if (get_current_button(this, pci)->auto_action_mode)
+    return dvdnav_button_activate(this, pci);
 }
 
-
 dvdnav_status_t dvdnav_upper_button_select(dvdnav_t *this, pci_t *pci) {
   btni_t *button_ptr;
   
-  if(!this)
-   return S_ERR;
-
-  if((button_ptr = __get_current_button(this, pci)) == NULL) {
+  if(!this) {
+    printerr("Passed a NULL pointer.");
     return S_ERR;
   }
 
+  if(!(button_ptr = get_current_button(this, pci)))
+    return S_ERR;
+
   dvdnav_button_select(this, pci, button_ptr->up);
-  if (dvdnav_button_auto_action(this, pci) ) {
-    dvdnav_button_activate(this, pci);
-  }
+  button_auto_action(this, pci);
  
   return S_OK;
 }
@@ -268,17 +259,16 @@
 dvdnav_status_t dvdnav_lower_button_select(dvdnav_t *this, pci_t *pci) {
   btni_t *button_ptr;
   
-  if(!this)
-   return S_ERR;
-
-  if((button_ptr = __get_current_button(this, pci)) == NULL) {
+  if(!this) {
+    printerr("Passed a NULL pointer.");
     return S_ERR;
   }
 
+  if(!(button_ptr = get_current_button(this, pci)))
+    return S_ERR;
+
   dvdnav_button_select(this, pci, button_ptr->down);
-  if (dvdnav_button_auto_action(this, pci) ) {
-    dvdnav_button_activate(this, pci);
-  }
+  button_auto_action(this, pci);
   
   return S_OK;
 }
@@ -286,18 +276,16 @@
 dvdnav_status_t dvdnav_right_button_select(dvdnav_t *this, pci_t *pci) {
   btni_t *button_ptr;
   
-  if(!this)
-   return S_ERR;
-
-  if((button_ptr = __get_current_button(this, pci)) == NULL) {
-    printerr("Error fetching information on current button.");
+  if(!this) {
+    printerr("Passed a NULL pointer.");
     return S_ERR;
   }
 
+  if(!(button_ptr = get_current_button(this, pci)))
+    return S_ERR;
+
   dvdnav_button_select(this, pci, button_ptr->right);
-  if (dvdnav_button_auto_action(this, pci) ) {
-    dvdnav_button_activate(this, pci);
-  }
+  button_auto_action(this, pci);
   
   return S_OK;
 }
@@ -305,34 +293,31 @@
 dvdnav_status_t dvdnav_left_button_select(dvdnav_t *this, pci_t *pci) {
   btni_t *button_ptr;
   
-  if(!this)
-   return S_ERR;
-
-  if((button_ptr = __get_current_button(this, pci)) == NULL) {
+  if(!this) {
+    printerr("Passed a NULL pointer.");
     return S_ERR;
   }
 
+  if(!(button_ptr = get_current_button(this, pci)))
+    return S_ERR;
+
   dvdnav_button_select(this, pci, button_ptr->left);
-  if (dvdnav_button_auto_action(this, pci) ) {
-    dvdnav_button_activate(this, pci);
-  }
+  button_auto_action(this, pci);
   
   return S_OK;
 }
 
-dvdnav_status_t dvdnav_get_highlight_area(pci_t* nav_pci , int32_t button, int32_t mode, 
-                                           dvdnav_highlight_area_t* highlight) {
+dvdnav_status_t dvdnav_get_highlight_area(pci_t *nav_pci , int32_t button, int32_t mode, 
+					  dvdnav_highlight_area_t *highlight) {
   btni_t *button_ptr;
+
 #ifdef BUTTON_TESTING
   fprintf(MSG_OUT, "libdvdnav: Button get_highlight_area %i\n", button);
 #endif
 
-  /* Set the highlight SPRM if the passed button was valid*/
-  if((button <= 0) || (button > nav_pci->hli.hl_gi.btn_ns)) {
-    fprintf(MSG_OUT, "libdvdnav: Unable to select button number %i as it doesn't exist\n",
-              button);
+  if((button <= 0) || (button > nav_pci->hli.hl_gi.btn_ns))
     return S_ERR;
-  }
+
   button_ptr = &nav_pci->hli.btnit[button-1];
 
   highlight->sx = button_ptr->x_start;
@@ -347,7 +332,7 @@
   highlight->pts = nav_pci->hli.hl_gi.hli_s_ptm;
   highlight->buttonN = button;
 #ifdef BUTTON_TESTING
-  fprintf(MSG_OUT, "libdvdnav: highlight.c:Highlight area is (%u,%u)-(%u,%u), display = %i, button = %u\n",
+  fprintf(MSG_OUT, "libdvdnav: highlight: Highlight area is (%u,%u)-(%u,%u), display = %i, button = %u\n",
                button_ptr->x_start, button_ptr->y_start,
                button_ptr->x_end, button_ptr->y_end,
                1,
@@ -360,21 +345,20 @@
 dvdnav_status_t dvdnav_button_activate(dvdnav_t *this, pci_t *pci) {
   int button;
   btni_t *button_ptr = NULL;
-  
-  if(!this) 
-   return S_ERR;
+
+  if(!this) {
+    printerr("Passed a NULL pointer.");
+    return S_ERR;
+  }
+
   pthread_mutex_lock(&this->vm_lock); 
 
-  /* Precisely the same as selecting a button except we want
-   * a different palette */
   if(dvdnav_get_current_highlight(this, &button) != S_OK) {
-    pthread_mutex_unlock(&this->vm_lock); 
+    pthread_mutex_unlock(&this->vm_lock);
     return S_ERR;
   }
-/* FIXME: dvdnav_button_select should really return a
- * special case for explicit NO-BUTTONS.
- */
-  if(dvdnav_button_select(this, pci, button) != S_OK) {
+
+  if((button <= 0) || (button > pci->hli.hl_gi.btn_ns)) {
     /* Special code to handle still menus with no buttons.
      * the navigation is expected to report to the appicatino that a STILL is
      * underway. In turn, the application is supposed to report to the user
@@ -389,42 +373,46 @@
       vm_get_next_cell(this->vm);
       this->position_current.still = 0;
       pthread_mutex_unlock(&this->vm_lock);
+      /* clear error message */
+      printerr("");
       return S_OK;
     }
     pthread_mutex_unlock(&this->vm_lock); 
     return S_ERR;
   }
-  /* FIXME: The button command should really be passed in the API instead. */ 
-  button_ptr = __get_current_button(this, pci);
-  /* Finally, make the VM execute the appropriate code and
+  
+  button_ptr = get_current_button(this, pci);
+  /* Finally, make the VM execute the appropriate code and probably
    * scedule a jump */
 #ifdef BUTTON_TESTING
   fprintf(MSG_OUT, "libdvdnav: Evaluating Button Activation commands.\n");
 #endif
-  if(vm_eval_cmd(this->vm, &(button_ptr->cmd)) == 1) {
+  if(vm_exec_cmd(this->vm, &(button_ptr->cmd)) == 1) {
     /* Command caused a jump */
     this->vm->hop_channel++;
     this->position_current.still = 0;
   }
+  
   pthread_mutex_unlock(&this->vm_lock); 
   return S_OK;
 }
 
 dvdnav_status_t dvdnav_button_activate_cmd(dvdnav_t *this, int32_t button, vm_cmd_t *cmd)
 {
-  if(!this || !this->vm) 
+  if(!this) {
+    printerr("Passed a NULL pointer.");
     return S_ERR;
-  pthread_mutex_lock(&this->vm_lock); 
-  /* make the VM execute the appropriate code and
+  }
+  
+  pthread_mutex_lock(&this->vm_lock);
+  /* make the VM execute the appropriate code and probably
    * schedule a jump */
 #ifdef BUTTON_TESTING
-  fprintf(MSG_OUT, "libdvdnav:dvdnav_button_activate_cmd: Evaluating Button Activation commands.\n");
+  fprintf(MSG_OUT, "libdvdnav: dvdnav_button_activate_cmd: Evaluating Button Activation commands.\n");
 #endif
   if(button > 0) {
-    printerrf("Select button number %i\n ",
-	      button);
     this->vm->state.HL_BTNN_REG = (button << 10);
-    if( (vm_eval_cmd(this->vm, cmd)) == 1) {
+    if(vm_exec_cmd(this->vm, cmd) == 1) {
       /* Command caused a jump */
       this->vm->hop_channel++;
     }
@@ -438,26 +426,20 @@
 dvdnav_status_t dvdnav_button_select(dvdnav_t *this, pci_t *pci, int button) {
   
   if(!this) {
-   printerrf("Unable to select button number %i as this state bad",
-	      button);
-   return S_ERR;
+    printerr("Passed a NULL pointer.");
+    return S_ERR;
   }
  
 #ifdef BUTTON_TESTING
   fprintf(MSG_OUT, "libdvdnav: Button select %i\n", button); 
 #endif
   
-  /* Set the highlight SPRM if the passed button was valid*/
-  /* FIXME: this->pci should be provided by the application. */
   if((button <= 0) || (button > pci->hli.hl_gi.btn_ns)) {
-    printerrf("Unable to select button number %i as it doesn't exist",
-	      button);
+    printerr("Button does not exist.");
     return S_ERR;
   }
+  
   this->vm->state.HL_BTNN_REG = (button << 10);
-
-  this->hli_state = 1; /* Selected */
-
   this->position_current.button = -1; /* Force Highligh change */
 
   return S_OK;
@@ -466,11 +448,8 @@
 dvdnav_status_t dvdnav_button_select_and_activate(dvdnav_t *this, pci_t *pci, 
 						  int button) {
   /* A trivial function */
-  if(dvdnav_button_select(this, pci, button) != S_ERR) {
+  if(dvdnav_button_select(this, pci, button) != S_ERR)
     return dvdnav_button_activate(this, pci);
-  }
-  
-  /* Should never get here without an error */
   return S_ERR;
 }
 
@@ -479,55 +458,49 @@
   uint32_t best,dist;
   int mx,my,dx,dy,d;
 
-  /* FIXME: At the moment, the case of no button matchin (x,y) is
-   * silently ignored, is this OK? */
-  if(!this)
-   return S_ERR;
-
-  if(dvdnav_get_current_highlight(this, &cur_button) != S_OK) {
+  if(!this) {
+    printerr("Passed a NULL pointer.");
     return S_ERR;
   }
 
-  best = 0; 
+  if(dvdnav_get_current_highlight(this, &cur_button) != S_OK)
+    return S_ERR;
+
+  best = 0;
   dist = 0x08000000; /* >> than  (720*720)+(567*567); */
   
-  /* Loop through each button */
-  for(button=1; button <= pci->hli.hl_gi.btn_ns; button++) {
-    btni_t *button_ptr = NULL;
-    button_ptr = &(this->pci.hli.btnit[button-1]);
+  /* Loop through all buttons */
+  for(button = 1; button <= pci->hli.hl_gi.btn_ns; button++) {
+    btni_t *button_ptr = &(this->pci.hli.btnit[button-1]);
+    
     if((x >= button_ptr->x_start) && (x <= button_ptr->x_end) &&
        (y >= button_ptr->y_start) && (y <= button_ptr->y_end)) {
       mx = (button_ptr->x_start + button_ptr->x_end)/2;
-	  my = (button_ptr->y_start + button_ptr->y_end)/2;
+      my = (button_ptr->y_start + button_ptr->y_end)/2;
       dx = mx - x;
       dy = my - y;
       d = (dx*dx) + (dy*dy);
       /* If the mouse is within the button and the mouse is closer
        * to the center of this button then it is the best choice. */
       if(d < dist) {
-        dist = d; best=button;
+        dist = d;
+        best = button;
       }
     }
   }
-			  
-  if (best!=0) {
-    /* As an efficiency measure, only re-select the button
-     * if it is different to the previously selected one. */
-    if(best != cur_button) {
-      dvdnav_button_select(this, pci, best);
-    }
-  }
-  
-  return S_OK;
+
+  /* As an efficiency measure, only re-select the button
+   * if it is different to the previously selected one. */
+  if (best != 0 && best != cur_button)
+    dvdnav_button_select(this, pci, best);
+
+  /* return S_OK only if we actually found a matching button */
+  return best ? S_OK : S_ERR;
 }
 
 dvdnav_status_t dvdnav_mouse_activate(dvdnav_t *this, pci_t *pci, int x, int y) {
   /* A trivial function */
-  if(dvdnav_mouse_select(this, pci, x,y) != S_ERR) {
+  if(dvdnav_mouse_select(this, pci, x,y) != S_ERR)
     return dvdnav_button_activate(this, pci);
-  }
-  
-  /* Should never get here without an error */
   return S_ERR;
 }
-