From: US1RMC::"seymour@m31.dgbt.doc.ca" "Seymour Shlien" 27-AUG-1993 15:05:31.82 To: 3d::pan CC: Subj: problems and fixes Dear Davis, I have now got the MPEG audio coder and decoder to work on both UNIX (Sparc 2) and Microsoft C. All the problems that we encountered were mainly restricted to the code in the file common.c and were related to the AIFF audio format functions. I gather from the comments in the text, other people had encountered similar problems. When compiled under UNIX, the gcc compiler gave messages similar to below. common.c: In function `aiff_read_headers': common.c:673: warning: multi-character character constant common.c:674: warning: multi-character character constant common.c:688: warning: multi-character character constant common.c:719: warning: multi-character character constant common.c:719: warning: passing arg 2 of `strcmp' makes pointer from integer without a cast common.c: In function `aiff_write_headers': common.c:807: warning: multi-character character constant common.c:808: warning: multi-character character constant common.c:809: warning: multi-character character constant 670 if (fread(&FormChunk, sizeof(Chunk), 1, file_ptr) != 1) 671 return(-1); 672 673 if (*(unsigned long *) FormChunk.ckID != IFF_ID_FORM || 674 *(unsigned long *) FormChunk.formType != IFF_ID_AIFF) 675 return(-1); 676 677 678 /* if (strcmp(FormChunk.ckID,IFF_ID_FORM) || 679 strcmp(FormChunk.formType,IFF_ID_AIFF)) 680 return(-1); 681 */ ... ... 715 aiff_ptr->numSampleFrames = CommChunk.numSampleFrames; 716 aiff_ptr->sampleSize = CommChunk.sampleSize; 717 718 /* } else if (*(unsigned long *)Header.ckID == IFF_ID_SSND) { */ 719 } else if (strcmp(Header.ckID,IFF_ID_SSND)) { 720 721 /* Similarly on MSDOS we obtained the following warnings Microsoft (R) Program Maintenance Utility Version 1.11 Copyright (c) Microsoft Corp 1988-90. All rights reserved. cl /AL /G2 /FPi87 /nologo /c /Gi musicin.c musicin.c musicin.c(959) : warning C4047: '!=' : different levels of indirection cl /AL /G2 /FPi87 /nologo /c /Gi common.c common.c common.c(814) : warning C4047: '=' : different levels of indirection common.c(815) : warning C4047: '=' : different levels of indirection common.c(816) : warning C4047: '=' : different levels of indirection 814 815 *(unsigned long *) FormChunk.ckID = IFF_ID_FORM; 816 *(unsigned long *) FormChunk.formType = IFF_ID_AIFF; 817 *(unsigned long *) CommChunk.ckID = IFF_ID_COMM; 818 819 double_to_extended(&aiff_ptr->sampleRate, temp_sampleRate); also a few similar error messages in musicin.c and musicout.c Despite these warnings both the encoder (musicin) and the decoder musicout appeared to run correctly on the data files orig.mpg and sine.dec. However command line input is not properly implemented on MicroSoft C. The program tries to make a file orig.mpg.dec which is not legal in MSDOS BUGS IN THE SOFTWARE For MS_DOS it was necessary to modify the mem_alloc function in order to avoid crashing the computer on startup. I do not know the reason as I am not an expert in MicroSoft C. Also, the software does not properly handle AIFF formatted files. Using the program musicout I created an AIFF formatted file from the orig.mpg file. When I ran musicin on the AIFF formatted file on our SUN work station (ie UNIX environment) , the program crashed. On the other hand, when the same test was performed on the MSDOS, the computer did not crash but it failed to recognize the file as an AIFF formatted file and it did not read the header block. Nevertheless it was still able to compress the file, once the appropriate header information was typed in. The problems seem to be related to the handling of the ID information in the chunk structure of the AIFF file. The ID is a 32 bit integer which can take one of 5 designated values. The designated values for convenience also can be decoded as ascii strings FORM, AIFF COMM, SSND and MPEG. The ISO software attempts to provide two ways of handling these numbers- either as a multicharacter constant or a string. In the common.h file /* * Note: The value of a multi-character constant * is implementation-defined. */ #if !defined(MS_DOS) && !defined(AIX) #define IFF_ID_FORM 'FORM' #define IFF_ID_AIFF 'AIFF' #define IFF_ID_COMM 'COMM' #define IFF_ID_SSND 'SSND' #define IFF_ID_MPEG 'MPEG' #else #define IFF_ID_FORM "FORM" #define IFF_ID_AIFF "AIFF" #define IFF_ID_COMM "COMM" #define IFF_ID_SSND "SSND" #define IFF_ID_MPEG "MPEG" #endif Unfortunately, the revisions to the code in common.c do not properly read or write the AIFF header block in either mode. If the IFF_* constants are defined as a long integer, (ie single quotes around FORM, AIFF, etc.), then the strcmp function will crash on some machines because the inputs Header.ckID and IFF_ID_FORM are not strings. Strcmp expects to receive null terminated strings. Since it instead receives addresses to nonterminated strings, the strcmp runs off to infinity searching for the null character. Note: when IFF_* are defined as strings, Header.ckID is still not a null terminated string. Fortunately strcmp stops when a null is encountered in either one of its arguments -- in this case a null is encountered in IFF_*. It might be safer to use strncmp which includes a string length argument. This was the source of our problem on the UNIX machine as the common.c attempted to do a string compare in one spot (around line 718). /* } else if (*(unsigned long *)Header.ckID == IFF_ID_SSND) { */ } else if (strcmp(Header.ckID,IFF_ID_SSND)) { The commented line would have worked correctly in the UNIX environment. A different problem occurs in MS_DOS. In the function aiff_write_header the following statement does not do what was intended when IFF_ID_FORM is a string. *(unsigned long *) FormChunk.ckID = IFF_ID_FORM; IFF_ID_FORM points to an address of the string "FORM". Though it was intended to put "FORM" in FormChunk.ckID, the above statement instead places the address of IFF_ID_FORM into FormChunk.ckID. This is what is put in the AIFF file. Musicin reads the file and fails to recognize it as an AIFF file. (The above statement would have been correct if IFF_ID_FORM was a long integer.) FIXES I have made the following changes in the software so that it can now handle AIFF formatted files in either MicroSoft C or UNIX. In common.h, I have introduced a new def called IFF_LONG whenever IFF_* is represented by multicharacter longs instead of strings. 187,188c187 < #if !defined(MS_DOS) && !defined(AIX) < #define IFF_LONG --- > #if !defined(MS_DOS) && !defined(AIX) In common.c the following changes were made For avoiding crashing the PC on startup 456,457c456 < /*ptr = (void FAR *) _fmalloc((unsigned int)block);*/ /* far memory, 92-07-08 sr */ < ptr = (void FAR *) malloc((unsigned int)block); /* far memory, 93-08-24 ss */ --- > ptr = (void FAR *) _fmalloc((unsigned int)block); /* far memory, 92-07-08 sr */ For debugging and testing 646,656d644 < < /**** for debugging < showchar(str) < char str[4]; < { < int i; < for (i=0;i<4;i++) printf("%c",str[i]); < printf("\n"); < } < ****/ < To correct the problems with the IFF_* ID's I do the string comparisons two different ways depending on whether IFF_LONG is defined. 685d672 < #ifdef IFF_LONG 689d675 < #else 691,694d676 < if (strncmp(FormChunk.ckID,IFF_ID_FORM,4) || < strncmp(FormChunk.formType,IFF_ID_AIFF,4)) < return(-1); < #endif 695a678,681 > /* if (strcmp(FormChunk.ckID,IFF_ID_FORM) || > strcmp(FormChunk.formType,IFF_ID_AIFF)) > return(-1); > */ 702d687 < #ifdef IFF_LONG 705,708c690,691 < #else < if (strncmp(Header.ckID,IFF_ID_COMM,4) == 0) { < #endif < --- > /* if (strcmp(Header.ckID,IFF_ID_COMM)) { > */ 711a695 > 734,738c718,720 < #ifdef IFF_LONG < } else if (*(unsigned long *)Header.ckID == IFF_ID_SSND) { < #else < } else if (strncmp(Header.ckID,IFF_ID_SSND,4) == 0) { < #endif --- > /* } else if (*(unsigned long *)Header.ckID == IFF_ID_SSND) { */ > } else if (strcmp(Header.ckID,IFF_ID_SSND)) { > 741a724 > 824d806 < #ifdef IFF_LONG 828,832d809 < #else < strncpy(FormChunk.ckID,IFF_ID_FORM,4); < strncpy(FormChunk.formType,IFF_ID_AIFF,4); < strncpy(CommChunk.ckID,IFF_ID_COMM,4); < #endif 1536a1514 > In musicin.c The following changes were made for the same reasons discussed above. 959d958 < #ifdef IFF_LONG 961,963d959 < #else < if (strncmp(&pcm_aiff_data->sampleType,IFF_ID_SSND,4)) { < #endif In musicout.c The following changes were made for the same reasons discussed above. 381d380 < #ifdef IFF_LONG 383,385d381 < #else < strncpy(&pcm_aiff_data.sampleType,IFF_ID_SSND,4); < #endif 388c384 < --- > This is a copy of the makefile that I am using on the IBM. Perhaps one of the switches might explain why I had trouble with the mem_alloc. ALL : musicin.exe musicout.exe CFLAGS = /AL /G2 /FPi87 /nologo /c /Gi /F 256 LFLAGS= /SE:256 /ST:16000 /F /PACKC musicin.obj: musicin.c common.h encoder.h cl $(CFLAGS) musicin.c common.obj: common.c common.h cl $(CFLAGS) common.c encode.obj: encode.c common.h encoder.h cl $(CFLAGS) encode.c subs.obj: subs.c common.h encoder.h cl $(CFLAGS) subs.c psy.obj: psy.c common.h encoder.h cl $(CFLAGS) psy.c tonal.obj: tonal.c common.h encoder.h cl $(CFLAGS) tonal.c musicout.obj: musicout.c common.h decoder.h cl $(CFLAGS) musicout.c decode.obj: decode.c common.h decoder.h cl $(CFLAGS) decode.c musicin.exe: musicin.obj common.obj encode.obj subs.obj psy.obj tonal.obj link $(LFLAGS) musicin common encode subs psy tonal,musicin,,; musicout.exe: musicout.obj common.obj decode.obj subs.obj link $(LFLAGS) musicout common decode subs,musicout,,; I am grateful, for the help from Daniel Lauzon -- our resident C and C++ expert. Also Bill Truerniet helped me with Microsoft C on the PC. seymour@dgbt.doc.ca % ====== Internet headers and postmarks (see DECWRL::GATEWAY.DOC) ====== % Received: by us1rmc.bb.dec.com; id AA06920; Fri, 27 Aug 93 14:58:09 -0400 % Received: by inet-gw-1.pa.dec.com; id AA02547; Fri, 27 Aug 93 11:59:18 -0700 % Received: from rigel.dgbt.doc.ca by m31.dgbt.doc.ca (4.1/SMI-4.1) id AA01692; Fri, 27 Aug 93 14:57:59 ED % Date: Fri, 27 Aug 93 14:57:59 EDT % From: seymour@m31.dgbt.doc.ca (Seymour Shlien) % Message-Id: <9308271857.AA01692@ m31.dgbt.doc.ca> % To: 3d::pan % Subject: problems and fixes