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

[Diego Biurrun]

Please don't top-post, thank you.

On Mon, Nov 30, 2009 at 08:54:04PM +0100, Emiel Neggers wrote:
> 
> Included is an updated version of the rar patch. It plays video files
> from uncompressed, multi-volume rar archives with full seek support.
> 
> usage: mplayer file.rar  or  mplayer rar://file.rar
> 
> This patch has been successfully tested on:
>  Debian GNU/Linux unstable (sid)
>  Ubuntu 9.04 (Jaunty Jackalope)
>  Fedora 11
>  Linux Mint
> 
> Please review or comment.
> 
> --- stream/stream_rar.c	(revision 0)
> +++ stream/stream_rar.c	(revision 0)
> @@ -0,0 +1,157 @@
> +/*  
> + *  This program is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program.  If not, see <http://www.gnu.org/licenses/>.

You have to license your code as GPL v2+ for it to be acceptable to us.

> +static int seek(stream_t *s,off_t newpos) {
> +//static int seek(stream_t *s,long long unsigned int newpos) {
> +  struct stream_priv_s* p = s->priv;
> +  s->pos = newpos;
> +  if(rar_seek(p->rar,s->pos)<0) {
> +    s->eof=1;
> +    return 0;
> +  }
> +  return 1;
> +}

Please use K&R style, 4 spaces indentation, be generous with spaces
around operators.

> +static int control(stream_t *s, int cmd, void *arg) {
> +  struct stream_priv_s* p = s->priv;
> +  switch(cmd) {
> +    case STREAM_CTRL_GET_SIZE: {
> +      *((long long unsigned int*)arg) = rar_size(p->rar);
> +      return 1;
> +    }
> +    case STREAM_CTRL_RESET: {
> +      rar_seek(p->rar, 0);
> +      //p->rar = rar_open(p->filename);

debug cruft?  There is more in other places.  Please remove
commented-out code.

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

This is an illegal identifier (starts with _ and an uppercase letter,
which is reserved for the system).  Also, it does not contain our
multiple inclusion guard prefix (MPLAYER_).

> +#ifndef __USE_FILE_OFFSET64
> +  #define __USE_FILE_OFFSET64
> +#endif

I think we already have this in CPPFLAGS.

> +/*
> +#define NAMING_OLD 0
> +#define NAMING_NEW 1
> +*/
> +
> +enum naming_scheme_t {
> +  NAMING_OLD = 0,  // .rar, .r00, .r01, ..
> +  NAMING_NEW       // .part01.rar, .part02.rar, ..
> +};

What is this for?

> +typedef struct {
> +} rar_archive_t;

The _t namespace is reserved for POSIX, please do not pollute it.

Diego