2

I'm following a course about programming and went smooth so far. Now I'm stuck into this exercise which asks us to reverse a WAV file in C.

I have two questions about it:

  1. It seems to work and correctly reverse few seconds audios but on longer ones it doesn't anymore. Why?

  2. I previously added a get_data_size function to know a value to use later in the program, but after discovering the fseek() function it wasn't needed anymore. So I wanted to delete it, but if I do I get the "segmentation fault" error, while if I keep it it just runs ok. Why?

EDIT: Ok, now MAGICALLY, I tried to delete the get_data size function again and compiled it... and it doesn't go to segfault anymore. without changing anything. It still doesn't reverse the longer files though.


    #include <stdint.h>
    #include <stdio.h>
    #include <stdlib.h>
    
    #include "wav.h"
    
    int check_format(WAVHEADER header);
    int get_block_size(WAVHEADER header);
    long get_data_size(FILE *input, int size);
    
    int main(int argc, char *argv[])
    {
        // Ensure proper usage
        if (argc != 3)
        {
            printf("insert input and output names");
            return 2;
        }
    
        // Open input file for reading
        char *infile = argv[1];
        char *outfile = argv[2];
    
        FILE *input = fopen(infile, "r");
    
        if  (input == NULL)
        {
            printf("could not open the file\n");
            return 1;
        }
    
        // Read header into an array
        WAVHEADER inwav;
        fread(&inwav, sizeof(WAVHEADER), 1, input);
    
        // Use check_format to ensure WAV format
        if (check_format(inwav) != 0)
        {
            fclose(input);
            printf("not a valid format");
            return 3;
        }
    
        // Open output file for writing
        FILE *output = fopen(outfile, "w");
    
        if  (output == NULL)
        {
            printf("could not open the file\n");
            return 1;
        }
    
        // Write header to file
        fwrite(&inwav, sizeof(WAVHEADER), 1, output);
    
        // Use get_block_size to calculate size of block
        int n = get_block_size(inwav);
    
        //not needed anymore
        long s = get_data_size(input, n);
    
        //Sets it to the end of the file
        fseek(input, n, SEEK_END);
    
        // Write audio to file from the last block to the first
        while (ftell(input) != sizeof(WAVHEADER))
        {
            WORD temp;
            fread(&temp, n, 1, input);
    
            fwrite(&temp, n , 1, output);
    
            fseek(input, -2 * n, SEEK_CUR);
    
        }
    
        fclose(input);
        fclose(output);
        return 0;
    
    }
    
    
    int check_format(WAVHEADER header)
    {
        // Checks for the WAVE spelt in the header
        if (header.format[0] != 'W' && header.format[1] != 'A' && header.format[2] != 'V' && header.format[3] != 'E')
        {
            return 1;
        }
        return 0;
    }
    
    
    int get_block_size(WAVHEADER header)
    {
        //Double checks the channels are either 1 or 2 and calculates the BYTES per audio block
        if (header.numChannels == 1 || header.numChannels ==2)
        {
            int size = (header.bitsPerSample / 8) * header.numChannels;
            return size;
        }
        else
        {
            printf("not supported\n");
            return 0;
        }
        return 0;
    }
    
    
    //not needed anymore
    long get_data_size(FILE *input, int size)
    {
        long data = 0;
        int temp;
        while (!feof(input))
            {
                fread (&temp, 1, 1, input);
                data++;
            }
            return data;
        }
    ```





    
    #include <stdint.h>
    
    
    typedef uint8_t   BYTE;
    typedef uint16_t  WORD;
    typedef uint32_t  DWORD;
    
    typedef struct
    {
        BYTE   chunkID[4];
        DWORD  chunkSize;
        BYTE   format[4];
        BYTE   subchunk1ID[4];
        DWORD  subchunk1Size;
        WORD   audioFormat;
        WORD   numChannels;
        DWORD  sampleRate;
        DWORD  byteRate;
        WORD   blockAlign;
        WORD   bitsPerSample;
        BYTE   subchunk2ID[4];
        DWORD  subchunk2Size;
    } __attribute__((__packed__))
    WAVHEADER;

NRevan
  • 21
  • 2
  • Are you sure that you deleted all uses of the of function in the code and the type header `long get_data_size(FILE *input, int size);`, also what do you intend deleted, removed the definition from the file or set its memory to zero in the code? – Caridorc Jan 28 '23 at 11:03
  • How big is a `WORD`? Is it always >= `n`? – Tom Karzes Jan 28 '23 at 11:05
  • @Caridorc from the code only, like removing all the lines referencing it, and yes I tried. – NRevan Jan 28 '23 at 11:08
  • @TomKarzes sorry I should have added it the header definition, it's a uint16_t. its information are set and given at the start of the exercise. – NRevan Jan 28 '23 at 11:08
  • It is hard to help you debug this without the `wav.h` header, I can give 3 suggestions: 1) Look at the documentation at all the functions from there that you are using and make sure that you are doing all clean-up needed 2) Comment out more and more lines until you get a normal error rather than a segfault 3) print as many variables as possible to make sure that you know their values – Caridorc Jan 28 '23 at 11:12
  • 1
    Open the files explicitly in binary mode: `FILE *input = fopen(infile, "rb")` and `FILE *output = fopen(outfile, "wb");` – Weather Vane Jan 28 '23 at 11:16
  • 2
    Please see [Why is `while ( !feof (file) )` always wrong?](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) You must use the value returned by `fread`: `while(fread(...) != 0)` And you always need to know how many items were read if `count` is more than 1. – Weather Vane Jan 28 '23 at 11:20
  • @Caridorc I added the wav.h header. I will try to print to see where the problem starts then. – NRevan Jan 28 '23 at 11:20
  • @WeatherVane the course didn't teach us yet the existence of "rb" and "wb" but I tried and nothing changes. That function where the "eof" is mentioned is the one I'm trying to remove but if I do it gives me a segfault. – NRevan Jan 28 '23 at 11:24
  • 2
    If you are using `fread` and `fwrite` you must open in binary mode. It might not make a difference on some platforms but it definitely will on others. – Weather Vane Jan 28 '23 at 11:25
  • A random guess: what if the header is only 3 chars long? then `check_format` will try to access out of bounds – Caridorc Jan 28 '23 at 11:28
  • @NRevan Are you certain that `n` is never greater than 2? If it is, then you're corrupting memory, since a `uint16_t` is 2 bytes. – Tom Karzes Jan 28 '23 at 11:33
  • 1
    @Caridorc I added a check for that, thanks to make me notice it. – NRevan Jan 28 '23 at 11:39
  • @TomKarzes it can either be (in decimal) 1 or 2 bytes depending on the number of channels the wav file has. – NRevan Jan 28 '23 at 11:40
  • Is the endianness of the header members correct? – Weather Vane Jan 28 '23 at 11:41
  • @WeatherVane I did like that because channels are in 1 or 2 bytes and I have to keep the order of the couple if they're 2. so going back by n every time allows me to write it backwards but keeping the 2 bytes couple with the original order. I don't know, it made sense to me. – NRevan Jan 28 '23 at 11:42
  • I overlooked that you are reversing the file content. But where endianness can matter is with the header struct members that are more than 1 byte. You do need to know the endianness of the standard file type, and read it accordingly. It might be different from your machine's endianness. In the header, a 2-byte unsigned value will be 256 * the higher-order byte plus the low-order byte. In the data, the output will be the same as the input. – Weather Vane Jan 28 '23 at 11:45
  • I've started programming 3-4 weeks ago soyou're allowed to treat me like an idiot without worrying too much. So the WAVHEADER part is not made by me but by the professors. my task was just check the format and reverse the audio by keeping the channels ordered. It works as long as I don't delete the get_data_size function and the audio file is not too long. – NRevan Jan 28 '23 at 11:45
  • 1
    Is `while (ftell(input) != sizeof(WAVHEADER))` correct? I would suppose it should be `while (ftell(input) >= sizeof(WAVHEADER))` because a) you need to read the data immediately following the header, and b) it's better practice to use a "catch-all" check rather than for a specific value. – Weather Vane Jan 28 '23 at 12:03

0 Answers0