Sending compressed JPG image over socket

Question

I am trying to create a program with Java which takes a screenshot of the user's screen, compresses the image, and sends it to a server over sockets. For some reason the image is saved corrupted in the end (it's unreadable). Can you maybe help me find what the problem could be?

CLIENT: (the screenshot is entered as a BufferedImage, and the returned byte array is then returned to the second function which sends it to the server)

    public static byte[] compressImage(BufferedImage image) throws IOException {
    System.out.println("starting compression");

    ByteArrayOutputStream os = new ByteArrayOutputStream(37628);

    float quality = 0.16f;

    // create a BufferedImage as the result of decoding the supplied InputStream

    // get all image writers for JPG format
    Iterator<ImageWriter> writers = ImageIO.getImageWritersByFormatName("jpg");
    //Iterator iter = ImageIO.getImageWritersByFormatName("jpeg");

    if (!writers.hasNext())
        throw new IllegalStateException("No writers found");

    ImageWriter writer = (ImageWriter) writers.next();
    ImageOutputStream ios = ImageIO.createImageOutputStream(os);
    writer.setOutput(ios);

    ImageWriteParam param = writer.getDefaultWriteParam();

    // compress to a given quality
    param.setCompressionMode(ImageWriteParam.MODE_EXPLICIT);
    param.setCompressionQuality(quality);

    // appends a complete image stream containing a single image and
    //associated stream and image metadata and thumbnails to the output
    writer.write(null, new IIOImage(image, null, null), param);
    os.flush();

    return os.toByteArray();
}

    public void uploadShot(byte[] imgData, String nickname) {

    try {
        /* Try to connect to the server on localhost, port 5555 */

        Socket sk = new Socket("localhost", 23232);
        OutputStream output = sk.getOutputStream();

        /* Send filename to server */

        OutputStreamWriter outputStream = new OutputStreamWriter(sk.getOutputStream());
        outputStream.write(nickname + "\n");
        outputStream.flush();

        /* Get response from server */

        BufferedReader inReader = new BufferedReader(new InputStreamReader(sk.getInputStream()));

        String serverStatus = inReader.readLine(); // Read the first line

        /* If server is ready, send the file */

        if (serverStatus.equals("READY")){
            int len = imgData.length;
            int start = 0;

            if (len < 0)
                throw new IllegalArgumentException("Negative length not allowed");
            if (start < 0 || start >= imgData.length)
                throw new IndexOutOfBoundsException("Out of bounds: " + start);
            // Other checks if needed.

            // May be better to save the streams in the support class;
            // just like the socket variable.
            OutputStream out = sk.getOutputStream(); 
            DataOutputStream dos = new DataOutputStream(out);

            dos.writeInt(len);
            if (len > 0) {
                dos.write(imgData, start, len);
            }
            dos.close();
            output.close();
            sk.close();

            System.out.println("Transfer complete.");
        }
    } catch (Exception ex){
        /* Catch any errors */
        System.out.println(ex.getMessage());
    }
}

SERVER: (the received image is saved to the folder mentioned with a timestamp)

public static void main(String args[]) throws Exception{
    System.out.println("Server running...");

    /* Listen on port 5555 */

    ServerSocket server = new ServerSocket(23232);

    /* Accept the sk */

    Socket sk = server.accept();

    System.out.println("Server accepted client");
    InputStream input = sk.getInputStream();
    BufferedReader inReader = new BufferedReader(new InputStreamReader(sk.getInputStream()));
    BufferedWriter outReader = new BufferedWriter(new OutputStreamWriter(sk.getOutputStream()));

    /* Read the filename */
    String nickname = inReader.readLine();

    if ( !nickname.equals("") ){

        /* Reply back to client with READY status */

        outReader.write("READY\n");
        outReader.flush();
    }


    String current = "/home/kasgel/screenshots";

    DateFormat dateFormat = new SimpleDateFormat("dd-MM-yyyy__HH:mm:ss");
    Date timestamp = new Date();
    File filename = new File(current + "/" + nickname + "-" + dateFormat.format(timestamp) + ".jpg");
    if (!filename.exists()) {
        filename.createNewFile();
    }
    FileOutputStream wr = new FileOutputStream(filename);

    byte[] buffer = new byte[sk.getReceiveBufferSize()];

    int bytesReceived = 0;

    while((bytesReceived = input.read(buffer))>0) {
        wr.write(buffer,0,bytesReceived);
    }
    wr.close();
}

And the error message which I get when opening the saved screenshot is the following: display.im6: Not a JPEG file: starts with 0x00 0x03 `MyNick-30-03-2015__19:27:58.jpg' @ error/jpeg.c/JPEGErrorHandler/316.

Answer 1

When you're writing your image, you first write a 32-bit signed integer containing the length in bytes of the image:

        dos.writeInt(len);
        if (len > 0) {
            dos.write(imgData, start, len);
        }

But when you're reading the image back, you don't read the length first; you're reading all the data (including the length) as if they were part of the image.

You have a second problem though, that will by itself also cause this problem. When you create a BufferedReader, and call readLine on it, it will read beyond the newline - it will read until its buffer is full. Which is not a problem if you keep reading from it, but after reading the line, you continue reading from the underlying InputStream, which will often have more bytes consumed from it after the newline.

The solution is: only use one abstraction to read/write data. In this case, the easiest is absolutely to use DataOutputStream and DataInputStream. Write the file name using writeUTF and read it back using readUTF. Write the length of the file with writeInt and read it back with readInt. Write the data with write and read it with read - and make sure to read only as many bytes as you received from the readInt call. And most important of all, keep using the same DataOutputStream and DataInputStream instances; don't constructor buffered readers and input-streams on the same underlying InputStream