Re: [Icecast] Icecast WebM Support Patch Second Edition

On 9 February 2012 06:12, Krad Radio <[email protected]> wrote:

> Updated full patch attached.

Some editorial issues:

> +       case FORMAT_TYPE_EBML:
> +               ret = format_ebml_get_plugin (source);
> +               break;

These tab indents should be spaces.

> +#include "logging.h"
> +
> +#define CATMODULE "format-ebml"

You have to define CATMODULE before including logging.h. It sets the
logging prefix.

> +    ret = client_send_bytes (client,
> +                             ebml_client_data->header->data + ebml_client_data->header_pos,
> +                             len);

It would be better to indent the continued arguments eight spaces and
stay in 72 columns instead of lining up with the first argument. Also
matches local style.

You're also not consistent on whether '{' is on a new line or not.
Local style is pretty strongly in favour of using a new line.

> +                            ERROR0("EBML Header to large, failing");

That should be, "EBML Header too large".

> +static unsigned char ebml_get_next_match_byte(unsigned char match_byte, uint64_t position,
> +                                              uint64_t *matched_byte_num, uint64_t *found);

You can make can make the search function much shorter by calling
memchr on the first byte, followed by memcmp for the full element id
when there's a match. Easier to read too. Unless there's some reason
you can't keep four bytes of look-ahead?

 -r