Recovering JPEG image from .raw file using C

Question

I am doing a problem set provided by Harvard's online lecture. I've finally made a solution to recover a set of JPEG images from a file (card.raw).

It seems like the code itself does not throw errors, but it is returning distorted image and I am a little clueless to why it might be happening.

[Link to an image example] https://prnt.sc/q0tb4f

Here's my code

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>


int main(int argc, char *argv[])
{
    //check usage
    if (argc != 2)
    {
        return 1;
    }
    // open file
    FILE* input = fopen(argv[1], "r");
    // return error if file does not existS
    if (!input)
    {
        return 1;
        printf("file does not exists");
    }
    // create an array with 512 bytess of bytes
    unsigned char bytes[512];
    // create count variable
    int count = 0;
    // create an empty string for filename
    char filename[7];
    // create an empty output file
    FILE* output = NULL;
    // repeat until end of input
    while (fread(bytes, 1, sizeof(bytes), input) != 0)
    {
        // read 1 block of 512 bytes at a time
        fread(bytes, 1, sizeof(bytes), input);

        // check if beginning of jpeg file
        if (bytes[0] == 0xff && bytes[1] == 0xd8 && bytes[2] == 0xff && (bytes[3] & 0xf0) == 0xe0)
        {
            // if already found a jpeg, close the file
            if (count > 0)
            {
                fclose(output);
            }
            // name file
            sprintf(filename, "%03i.jpg", count);
            // open file
            output = fopen(filename, "w");
            // write file
            fwrite(bytes, 1, sizeof(bytes), output);
            // increment count
            count++;
        }
        if (output != NULL)
        {
            // keep writing if jpeg header is already found
            fwrite(bytes, 1, sizeof(bytes), output);
        }
    }
     fclose(output);
     fclose(input);
}

My uneducated assumption is unable to see why it might be happening. I can only imagine that this might be happening from opening and closing files in improper step.

This code is not reading properly. The `fread()` function returns the number of items it actually read, so you have to capture that result to know if you got a full buffer (of 512 bytes) or a partial read, then act accordingly. — Steve Friedl, Nov 22 '19 at 23:14
@SteveFriedl okay, that sounds like I need something outside of the `while loop`, correct? but am I not doing it by declaring `fclose()'? — JasonTheMarketer, Nov 22 '19 at 23:15
`int nread; while ( (nread = fread(...)) > 0) { ... stuff in body }` — Steve Friedl, Nov 22 '19 at 23:16
It's just a int variable that holds the result of the read, and later in the loop you use it to determine how much of the buffer you actually got. — Steve Friedl, Nov 22 '19 at 23:19

user3386109 · Accepted Answer · 2019-11-22T23:46:51.603

This is the problem:

while (fread(bytes, 1, sizeof(bytes), input) != 0)
{
    // read 1 block of 512 bytes at a time
    fread(bytes, 1, sizeof(bytes), input);

You're calling fread twice per loop. As a result, the body of the loop only sees the odd-numbered blocks. Remove the second fread.

A second problem (as @SteveFriedl points out) is that the buffer the code uses for the filename is too small.

char filename[7];
sprintf(filename, "%03i.jpg", count);

You need at least 8 bytes for a file name like "123.jpg", because you need room for the NUL terminator. However, note that "%03i" uses at least 3 characters. It could use more, e.g. if count reaches 1000. So I would declare the buffer as char filename[32]; to avoid any chance of buffer overflow.

You've also got two fwrites when only one is needed:

        output = fopen(filename, "w");
        // write file
        fwrite(bytes, 1, sizeof(bytes), output);
        // increment count
        count++;
    }
    if (output != NULL)
    {
        // keep writing if jpeg header is already found
        fwrite(bytes, 1, sizeof(bytes), output);
    }

After opening a new file, the code writes the first block, and then writes it again. Remove the first fwrite and let the second fwrite take care of the first block.

Another problem is that the code makes an implicit assumption that if fread doesn't return 0, then it's read a full block. That assumption is OK if the file size is a multiple of the block size, but it's better not to make any assumptions. So the condition in the while loop should be

while (fread(bytes, 1, sizeof(bytes), input) == sizeof(bytes))

Also, `filename` needs to be 8, not 7 to account for the trailing NUL byte. — Steve Friedl, Nov 22 '19 at 23:05
Thank you guys. After changing `7` to `8` it returned many more images. However, the problem is still there even after removing the second `fread()`. What else could I be doing wrong here? — JasonTheMarketer, Nov 22 '19 at 23:07
I've appended the link to the screenshot - https://prnt.sc/q0tb4f — JasonTheMarketer, Nov 22 '19 at 23:08
These are fantastic! Have my upvote. Specially the last point, it's an elegant solution. That said, I am still having the same image outputs. What could actually be happening here? Actually returning about 80% of the images intact. I will touch base with the course adviser. — JasonTheMarketer, Nov 22 '19 at 23:29
@user3386109 The program returned 50 images and only 8-9 of them were broken. Weirdly in the first 10 images only. Everything fell within the range of `000.jpg ~ 010.jpg`. Thank you for answering though! — JasonTheMarketer, Nov 22 '19 at 23:38

score 0 · Answer 2 · answered Sep 18 '20 at 14:47

here is recover working.

#include <stdio.h>
#include <stdlib.h>

int isAJpg(unsigned char bytes[]);

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        return 1;
    }
    FILE *file = fopen(argv[1], "r");
    if (file == NULL)
    {
        return 1;
    }
   
    char filename[10];
    int count = 0;
    unsigned char bytes[512];
    FILE *output;
    int jpgfound = 0;
   
    while (fread(bytes, 512, 1, file) != 0)
    {
       
        // if it is a jpg
        if (isAJpg(bytes) == 1)
        {
            if (jpgfound == 1)
            {
                fclose(output);
            }
            else
            {
                jpgfound = 1;
            }
            // name file
            sprintf(filename, "%03d.jpg", count);
           
            // open file
            output = fopen(filename, "a");
           
            count++;
        }
        if (jpgfound == 1)
        {
            // writes to a file
            fwrite(&bytes, 512, 1, output);
        }
    }
    //close the files
    fclose(output);
    fclose(file);
}

// check in it is a jpg
int isAJpg(unsigned char bytes[])
{
    if (bytes[0] == 0xff && bytes[1] == 0xd8 && bytes[2] == 0xff && (bytes[3] & 0xf0) == 0xe0)
    {
        return 1;
    }
    return 0;
}

Recovering JPEG image from .raw file using C

2 Answers2