Stale comment referring to nonexisting WriteMetadata() function

[juj]

brotli/c/enc/encode.c

Line 932 in 9ff341d

bits use WriteMetadata() to append an empty meta-data block.

references a function WriteMetadata(), but no such function seems to exist in the repository.

Maybe it is intended to refer to the function WriteMetadataHeader, or maybe it is referring to an earlier now deleted function BrotliEncoderWriteMetadata?

[juj]

Btw, commit 0a63f99 removed the WriteMetadataHeader function, and the commit message says * added BROTLI_OPERATION_EMIT_METADATA instead. Is that something that should be available to users from the command line, or is it just an internally supported operation?

[eustas]

This was supported by encoder back then, but not in decoder. Earlier this year I've added API to access metadata seen by decoder. Hopefully soon this will be available in command line tool for "watermarking" (see #628, #827)

[juj]

Thanks @eustas , that makes sense.

At Unity, we have been relying for quite some time (since 2016) on a manual fork of the encoder that added a feature to inject a custom string. This is used to identify whether a browser fed us a brotli-compressed or uncompressed data stream.

Recently we have been working on getting us to a newer version of Brotli to clear some CVEs that were raised. As part of that update, I have tried to bring back the WriteMetadata function since that is what was used at the time. I had to change the code a little bit, as last_byte and last_byte_bits had been renamed to last_bytes and last_bytes_bits. (iiuc this was a u8 to u16 expansion of some trailing bytes going into a bit writer, or similar)

The code that I brought back is this: https://github.com/Unity-Technologies/brotli/pull/7/files#diff-f7342e36363c8b43f6debffb54f2de0d6d958a48320e4b7811efe2bc4942ead3R924-R964

@eustas I wonder if you might be able to know from a glance if there is any hope from that code still being correct? Should I expect to be able to utilize that kind of metadata serialization construct in the latest codebase branch?

[eustas]

Sure, will take a look tomorrow morning.

[eustas]

At glance looks ok. Perhaps stream_state_ and is_last_block_emitted_ should be checked and set accordingly (if metadata block is last).

[juj]

Great, thanks for taking a peek! I'll read more into those parts, and see how to accommodate that in if needed.