[MPlayer-dev-eng] [PATCH] Initial Bluray support

Diego Biurrun diego at biurrun.de
Sun Sep 13 16:37:05 CEST 2009


On Sun, Sep 13, 2009 at 01:01:19PM +0300, Jonathan Nell wrote:
> 
> Feedback would be great!

Here it comes..

> --- stream/stream_bd.c	(revision 0)
> +++ stream/stream_bd.c	(revision 0)
> @@ -0,0 +1,226 @@
> +#include <stdio.h>
> +#include <malloc.h>

The standard license header is missing

> +static int bd_stream_open(stream_t *s, int mode, void* opts, int* file_format) {
> +  unsigned int byte;
> +  int i;
> +  char filename[200];
> +
> +  struct stream_priv_s* p=(struct stream_priv_s *)opts;
> +  bd_priv_t *bd=malloc(sizeof(bd_priv_t));
> +
> +  memset(bd,0,sizeof(bd_priv_t));

Please use K&R style and 4 spaces indentation for new files.  Place
spaces around operators and after commas, space after keywords, no space
after function names, opening brace for function declarations on the
next line.

> +  s->sector_size=BD_UNIT_SIZE;
> +  s->flags=STREAM_READ | STREAM_SEEK;
> +  s->fill_buffer=bd_stream_fill_buffer;
> +  s->seek=bd_stream_seek;
> +  s->close=bd_stream_close;
> +  s->start_pos=0;
> +  s->priv=(void *)bd;
> +  s->type = STREAMTYPE_BD;
> +  s->url=strdup("bd://");

This could be made more readable by aligning the =, more below.

> --- stream/stream_bd.h	(revision 0)
> +++ stream/stream_bd.h	(revision 0)
> @@ -0,0 +1,41 @@
> +
> +#include <stdio.h>

license header

> +typedef struct bd_priv {
> +} bd_priv_t;

The _t namespace is reserved for POSIX, please don't pollute it more.

You should eventually add some sort of check into configure.

Diego



More information about the MPlayer-dev-eng mailing list