#484 Video capture failure and crash

[rainwarrior]

Some versions of libpng packages will define Z_SOLO for zlib, which causes capture.video.codec->SetupCompress to fail in hardware.cpp, which in turn will silently generate an empty video file, and crash when it tries to close the file later.

This is not an issue for builds where zlib is set up without Z_SOLO but it seems to have come up for many people (myself included, also visible in many EmuCR builds, etc.) and seems like it could be prevented easily. Especially an issue if you were recording for a while, and end up an hour later with a crash and an empty video file.

I would recommend putting a log message in the error case for SetupCompress before jumping to skip_video, so at least the user might be given a warning about it. That this error is currently silent is the biggest part of the problem.

There's the crash too, but it and all the other errors handled by skip_video seem extremely unlikely if zlib is configured correctly. The main thing is just that the user needs to know that video capture is broken when they try to use this, and probably also the maintainer needs to know how to fix it.

I'd maybe replace the goto line (harware.cpp:487) with something like:

{
    LOG_MSG("Video Codec failed to set up compressor. (Was zlib compiled with Z_SOLO by mistake?)");
    goto skip_video;
}

[Qbix]

Mmm, never heard of Z_SOLO. Got to check that.

It might be even better to also give a warning/errror when compiling. (Or require zlib itself explicitly). Most users probably won't know how to fix it, so best to catch it at compile level.

Nonetheless, a runtime warning is a good idea.

[rainwarrior]

I'm not sure if it's possible to catch at compile time. Z_SOLO has an affect on zlib when it's compiled, but I don't think this changes anything you can query directly. (I think it's supposed to be up to the user to define it both when compiling zlib and when they use the library in their project.)

This specific error happens because the Z_SOLO version requires allocator functions to be passed as part of the stream structure. Actually, if you sent malloc/free here in the zmbv implementation it might prevent the error even when Z_SOLO is used? (Not entirely sure.)

[Qbix]

Yeah, was thinking of setting them, but not sure if compression then works.
Maybe only when Z_SOLO is defined. Can you tell me where you got the "bad" package from ? Could give it a spin with my setup.

[rainwarrior]

I'm not sure about other build platforms, but for me, the Z_SOLO was defined by the VS project for zlib provided with the latest version of libping. (lpng1634/projects/vstudio/zlib/zlib.vcproj)

I tried replacing the bottom of zmbv.cpp with the following as a test, and it resulted in no error and correctly captured video, using either the zlib library compiled with Z_SOLO defined or not.

voidpf zalloc(voidpf opaque, uInt items, uInt size)
{
    if (opaque) opaque = 0;
    return calloc(items, size);
}

void zfree(voidpf opaque, voidpf ptr)
{
    if (opaque) opaque = 0;
    free(ptr);
}

VideoCodec::VideoCodec() {
    CreateVectorTable();
    blocks = 0;
    buf1 = 0;
    buf2 = 0;
    work = 0;
    memset( &zstream, 0, sizeof(zstream));
    zstream.zalloc = zalloc;
    zstream.zfree = zfree;
}

As far as what Z_SOLO changes about about compress/compress2/compressBound, I don't think that's an issue. I believe not having Z_SOLO causes those functions to be named with a z prefix internally to prevent conflict with other libraries that may use the name "compress" for an external function. (The Z_SOLO flag appears to mean "zlib alone with no external libraries" in some way.)

Whatever that does, though, I don't believe either libpng or zmbv uses zlib's compress either directly or indirectly, so it wouldn't matter anyway. (libpng provides its own allocators to zlib as well.)

Not sure if the lines above that zero opaque are important, since that's for zmbv's internal use, but the default zlib zcalloc/zcfree appear to do it, so I kept it for consistency.

[Qbix]

Looks okay. But want to give it some testing first in a few of my setups.