On Fri Oct 20 01:24:10 2023 +0000, Alfred Agrell wrote:
Yeah, that entire function is confusing. It's supposed to return the max size of a compressed frame, but I don't think there is any real upper bound for that, you can cram in as many optional extensions as you want. So yes, it's guessing decompressed size as an upper bound. It'll break for 1x1 videos (can't fit the MPEG headers in three bytes), and videos crammed full of junk data, but let's just not care about those. But comments, sure, can do. And the Cinepak branch's comment is suspicious at best - what does biSizeImage have to do with compressed size?
wg_format_get_max_size() has basically three purposes currently:
* to fill the biSizeImage field, which is public API and is broadly *supposed* to represent the size of each video frame. For uncompressed video frames this is obviously a constant and easily calculable. For compressed video frames that's not the case, but the field exists in the DirectShow API anyway.
* Because that field represents the size of each video frame, we use it internally to determine how large are the samples we're going to allocate for our pool. This partly dates back to when we only handled uncompressed samples, but also, it's not really like we can do better for Cinepak.
* It's also used for IWMSyncReader::GetMaxStreamSampleSize(), which is also public API, but for a different frontend not related to DirectShow. The considerations are similar, except that I don't think WM pools, so we don't actually care in that case.
Given this context, why the Cinepak behaviour? The answer is: I believe AVI files basically encode a BITMAPINFOHEADER directly into the file, and so the native AVI demuxer just takes values from there, so it's up to the encoder what gets put in that field. We haven't yet needed to report the *true* value, because applications don't as a rule actually care about biSizeImage. Instead we just sort of copy the behaviour of one specific encoder, because it's good enough for *our* purposes.
What about MPEG-1? Well, same deal here. Nominally the right thing to return is what the native demuxer returns for biSizeImage, but that might not be easy to emulate exactly, or it might be some dummy value that won't work for allocating from the pool. Since it probably doesn't actually matter I don't think it's a bad thing to just overestimate.
(It's also worth noting that we clamp the minimum video size to 16 KiB just in case. That is probably overkill though, especially for uncompressed formats.)