Huffyuv Bug Fix - Need Help

[Mark Fredrickson]

Huffyuv has a bug where it gets a buffer overflow when capturing static video and crashes.

Avery Lee identified it and modified VirtualDub to accomodate it. He mentioned that it would be possible to modify Huffyuv and recompile the source (http://www.math.berkeley.edu/~benrg/...urce-2.1.1.zip).

Has anyone done this or does anyone have the skills to do it? I've downloaded the source and debugged it with VC++ but my skills are not sufficient to find and fix it.

Here is what Avery says:

Symptoms: Capturing with the Huffyuv video codec causes an Access Violation in VirtualDub.

Cause: Huffyuv underestimates the maximum buffer size that it reports to VirtualDub by about half. Because Huffyuv is a lossless compression algorithm, it will sometimes end up outputting compressed frames that are bigger than the original, uncompressed frame. If the source is seriously noisy, Huffyuv's compression ratio will drop below 1.0:1 and start outputting these oversized frames. When the compression ratio drops below 0.66:1 for UYVY/YUY2, or 0.6:1 for RGB24, Huffyuv overruns its output buffer and crashes. It turns out that one of the easiest ways to do this is to capture the static that appears when a tuner does not have a valid input.

Workaround: Avoid using Huffyuv in situations where it may potentially overrun its output buffer. Usually, these are cases where Huffyuv is not giving significant compression anyway and consumes a lot of CPU power, so you will not want to do so anyway. Unfortunately, this is not a perfect workaround since the crash will occur if any one frame exceeds the output buffer. Another option is to recompile Huffyuv to report the correct size, which is noted in the source code comments for that codec.

Any help would be most appreciated as this bug is driving me and others nuts trying to capture our home videos.

Thanks,

Mark.

[DSPguru]

look for malloc(x), repleace with malloc(2*x)

[gabest]

Here is the recompiled dll for you.

(I believe you have to wait a little till the attachment becomes available)

[Mark Fredrickson]

DSPguru - there is no malloc in the code.

gabest - No dll yet. Any chance you can email it to me at mark.fredrickson@attbi.com. Also, I would be interested in the change you made so if you don't mind, how about sending the source too?

Thanks.

[DSPguru]

hi Mark !

look for

new unsigned char

malloc is used in ".c", new is used in ".cpp"

[Mark Fredrickson]

DSPguru,

I think that this is the culprit. What do you recommend?

// allocate buffer if compressing RGB->YUY2->HFYU
if (outtype == -1 && (method == methodGrad || intype == 3))
yuy2_buffer = new unsigned char[lpbiIn->biWidth * lpbiIn->biHeight * 2 + 4];
if (method == methodMedian)
median_buffer = new unsigned char[lpbiIn->biWidth * lpbiIn->biHeight * 2 + 8];
if (outtype == -2 && method == methodGrad+flagDecorrelate)
rgb_buffer = new unsigned char[lpbiIn->biWidth * lpbiIn->biHeight * 3 + 4];
if (outtype == -3 && method == methodGrad+flagDecorrelate)
rgb_buffer = new unsigned char[lpbiIn->biWidth * lpbiIn->biHeight * 4 + 4];

InitClip();

return ICERR_OK;
}

DWORD CodecInst::CompressGetSize(LPBITMAPINFOHEADER lpbiIn, LPBITMAPINFOHEADER lpbiOut) {
// Assume 24 bpp worst-case for YUY2 input, 40 bpp for RGB.
// The actual worst case is 43/51 bpp currently, but this is exceedingly improbable
// (probably impossible with real captured video)
return lpbiIn->biWidth * lpbiIn->biHeight * ((GetBitmapType(lpbiIn) <= 2) ? 3 : 5);
}


gabest,

Am we on the right track?

Mark.

PS. Please excuse my ignorance, buy what does the -> and ? mean?

[gabest]

Damn, our moderator seems to be away, and won't authorize the zip...

Originally posted by Mark Fredrickson
DWORD CodecInst::CompressGetSize(LPBITMAPINFOHEADER lpbiIn, LPBITMAPINFOHEADER lpbiOut) {
// Assume 24 bpp worst-case for YUY2 input, 40 bpp for RGB.
// The actual worst case is 43/51 bpp currently, but this is exceedingly improbable
// (probably impossible with real captured video)
return lpbiIn->biWidth * lpbiIn->biHeight * ((GetBitmapType(lpbiIn) <= 2) ? 3 : 5);
}


gabest,

Am we on the right track?

Mark.

PS. Please excuse my ignorance, buy what does the -> and ? mean?

Since the "actual worst case is 43/51 bpp currently" you have to change the max buffer size to

  return lpbiIn->biWidth * lpbiIn->biHeight * ((GetBitmapType(lpbiIn) <= 2) ? 6 : 7);

6*8=48bit > 43bit
7*8=56bit > 51bit

[gabest]

Originally posted by Mark Fredrickson
PS. Please excuse my ignorance, buy what does the -> and ? mean?

-> is for accessing a member of a pointer pointing to a structure/class
" ? : " is just a simple "if then else".

[Steady]

Authorizations for attachments don't seem to go to me - sorry.

[western shinma]

gabest, why don't you just put it on your site?

[gabest]

http://vobsub.edensrising.com/get.php/huffyuv.dll

I've just recompiled it with the above mentioned modification, but havn't tried it.

[SleepEXE]

Regarding attachments, the way the bulletin board deals with them is really squirrelly. I haven't posted an attachment in a few weeks, but here's what worked at that time...

Go ahead an compose your message and attach the desired file, of course it won't appear immediately. Instead, click "Edit" and you will be able to view the URL to the attachment. Copy and paste the URL text into your message and the bulletin board will create a working link so people will be able to download it (assuming the "Automatically parse URL's" option is checked).

I've wondered why Doom9 hasn't announced it somewhere, but evidently it isn't common knowledge since Steady wasn't familiar with the workaround.

Best regards,
SleepEXE

[gabest]

LOL, I like smart ppl

huffyuv.zip


Update:

And it really works!

[Steady]

Well don't I feel like an idiot. For some reason I thought this was in the avisynth forum - I am not a moderator here anyway !

[Mark Fredrickson]

Hey, thanks guys. It works great!

Now, how do we publish this so others don't go though all the pain I've been in for the last couple of months?

My first thought was to have Doom9 and Luke (LukesVideo) add it to their software download page (both the dll and the source code fix).

Any other thoughts?

Oh, BTW, I just uncovered a bug with AVI_IO. It seems that Markus has coded AVI_IO to initially create 0 size files that maximize the amount of free disk space (eg. if you set your AVI_IO max file size to 1GB and you have 30GB of free disk space, 30 zero size files will be created when you start the capture). When he fills up the last file and the capture is still going, he reports a disk full error. However, it looks like he uses the Huffyuv reported buffer size to keep track of the amount he is writing to the file, but he actually only writes the true content of the buffer which is less than the reported size. This became very apparent after our patch since we increased the reported buffer size significantly. My 1GB files were only half full and the capture stopped (with a 'disk full') error when the last file was 'filled'. I've emailed Markus about this and hopefully he will fix it.

Does anybody know whether that CompressGetSize routine that we moded is suppose to return the actual size of the buffer, or the maximum allowed size?

Mark.

[gabest]

Platform SDK: Windows Multimedia / ICM_COMPRESS_GET_SIZE:

The ICM_COMPRESS_GET_SIZE message requests that the video compression driver supply the maximum size of one frame of data when compressed into the specified output format.

...

Return Values
Returns the maximum number of bytes a single compressed frame can occupy.

Remarks
Typically, applications send this message to determine how large a buffer to allocate for the compressed frame.

The driver should calculate the size of the largest possible frame based on the input and output formats.

[Mark Fredrickson]

Thanks gabest. So it looks like we got Huffyuv fixed correctly. Now it's up to Markus to fix AVI_IO. Hope he does.

[Doom9]

@sleepexe: you found a bug in the board code.. the mysql statement in the release code doesn't verify if the attachment has been moderated..

this "workaround" actually undermines the whole idea of post moderation (which we have to prevent warez attachments) so we'll have to implement the code fix rather than to announce this publicly. I understand that sometimes you'd like to have your attachments validated fast.. but there's 3 super moderators and 3 admins and many moderators so the chance to catch one and ask him to validate your attachment is pretty big.. and normally we check attachments daily

[SleepEXE]

Well, I guess I'll take that as a feather in my cap...never been credited with uncovering a security flaw of sorts. From the handful of times I've run across someone attempting to post an attachment, I got the impression the current behavior was undesirable, as opposed to awaiting a moderator to enable it.

I understand the need for moderating content, but with all due respect, I've seen attachments sit dormant for a few days awaiting authorization. I'm not implying some sense of entitlement to speedy service, but is there some sort of automated notification sent to the moderator when an attachment is made...alerting them that it needs attention?

Best regards,
SleepEXE

[Doom9]

talk to the vbb developers about such a feature.. many have requested it.. and it hasn't been done. sometimes I wonder why they don't include 3rd party code.. I've already submitted 2 working hacks, and there's many other proven solutions for things the board doesn't have but the developers just won't integrate it into the main code. I will put a "check attachements daily" into the moderation rules but that's all I can do.. I honestly have better things to do than check attachments but many times already did I have to do take care of that.