72

I am having problems downloading a binary file (video) in my app from the internet. In Quicktime, If I download it directly it works fine but through my app somehow it get's messed up (even though they look exactly the same in a text editor). Here is a example:

    URL u = new URL("http://www.path.to/a.mp4?video");
    HttpURLConnection c = (HttpURLConnection) u.openConnection();
    c.setRequestMethod("GET");
    c.setDoOutput(true);
    c.connect();
    FileOutputStream f = new FileOutputStream(new File(root,"Video.mp4"));


    InputStream in = c.getInputStream();

    byte[] buffer = new byte[1024];
    int len1 = 0;
    while ( (len1 = in.read(buffer)) > 0 ) {
         f.write(buffer);
    }
    f.close();
Isaac Waller
  • 32,709
  • 29
  • 96
  • 107

6 Answers6

93

I don't know if it's the only problem, but you've got a classic Java glitch in there: You're not counting on the fact that read() is always allowed to return fewer bytes than you ask for. Thus, your read could get less than 1024 bytes but your write always writes out exactly 1024 bytes possibly including bytes from the previous loop iteration.

Correct with:

 while ( (len1 = in.read(buffer)) > 0 ) {
         f.write(buffer,0, len1);
 }

Perhaps the higher latency networking or smaller packet sizes of 3G on Android are exacerbating the effect?

Ry4an Brase
  • 78,112
  • 7
  • 148
  • 169
  • 4
    What a stupid mistake... thanks! This is what happens when you don't read the tutorial properly :) – Isaac Waller Feb 23 '09 at 05:03
  • What about initializing the buffer? What about protecting against exception? What about releasing the resources? I think it is a good but not a complete answer. There are other more complete answers here. – AlikElzin-kilaka Jun 23 '11 at 17:04
  • I'm having the same issue.. but in my case even this solution makes no difference. I'm still getting a 46k file for a 430k image. – lavelle Jul 15 '11 at 09:41
  • 3
    I would like to point out that the > 0 test can prematurely end the reading. The Documentation says that -1 is returned at the end of the stream. – Clint Nov 18 '11 at 21:05
  • 2
    @Clint : true, but the documentation also says (as of java 5), that 0 cannot be returned unless the 'len' param is 0 (If no byte is available (...) -1 is returned; otherwise, at least one byte is read). In java 2, 0 can be returned. – njzk2 Feb 27 '12 at 15:47
  • Wrapping the `InputStream` into a `BufferedInputStream`, and the `FileOutputStream` into a `BufferedOutputStream` would make your life much easier. – NickPro May 06 '14 at 18:06
  • Buffering wrappers is certainly a good idea for any network IO, but they don't fix the problem -- they're still allowed to return fewer bytes than requested too. – Ry4an Brase May 07 '14 at 18:24
  • @Clint So how exactly can the > 0 test prematurely end reading? Be clear. – user207421 Jun 20 '17 at 03:37
  • @ndk Zero can only be returned when the buffer is zero length or a zero count is supplied. Including Java 1 and 2. – user207421 Jun 20 '17 at 03:38
  • 1
    Did you know you were Jesus? You just saved me from hell :) – Mehdi Haghgoo Dec 24 '17 at 13:52
29
new DefaultHttpClient().execute(new HttpGet("http://www.path.to/a.mp4?video"))
        .getEntity().writeTo(
                new FileOutputStream(new File(root,"Video.mp4")));
njzk2
  • 38,969
  • 7
  • 69
  • 107
16

One problem is your reading of the buffer. If every read of the input stream is not an exact multiple of 1024 you will copy bad data. Use:

byte[] buffer = new byte[1024];
int len1 = 0;
while ( (len1 = in.read(buffer)) != -1 ) {
  f.write(buffer,0, len1);
}
Clint
  • 8,988
  • 1
  • 26
  • 40
14
 public class download extends Activity {

     private static String fileName = "file.3gp";
     private static final String MY_URL = "Your download url goes here";

     @Override
     public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);

        try {
            URL url = new URL(MY_URL);
            HttpURLConnection c = (HttpURLConnection) url.openConnection();
            c.setRequestMethod("GET");
            c.setDoOutput(true);
            c.connect();

            String PATH = Environment.getExternalStorageDirectory()
                + "/download/";
            Log.d("Abhan", "PATH: " + PATH);
            File file = new File(PATH);
            if(!file.exists()) {
               file.mkdirs();
            }
            File outputFile = new File(file, fileName);
            FileOutputStream fos = new FileOutputStream(outputFile);
            InputStream is = c.getInputStream();
            byte[] buffer = new byte[1024];
            int len1 = 0;
            while ((len1 = is.read(buffer)) != -1) {
                fos.write(buffer, 0, len1);
            }
            fos.flush();
            fos.close();
            is.close();
        } catch (IOException e) {
            Log.e("Abhan", "Error: " + e);
        }
        Log.i("Abhan", "Check Your File.");
    } 
}
  • This answer will not work. Network connections on the main thread will throw `android.os.NetworkOnMainThreadException`. – JBirdVegas Nov 09 '14 at 15:24
  • @JBirdVegas Don't run network related operation on main thread. Kindly create a worker thread. –  May 31 '15 at 11:33
  • Use an AsyncTask doInBackground to do the try {}catch code. and remove setDoOutput(true) from it. – Zar E Ahmer Aug 05 '15 at 11:11
3

I fixed the code based on previous feedbacks on this thread. I tested using eclipse and multiple large files. It is working fine. Just have to copy and paste this to your environment and change the http path and the location which you would like the file to be downloaded to.

try {
    //this is the file you want to download from the remote server
    String path ="http://localhost:8080/somefile.zip";
    //this is the name of the local file you will create
    String targetFileName
        boolean eof = false;
    URL u = new URL(path);
    HttpURLConnection c = (HttpURLConnection) u.openConnection();
    c.setRequestMethod("GET");
    c.setDoOutput(true);
    c.connect();
    FileOutputStream f = new FileOutputStream(new File("c:\\junk\\"+targetFileName));
        InputStream in = c.getInputStream();
        byte[] buffer = new byte[1024];
        int len1 = 0;
        while ( (len1 = in.read(buffer)) > 0 ) {
        f.write(buffer,0, len1);
                 }
    f.close();
    } catch (MalformedURLException e) {
    // TODO Auto-generated catch block
    e.printStackTrace();
    } catch (ProtocolException e) {
    // TODO Auto-generated catch block
    e.printStackTrace();
    } catch (FileNotFoundException e) {
    // TODO Auto-generated catch block
    e.printStackTrace();
    } catch (IOException e) {
    // TODO Auto-generated catch block
    e.printStackTrace();
}

Good luck Alireza Aghamohammadi

grepit
  • 21,260
  • 6
  • 105
  • 81
  • in this way, will the same file be downloaded. i mean if the file is already downloaded will it give a alert? – Loshi Aug 15 '12 at 04:01
2

Just use apache's copy method (Apache Commons IO) - the advantage of using Java!

IOUtils.copy(is, os);

Do not forget to close the streams in a finally block:

try{
      ...
} finally {
  IOUtils.closeQuietly(is);
  IOUtils.closeQuietly(os);
}
AlikElzin-kilaka
  • 34,335
  • 35
  • 194
  • 277