[FFmpeg-devel] [PATCH] Matroska Muxer

[Diego Biurrun]

On Mon, Aug 13, 2007 at 08:38:25PM -0400, David Conrad wrote:
>
> Comments and more testing welcome.

Some nits below, the build system changes are OK.

> --- /dev/null
> +++ b/libavformat/matroskaenc.c
> @@ -0,0 +1,811 @@
> +/*
> + * Matroska file muxer

I think just Matroska muxer is enough.

> +// 2 bytes * 3 for EBML IDs, 3 1-byte EBML lengths, 8 bytes for 64 bit offset, 4 bytes for target EBML ID
> +#define MAX_SEEKENTRY_SIZE 21
> +
> +// per-cuepoint-track - 3 1-byte EBML IDs, 3 1-byte EBML sizes, 2 8-byte uint max
> +#define MAX_CUETRACKPOS_SIZE 22

C has multiline comments :)  Please keep lines below 80 characters where
it makes sense.

> +/**
> + * Calculate how many bytes are needed to represent a given size in EBML

Add a period at the end.

> +        // don't care how many bytes are used, so use the min
> +        bytes = needed_bytes;
> +    else if (needed_bytes > bytes) {
> +        // the bytes needed to write the given size would exceed the bytes
> +        // that we need to use, so write unknown size. This shouldn't happen.

> +/**
> + * Writes a void element of a given size. Useful for reserving space in the file to be
> + * written to later.

needlessly long line

> +    put_ebml_id(pb, EBML_ID_VOID);
> +    // we need to subtract the length needed to store the size from the size we need to reserve
> +    // so 2 cases, we use 8 bytes to store the size if possible, 1 byte otherwise

ditto

> +/**
> + * Initialize a mkv_seekhead element to be ready to index level 1 Matroska elements.
> + * If a maximum number of elements is specified, enough space will be reserved at
> + * the current file location to write a seek head of that size.
> + *
> + * @param segment_offset The absolute offset to the position in the file where the segment begins
> + * @param numelements the maximum number of elements that will be indexed by this
> + *                    seek head, 0 if unlimited.

again

> +        // 21 bytes max for a seek entry, 10 bytes max for the SeekHead ID and size,
> +        // and 3 bytes to guarantee that an EBML void element will fit afterwards

and again

> +/**
> + * Write the seek head to the file and free it. If a maximum number of elements was
> + * specified to mkv_start_seekhead(), the seek head will be written at the location
> + * reserved for it. Otherwise, it is written at the current location in the file.

and ..

.. some more occurrences can be found below ..

> +        av_log(codec, AV_LOG_WARNING, "no aac extradata, unable to determine sample rate\n");

AAC, samplerate, same below

Diego