• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 754
  • Last Modified:

ifstream (infile) corruption

Messing around with a program, and I wrote a function that somehow corrupts the input file.  It reads in an mp3 and spits out some info about it.  It works perfectly for VBR mp3s, but when you toss it a CBR mp3, it goes whacko.  So I'm doing something in the VBR detection code that's specifically not corrupting the in stream, and if a CBR file is tossed at it, that's not being done, so the stream gets corrupted.  Yeah, doesn't make sense to me either.  To run this, just compile (took out all irrelevant bits), get a CBR mp3, name it test.mp3, and let it go.  The corrupting function is isVBR(), and wherever you put it, it corrupts the stream.  I know the code is rough, but I'm just playing around here.

// mp3Test.cpp : Defines the entry point for the console application.
//

#include <iostream>
#include <string>
#include <fstream>

using namespace std;

#define HEADERBYTES 1

int findFrame (ifstream &in)
{
      unsigned char *mp3Header = new unsigned char[HEADERBYTES];

      in.seekg(ios::beg);

      for (int iBytesRead = 0; iBytesRead < 10 * 1024; iBytesRead += HEADERBYTES) // if it isn't in the first 10k, it's nowhere
      {
            in.seekg(iBytesRead);
            in.read((char *)mp3Header, HEADERBYTES);

            if (mp3Header[0] == 0xFF)
            {
                  in.seekg(iBytesRead + HEADERBYTES);
                  in.read((char *)mp3Header, HEADERBYTES);
                  if ((mp3Header[0] & 0xE0) == 0xE0)
                  {
                        cout << "Found header at " << iBytesRead << endl;
                        return iBytesRead;
                  }
            }
      }
      return -1;
}

bool isVBR (ifstream &in)
{
      in.seekg(ios::beg);

      for (int i = 0; i < 10 * 1024; i++)
      {
            char *vbr = new char;
            in.read(vbr, 1);

            if (*vbr == 'X' || *vbr == 'I')
            {
                  string lastThree;
                  char *chars = new char[3];

                  in.seekg(i + 1);
                  in.read(chars, 3); // this puts garbage in the chars for some dumbass reason
                  lastThree = string(chars);

                  if (lastThree.substr(0, 3) == "ing" || lastThree.substr(0, 3) == "nfo") // hack cause of garbage
                  {
                        cout << "VBR header at " << i << endl;;
                        return true;
                  }
            }
            in.seekg(i + 1);
      }
      return false;
}

int main(int argc, char *argv[])
{
      ifstream in;
      int iFirstFramePos = 0;

      in.open("test.mp3");

      if (!in)
      {
            cout << "Can't open test.mp3" << endl;
            return 0;
      }

      if (isVBR(in))
            cout << "File is VBR." << endl;

      int iPos = findFrame(in);

      if (iPos == -1)
      {
            cout << "Couldn't find first frame header!" << endl;
            return 0;
      }

      unsigned char *pFrameHeader = new unsigned char[4];
      in.seekg(iPos);
      in.read((char *)pFrameHeader, 4);

      return 0;
}

What's happening if you run the code as-is, is when you go into findFrame, the in.seekg won't progress, thus not finding a frame header.  If you put the isVBR call after the findFrame, it finds the frame, but then it corrupts the instream so the stuff in pFrameHeader is completely out of whack.

Thanks for any help... this one's been causing me a headache all day.  Oh, and bust out the debugger and check out where I say it's spitting out garbage.  Explain that one...
0
dbunder
Asked:
dbunder
  • 10
  • 4
  • 4
2 Solutions
 
rstaveleyCommented:
The string constructor you are using expects an ASCIIZ (i.e. a '\0' terminator)

               lastThree = string(chars);  // Keeps reading until it finds a '\0'

Try:
               lastThree = string(chars,3); // Takes 3 characters

(Beware that chars needs to be deleted. Why not simply use an automatic rather than a dynamic array?)
0
 
rstaveleyCommented:
Similarly you can use an automatic here:

          char vbr;
          in.read(&vbr, 1);

          if (vbr == 'X' || vbr == 'I')
0
 
rstaveleyCommented:
After in.seekg(i + 1), you should test the state of the stream.

Do you actually want...

   is.seekg (i + 1, ios::beg);

I think the default parameter is ios::cur (i.e. relative to the last position), but I stand to be corrected :-)
0
Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
rstaveleyCommented:
> I think the default parameter is ios::cur (i.e. relative to the last position), but I stand to be corrected :-)

Ignore that. It does default to ios::beg as you can see at http://www.cppreference.com/cppio/seekg.html - I always put that parameter in because I've got a poor memory.
0
 
itsmeandnobodyelseCommented:
>>>> Why not simply use an automatic rather than a dynamic array?)

Take that

               string lastThree;
               char chars[4] = { '\0' };  make all zero

               in.seekg(i + 1);
               in.read(chars, 3); // this puts garbage in the chars for some dumbass reason
                                         // should be ok now cause the string is correctly terminated now
               lastThree = chars;  // std::string has an assignment operator as well
              if (lastThree == "ing" || lastThree == "nfo")
              ...

Regards, Alex
0
 
rstaveleyCommented:
My guess is that isVBR reaches the end of the stream and sets the eof bit in the process, and all you need to do is call is.clear() at the end of it to get the stream back into a readable state.

See http://www.cppreference.com/cppio/all.html
0
 
rstaveleyCommented:
Actually this works too:

             char chars[3];
             in.seekg(i + 1);
             in.read(chars, 3);
             string lastThree(chars,3);  // Using ctor with length specifier
             if (lastThree == "ing" || lastThree == "nfo")

You don't need '\0' termination, if you use the string ctor with a length specifier.
0
 
itsmeandnobodyelseCommented:
You might consider to read the whole file to memory and make the checks there.

#include <sys/stat.h>
#include <fstream>
#include <iostream>
#include <string>
using namespace std;

int main()
{
       struct stat filestat;
       if (stat("test.mp3", &filestat) != 0)
            return 1;  // file does not exist
       char* buf = new char[filestat.st_size];
       
       ifstream ifs( "test.mp3", ios::in | ios::binary);
       if (!ifs.read(buf, filestat.st_size))
             return 2;  // file cannot be read
       ifs.close();

       string all(buf, filestat.st_size);

       int pos = 0;
       if ((pos = all.find("Xing")) != string::npos)
       {
             cout << "Xing detected at " << pos  << endl;
       }
       else if ((pos = all.find("Info")) != string::npos)
       {
             cout << "Info detected at " << pos  << endl;
       }
       char mp3 = 0xff;      
       int    len = all.length()-1;
       while ((pos = all.find(mp3, pos)) != string::npos && pos < len )
       {
            if ( (string[pos+1] & 0xe0) == 0xe0)
            {
                cout << "Frame detected at " << pos  << endl;
                break;
            }
            pos++;
       }
       return 0;
}

Regards, Alex
0
 
rstaveleyCommented:
That's very good advice.

[IMHO.. Using iterators with IOStreams and the standard libary algorithms *ought* to do that automagically for you too (ought = if I ruled the world), but istream_iterator<> is an input iterator and it can't be used as a forward iterator, which is what you'd need to use the search algorithm. It seems a shame to me that streams can't be streated more like containers.]
0
 
dbunderAuthor Commented:
All very good ideas, and some stuff I will implement.  Like I said, it's rough code and I'm not worrying about memory management or anything just yet.  Thus, the dynamic arrays.  As far as the '\0' goes, it's been, well, at least 2 years since I've touched C and there're some things oyu forget.

But the question is: why is my ifstream being corrupted?  

I may split this one between some people due to good suggestions, and if my ifstream corruption question is answered, some credit to them.
0
 
rstaveleyCommented:
> why is my ifstream being corrupted?

My guess was... Because you reached end of file. You need to clear the EOF bit, by calling is.clear() after hitting EOF.
0
 
dbunderAuthor Commented:
Shouldn't be the case (I stop it at 10k), but I will give it a shot.  Thanks.

I haven't slept, so I don't wanna be buggered with code right now, so I'll try later and report results. :)
0
 
itsmeandnobodyelseCommented:
>>>> But the question is: why is my ifstream being corrupted?

rstaveley is right. The ifstream object isn't corrupted but you read beyond EOF when checking for VBR. Then, the EOF bit of the stream's state word is set what prevents any further read. To make both functions mutually independent  you should open (and close) the file in the  check functions, e. g. by passing the filename as string (more precise as 'const string&').When opening the file in IsVBR and findFrame you could omit the seekg calls. They are unneeded when reading the file sequentially.

Regards, Alex
 
0
 
dbunderAuthor Commented:
In isVBR, it stops at 10k, which is FAR beyond the end of the file.  But I'll check the in.clear thing.
0
 
rstaveleyCommented:
If you are on a DOS system, you should open the stream thus...

    in.open("test.mp3",ios::binary|ios::in);

...because a DOS Ctrl+Z end of file marker might be marking end of file prematurely.
0
 
dbunderAuthor Commented:
ctrl+z sounds completely valid.  stupid microslut.

i've been very busy today and unable to put some of your ideas in my code (streamlining, reopening files, reading as only infiles, not using pointers where not needed, etc), but i will be able tomorrow.  it may result in points being doled out, it may result in more questions.  we'll hope for best case.

thanks much all!!!!  if anyone has any other answers for me, please spit them out! :)  amongst how many users can you dole out points?
0
 
itsmeandnobodyelseCommented:
>>>> amongst how many users can you dole out points?

Unfortunately it is less than 251 users cause you need to give 1 point to anyone ;-)

>>>> If you are on a DOS system ...

Actually, you should open the file in binary mode if it contains binary data. Regardless whether it is DOS (????) or any other OS.

Regards, Alex
0
 
rstaveleyCommented:
> you should open the file in binary mode if it contains binary data

That's true, but UN*X lets you get away with more. There's no special treatment for Ctrl+D in text file I/O in Linux. It is an oddity of DOS that you can mark the end of the file before the end designated by the file system. I remember reading some DOS source code many moons ago that placed a Ctrl+Z before a bunch of disabled functions, which were appended to the end of the file (like putting them in a #if 0...#endif block). You can still do it with Win32 compilers, if you want to make yourself unpopular with your peers.

> DOS (????)

Freudian slip. Windoze of course. I can't pretend that I'm using Lynx to access Experts Exchange 8-)
0

Featured Post

Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

  • 10
  • 4
  • 4
Tackle projects and never again get stuck behind a technical roadblock.
Join Now