Patch for Issue 485 and some code clean up

Discuss issues related to PS3 Media Server development (only for programmers)

Patch for Issue 485 and some code clean up

Postby spots » Wed May 25, 2011 4:02 pm

Below is the patch for issue 485.
As part of this patch I also cleaned up some of the warnings given by Eclipse, made some minor code improvements, and started the Logger refactor to remove the wrapped methods as per a TODO note.
On that note, it would probably be good to look at moving the Logger invocation to be on a final static private class by class basis. This seems to be the standard way to go.

Of particular note, I removed the ChannelPipelineCoverage("one") annotation which was deprecated. I was able to do this since the new Sharable annotation should only be used when one wants to share an object. However the ChannelPipelineCoverage("one") annotation means you must create a new instance of the annotated handler type for each new channel. Thus my interpretation was that the code could simply be removed.

Code: Select all
Index: pms/dlna/RootFolder.java
===================================================================
--- pms/dlna/RootFolder.java   (revision 594)
+++ pms/dlna/RootFolder.java   (working copy)
@@ -170,7 +170,7 @@
       boolean refreshed = false;
       File files[] = null;
       try {
-         files = PMS.get().loadFoldersConf(PMS.getConfiguration().getFolders());
+         files = PMS.get().loadFoldersConf(PMS.getConfiguration().getFolders(), true);
          if (files == null || files.length == 0)
             files = File.listRoots();
          int i = 0;
Index: pms/network/Request.java
===================================================================
--- pms/network/Request.java   (revision 594)
+++ pms/network/Request.java   (working copy)
@@ -22,7 +22,6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.net.InetAddress;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
Index: pms/network/RequestHandlerV2.java
===================================================================
--- pms/network/RequestHandlerV2.java   (revision 594)
+++ pms/network/RequestHandlerV2.java   (working copy)
@@ -34,7 +34,6 @@
 import org.jboss.netty.channel.Channel;
 import org.jboss.netty.channel.ChannelFutureListener;
 import org.jboss.netty.channel.ChannelHandlerContext;
-import org.jboss.netty.channel.ChannelPipelineCoverage;
 import org.jboss.netty.channel.ChannelStateEvent;
 import org.jboss.netty.channel.ExceptionEvent;
 import org.jboss.netty.channel.MessageEvent;
@@ -48,7 +47,6 @@
 import org.jboss.netty.handler.codec.http.HttpResponseStatus;
 import org.jboss.netty.handler.codec.http.HttpVersion;
 
-@ChannelPipelineCoverage("one")
 public class RequestHandlerV2 extends SimpleChannelUpstreamHandler {
 
    private volatile HttpRequest nettyRequest;
@@ -166,8 +164,8 @@
             PMS.get().setRendererfound(request.getMediaRenderer());
          }
       }
-      if (nettyRequest.getContentLength() > 0) {
-         byte data[] = new byte[(int) nettyRequest.getContentLength()];
+      if (HttpHeaders.getContentLength(nettyRequest) > 0) {
+         byte data[] = new byte[(int) HttpHeaders.getContentLength(nettyRequest)];
          ChannelBuffer content = nettyRequest.getContent();
          content.readBytes(data);
          request.setTextContent(new String(data, "UTF-8"));
Index: pms/network/RequestV2.java
===================================================================
--- pms/network/RequestV2.java   (revision 594)
+++ pms/network/RequestV2.java   (working copy)
@@ -20,7 +20,6 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.InetAddress;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
Index: pms/PMS.java
===================================================================
--- pms/PMS.java   (revision 594)
+++ pms/PMS.java   (working copy)
@@ -120,6 +120,11 @@
 
 public class PMS {
 
+   private static final String SCROLLBARS = "scrollbars"; //$NON-NLS-1$
+   private static final String NATIVELOOK = "nativelook"; //$NON-NLS-1$
+   private static final String CONSOLE = "console"; //$NON-NLS-1$
+   private static final String NOCONSOLE = "noconsole"; //$NON-NLS-1$
+   
    /**
     * Update URL used in the {@link AutoUpdater}.
     */
@@ -129,6 +134,7 @@
     */
    public static final String VERSION = "1.22.0"; //$NON-NLS-1$
    public static final String AVS_SEPARATOR = "\1"; //$NON-NLS-1$
+   
    // (innot): The logger used for all logging.
    public static final Logger logger = LoggerFactory.getLogger(PMS.class);
    // TODO(tcox):  This shouldn't be static
@@ -309,7 +315,7 @@
          return true;
       } catch (Exception e) {
          if (error) {
-            error("Cannot launch " + name + " / Check the presence of " + params[0] + " ...", e); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+            logger.error("Cannot launch " + name + " / Check the presence of " + params[0] + " ...", e); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
          }
          return false;
       }
@@ -356,7 +362,7 @@
       registry = new WinUtils();
 
       AutoUpdater autoUpdater = new AutoUpdater(UPDATE_SERVER_URL, VERSION);
-      if (System.getProperty("console") == null) {//$NON-NLS-1$
+      if (System.getProperty(CONSOLE) == null) {//$NON-NLS-1$
          frame = new LooksFrame(autoUpdater, configuration);
          autoUpdater.pollServer();
       } else {
@@ -462,7 +468,7 @@
       try {
          ExternalFactory.lookup();
       } catch (Exception e) {
-         error("Error loading plugins", e);
+         logger.error("Error loading plugins", e);
       }
 
       registerPlayers();
@@ -547,7 +553,6 @@
 
       return true;
    }
-   private static final String PMSDIR = "\\PMS\\";
 
    /**Creates a new Root folder for a given configuration. It adds following folders in this order:
     * <ol><li>Directory folders as stated in the configuration pane
@@ -562,9 +567,7 @@
     * @throws IOException
     */
    public void manageRoot(RendererConfiguration renderer) throws IOException {
-      String webConfPath;
-
-      File files[] = loadFoldersConf(configuration.getFolders());
+      File files[] = loadFoldersConf(configuration.getFolders(), true);
       if (files == null || files.length == 0) {
          files = File.listRoots();
       }
@@ -711,7 +714,7 @@
                minimal("iPhoto folder not found !?");
             }
          } catch (Exception e) {
-            error("Something went wrong with the iPhoto Library scan: ", e); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+            logger.error("Something went wrong with the iPhoto Library scan: ", e); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
          }
       }
    }
@@ -759,7 +762,7 @@
             location = location + "\\iTunes\\iTunes Music Library.xml";
             iTunesFile = location;
          } else {
-            minimal("Could not find the My Music folder");
+            logger.info("Could not find the My Music folder");
          }
       }
 
@@ -810,10 +813,10 @@
                }
                getRootFolder(renderer).addChild(vf);
             } else {
-               minimal("Could not find the iTunes file");
+               logger.info("Could not find the iTunes file");
             }
          } catch (Exception e) {
-            error("Something went wrong with the iTunes Library scan: ", e); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+            logger.error("Something went wrong with the iTunes Library scan: ", e); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
          }
       }
    }
@@ -1099,7 +1102,7 @@
       } else {
          if (isWindows()) {
             if (p.executable() == null) {
-               minimal("Executable of transcoder profile " + p + " not found"); //$NON-NLS-1$ //$NON-NLS-2$
+               logger.info("Executable of transcoder profile " + p + " not found"); //$NON-NLS-1$ //$NON-NLS-2$
                return;
             }
             File executable = new File(p.executable());
@@ -1108,7 +1111,7 @@
             if (executable.exists() || executable2.exists()) {
                ok = true;
             } else {
-               minimal("Executable of transcoder profile " + p + " not found"); //$NON-NLS-1$ //$NON-NLS-2$
+               logger.info("Executable of transcoder profile " + p + " not found"); //$NON-NLS-1$ //$NON-NLS-2$
                return;
             }
             if (p.avisynth()) {
@@ -1116,7 +1119,7 @@
                if (registry.isAvis()) {
                   ok = true;
                } else {
-                  minimal("Transcoder profile " + p + " will not be used because AviSynth was not found"); //$NON-NLS-1$ //$NON-NLS-2$
+                  logger.info("Transcoder profile " + p + " will not be used because AviSynth was not found"); //$NON-NLS-1$ //$NON-NLS-2$
                }
             }
          } else if (!p.avisynth()) {
@@ -1124,22 +1127,19 @@
          }
       }
       if (ok) {
-         minimal("Registering transcoding engine: " + p /*+ (p.avisynth()?(" with " + (forceMPlayer?"MPlayer":"AviSynth")):"")*/); //$NON-NLS-1$
+         logger.info("Registering transcoding engine: " + p /*+ (p.avisynth()?(" with " + (forceMPlayer?"MPlayer":"AviSynth")):"")*/); //$NON-NLS-1$
          players.add(p);
       }
    }
 
    /**Transforms a comma separated list of directory entries into an array of {@link String}.
-    * The function checks that the directory exists and is a valid directory.
+    * Checks that the directory exists and is a valid directory.
     * @param folders {@link String} Comma separated list of directories.
+    * @param log whether to output log information
     * @return {@link File}[] Array of directories.
     * @throws IOException
     * @see {@link PMS#manageRoot(RendererConfiguration)}
     */
-   public File[] loadFoldersConf(String folders) throws IOException {
-      return loadFoldersConf(folders, true);
-   }
-
    // this is called *way* too often (e.g. a dozen times with 1 renderer and 1 shared folder),
    // so log it by default so we can fix it.
    // BUT it's also called when the GUI is initialized (to populate the list of shared folders),
@@ -1160,14 +1160,14 @@
          }
          File file = new File(folder);
          if (file.exists()) {
-            if (file.isDirectory()) {
-               directories.add(file);
-            } else {
-               error("File " + folder + " is not a directory!", null); //$NON-NLS-1$ //$NON-NLS-2$
+            if (!file.isDirectory()) {
+               logger.warn("The file " + folder + " is not a directory! Please remove it from your Shared folders list on the Navigation/Share Settings tab"); //$NON-NLS-1$ //$NON-NLS-2$
             }
          } else {
-            error("The directory " + folder + " does not exist. Please remove it from your Shared folders list on the Navigation/Share Settings tab", null); //$NON-NLS-1$ //$NON-NLS-2$
+            logger.warn("The directory " + folder + " does not exist. Please remove it from your Shared folders list on the Navigation/Share Settings tab"); //$NON-NLS-1$ //$NON-NLS-2$
          }
+         // add the file even if there are problems so that the user can update the shared folders as required.
+         directories.add(file);
       }
       File f[] = new File[directories.size()];
       directories.toArray(f);
@@ -1209,18 +1209,14 @@
     * @param msg {@link String} to be added to the debug stream.
     */
    public static void debug(String msg) {
-      if (logger != null) {
-         logger.trace(msg);
-      }
+      logger.trace(msg);
    }
 
    /**Adds a message to the info stream.
     * @param msg {@link String} to be added to the info stream.
     */
    public static void info(String msg) {
-      if (logger != null) {
-         logger.debug(msg);
-      }
+      logger.debug(msg);
    }
 
    /**Adds a message to the minimal stream. This stream is also
@@ -1228,9 +1224,7 @@
     * @param msg {@link String} to be added to the minimal stream.
     */
    public static void minimal(String msg) {
-      if (logger != null) {
-         logger.info(msg);
-      }
+      logger.info(msg);
    }
 
    /**Adds a message to the error stream. This is usually called by
@@ -1239,10 +1233,9 @@
     * @param t {@link Throwable} comes from an {@link Exception}
     */
    public static void error(String msg, Throwable t) {
-      if (logger != null) {
-         logger.error(msg, t);
-      }
+      logger.error(msg, t);
    }
+   
    /**Universally Unique Identifier used in the UPnP server.
     *
     */
@@ -1272,11 +1265,11 @@
                   uuid = UUID.nameUUIDFromBytes(addr).toString();
                   uuidBasedOnMAC = true;
                } else {
-                  minimal("Unable to retrieve MAC address for UUID creation: using a random one..."); //$NON-NLS-1$
+                  logger.info("Unable to retrieve MAC address for UUID creation: using a random one..."); //$NON-NLS-1$
                }
             }
          } catch (Throwable e) {
-            minimal("Switching to random UUID cause there's an error in getting UUID from MAC address: " + e.getMessage()); //$NON-NLS-1$
+            logger.info("Switching to random UUID cause there's an error in getting UUID from MAC address: " + e.getMessage()); //$NON-NLS-1$
          }
 
          if (!uuidBasedOnMAC) {
@@ -1286,7 +1279,7 @@
                uuid = UUID.randomUUID().toString();
             }
          }
-         minimal("Using the following UUID: " + uuid); //$NON-NLS-1$
+         logger.info("Using the following UUID: " + uuid); //$NON-NLS-1$
       }
       return "uuid:" + uuid; //$NON-NLS-1$ //$NON-NLS-2$
       //return "uuid:1234567890TOTO::";
@@ -1319,9 +1312,9 @@
                instance = new PMS();
                try {
                   if (instance.init()) {
-                     minimal("The server should now appear on your renderer"); //$NON-NLS-1$
+                     logger.info("The server should now appear on your renderer"); //$NON-NLS-1$
                   } else {
-                     minimal("A serious error occurred"); //$NON-NLS-1$
+                     logger.error("A serious error occurred during PMS init"); //$NON-NLS-1$
                   }
                } catch (Exception e) {
                   e.printStackTrace();
@@ -1369,32 +1362,32 @@
    public static void main(String args[]) throws IOException, ConfigurationException {
       if (args.length > 0) {
          for (int a = 0; a < args.length; a++) {
-            if (args[a].equals("console")) //$NON-NLS-1$
+            if (args[a].equals(CONSOLE))
             {
-               System.setProperty("console", "true"); //$NON-NLS-1$ //$NON-NLS-2$
-            } else if (args[a].equals("nativelook")) //$NON-NLS-1$
+               System.setProperty(CONSOLE, Boolean.toString(true));
+            } else if (args[a].equals(NATIVELOOK))
             {
-               System.setProperty("nativelook", "true"); //$NON-NLS-1$ //$NON-NLS-2$
-            } else if (args[a].equals("scrollbars")) //$NON-NLS-1$
+               System.setProperty(NATIVELOOK, Boolean.toString(true));
+            } else if (args[a].equals(SCROLLBARS))
             {
-               System.setProperty("scrollbars", "true"); //$NON-NLS-1$ //$NON-NLS-2$
-            } else if (args[a].equals("noconsole")) //$NON-NLS-1$
+               System.setProperty(SCROLLBARS, Boolean.toString(true));
+            } else if (args[a].equals(NOCONSOLE))
             {
-               System.setProperty("noconsole", "true"); //$NON-NLS-1$ //$NON-NLS-2$
+               System.setProperty(NOCONSOLE, Boolean.toString(true));
             }
          }
       }
       try {
          Toolkit.getDefaultToolkit();
-         if (GraphicsEnvironment.isHeadless() && System.getProperty("noconsole") == null) //$NON-NLS-1$
+         if (GraphicsEnvironment.isHeadless() && System.getProperty(NOCONSOLE) == null)
          {
-            System.setProperty("console", "true"); //$NON-NLS-1$ //$NON-NLS-2$
+            System.setProperty(CONSOLE, Boolean.toString(true));
          }
       } catch (Throwable t) {
-         System.err.println("Toolkit error: " + t.getMessage()); //$NON-NLS-1$
-         if (System.getProperty("noconsole") == null) //$NON-NLS-1$
+         System.err.println("Toolkit error: " + t.getMessage());
+         if (System.getProperty(NOCONSOLE) == null)
          {
-            System.setProperty("console", "true"); //$NON-NLS-1$ //$NON-NLS-2$
+            System.setProperty(CONSOLE, Boolean.toString(true));
          }
       }
 
@@ -1429,7 +1422,7 @@
       try {
          configuration.save();
       } catch (ConfigurationException e) {
-         error("Could not save configuration", e); //$NON-NLS-1$
+         logger.error("Could not save configuration", e); //$NON-NLS-1$
       }
    }
 


If there are any questions about the patch, please let me know.

Cheers.
spots
 
Posts: 2
Joined: Thu May 19, 2011 6:04 am

Re: Patch for Issue 485 and some code clean up

Postby SubJunk » Mon May 30, 2011 2:03 am

Thanks for this :)
Will try it in the next SubJunk Build and if it works well it will go into the official version.
SubJunk
 
Posts: 1210
Joined: Fri Mar 27, 2009 5:25 am


Return to Developers

Who is online

Users browsing this forum: Yahoo [Bot] and 3 guests