Re: avifile tests

[Dmitry Timoshkov]

Hi Julius,

"Julius Schwartzenberg"  wrote:
> +#include "api_test.h"

If the definitions in api_test.h are used only in 1 source file there is no
need to create an .h file at all, just place all the data in that source file.
> +void init_test_struct(COMMON_AVI_HEADERS *cah)

This should be static, same as all other functions and data structures below.
> +{
> - MainAVIHeader *mah = malloc(sizeof(MainAVIHeader));
> - AVIStreamHeader *ash0 = malloc(sizeof(AVIStreamHeader));
> - AVIStreamHeader *ash1 = malloc(sizeof(AVIStreamHeader));
> - PCMWAVEFORMAT *pcmwf = malloc(sizeof(PCMWAVEFORMAT));
> - memcpy(mah, &defmah, sizeof(defmah));
> - memcpy(ash0, &defash0, sizeof(defash0));
> - memcpy(ash1, &defash1, sizeof(defash1));
> - memcpy(pcmwf, &defpcmwf, sizeof(defpcmwf));
> - cah->mah = mah;
> - cah->ash0 = ash0;
> - cah->ash1 = ash1;
> - cah->pcmwf = pcmwf;
>
> +}

malloc() should not be used in the Wine code. Also, since you allocate
fixed buffer sizes it would be simpler to have structures not pointers
to the structures inside main structure.
> +void create_avi_file(COMMON_AVI_HEADERS *cah)

'cah' should be const.
> +{
> - FILE * pFile;
> - pFile = fopen("small.avi", "wb");
> -
> - fwrite(file_header, 1, sizeof(file_header), pFile); /* de file wordt hier niet goed aangemaakt */
> - fwrite(cah->mah, 1, sizeof(MainAVIHeader), pFile);
> - fwrite(streamlist, 1, sizeof(streamlist), pFile);
> - fwrite(cah->ash0, 1, 0x38, pFile);
> - fwrite(videostreamformat, 1, sizeof(videostreamformat), pFile);
> - fwrite(padding1, 1, sizeof(padding1), pFile);
> - fwrite(videopropheader, 1, sizeof(videopropheader), pFile);
> - fwrite(cah->ash1, 1, 0x38, pFile);
> - fwrite(audiostreamformat_pre, 1, sizeof(audiostreamformat_pre), pFile);
> - fwrite(cah->pcmwf, 1, sizeof(PCMWAVEFORMAT), pFile);
> - fwrite(data, 1, sizeof(data), pFile);
> -
> - fclose (pFile);
>
> +}

Please use CreateFile()/WriteFile()/etc. instead of fopen/fwrite/etc.
> +void test_default_data()

Please don't omit 'void' in case of no parameters.
> +{
> - COMMON_AVI_HEADERS *cah;
> - PAVIFILE pFile;
> - int res;
> - LONG lSize;
> - WAVEFORMATEX wfx;
> - PAVISTREAM pStream;
> -
> - cah = calloc(sizeof(int),4);

Same comment as for malloc above. Also this can't work.
> - init_test_struct(cah);
> - create_avi_file(cah);
> -
> - res = AVIFileOpen(&pFile, "small.avi", OF_SHARE_DENY_WRITE, 0L);
> - ok(res == 0, "Unable to open file: error=%u\n", res);
> -
> - res = AVIFileGetStream(pFile, &pStream, 0, 1);
> - ok(res == 0, "Unable to open audio stream: error=%u\n", res);
> -
> - ok(AVIStreamReadFormat(pStream, AVIStreamStart(pStream), NULL, &lSize) == 0, "Unable to read format size\n");
> - ok(AVIStreamReadFormat(pStream, AVIStreamStart(pStream), &wfx, &lSize) == 0, "Unable to read format\n");

Please use 'res' as an intermediate result storage as you do above.
> +static const PCMWAVEFORMAT defpcmwf =
> +{
> - {
> - 1,      /* wFormatTag      */
> - 2,      /* nChannels       */
> - 11025,  /* nSamplesPerSec  */
> - 22050,  /* nAvgBytesPerSec */
> - 2,      /* nBlockAlign     */
> - },
> - 8,      /* wBitsPerSample  */
>
> +};

Indentation of above structure and all others below this one look strange.
4 spaces would be quite enough.

Do you really need all that separate structures? Wouldn't it be simpler to
have a single buffer like in dlls/winmm/tests/mmio.c?