avifile tests

[Dmitry Timoshkov]

Hi Julius,

"Julius Schwartzenberg" <julius.schwartzenberg at gmail.com> 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?

-- 
Dmitry.