[MPlayer-dev-eng] [patch] play and seek files inside multivolume uncompressed rar files

Diego Biurrun diego at biurrun.de
Fri Dec 18 10:19:07 CET 2009


On Tue, Dec 08, 2009 at 08:12:02PM +0100, Emiel Neggers wrote:
> 
> Please let me know if further changes are needed.
> 
> --- stream/stream_rar.c	(revision 0)
> +++ stream/stream_rar.c	(revision 0)
> @@ -0,0 +1,150 @@
> +
> +static int fill_buffer(stream_t *s, char* buffer, int max_len){

K&R has the { on the next line for function declarations.

> +    struct stream_priv_s* p = s->priv;
> +    int r = rar_read(p->rar,buffer,max_len);
> +    return (r <= 0) ? -1 : r;
> +}
> +
> +static int seek(stream_t *s,off_t newpos) {

Sometimes you have space after commas, sometimes you don't; you should
always have them.

> +    struct stream_priv_s* p = s->priv;
> +    s->pos = newpos;
> +    if( rar_seek(p->rar, s->pos) < 0 ) {

if (

Drop the spaces inside parentheses, which you sometimes have.

> +        s->eof=1;

spaces around operators; you use it most of the time.

> +static int control(stream_t *s, int cmd, void *arg) {
> +    struct stream_priv_s* p = s->priv;
> +    switch(cmd) {
> +        case STREAM_CTRL_GET_SIZE: {

K&R indents switch and case at the same depth.

> --- stream/unrar.c	(revision 0)
> +++ stream/unrar.c	(revision 0)
> @@ -0,0 +1,386 @@
> +
> +#define UINT32(b) ( (*((b)+3))<<24 | *(((b)+2))<<16 | *(((b)+1))<<8 | *(((b)+0)) )

?

> +int _rar_init_part(rar_archive *rar) {

Identifiers should not start with _.

> --- stream/unrar.h	(revision 0)
> +++ stream/unrar.h	(revision 0)
> @@ -0,0 +1,151 @@
> +
> +#ifndef MPLAYER_UNRAR_H
> +  #define MPLAYER_UNRAR_H

Don't indent the preprocessor statement, we do this nowhere else.

> +typedef struct {
> +} rar_archive;

What is it with typedefs on structs?  I don't think they make things
more readable.  I suggest avoiding them.

> --- Makefile	(revision 29978)
> +++ Makefile	(working copy)
> @@ -510,6 +510,8 @@
>                stream/stream_mf.c \
> +              stream/unrar.c \
> +              stream/stream_rar.c \
>                stream/stream_null.c \
>                stream/url.c \

alphabetical order

In any case, this sounds like a feature that should have a disable option.

Diego



More information about the MPlayer-dev-eng mailing list