[MPlayer-dev-eng] [PATCH] JPEG video output to multiple subdirectories v2

[Attila Kinali]

On Wed, Jul 14, 2004 at 08:04:28AM +0200, Ivo wrote:
> My previous patch didn't preserve the pre-patch behaviour. This patch does. 
> If -jpeg subdirs=somedir isn't specified, it acts just like before and 
> writes all JPEG files to . or outdir.
> 
> Thanks to KotH on IRC for pointing this out :)

And more comments to come :)

> ? vo_jpeg_patch
> Index: cfg-mplayer.h
> ===================================================================
> RCS file: /cvsroot/mplayer/main/cfg-mplayer.h,v
> retrieving revision 1.221
> diff -u -r1.221 cfg-mplayer.h
> --- cfg-mplayer.h	28 Jun 2004 12:17:36 -0000	1.221
> +++ cfg-mplayer.h	14 Jul 2004 06:03:20 -0000
> @@ -36,6 +36,8 @@
>  extern int jpeg_smooth;
>  extern int jpeg_quality;
>  extern char * jpeg_outdir;
> +extern char * jpeg_subdirs;
> +extern int jpeg_maxfiles;
>  #endif
>  #ifdef HAVE_SDL
>  //extern char *sdl_driver;
> @@ -149,6 +151,8 @@
>  	{"smooth", &jpeg_smooth, CONF_TYPE_INT, CONF_RANGE, 0, 100, NULL},
>  	{"quality", &jpeg_quality, CONF_TYPE_INT, CONF_RANGE, 0, 100, NULL},
>  	{"outdir", &jpeg_outdir, CONF_TYPE_STRING, 0, 0, 0, NULL},
> +	{"subdirs", &jpeg_subdirs, CONF_TYPE_STRING, 0, 0, 0, NULL},
> +	{"maxfiles", &jpeg_maxfiles, CONF_TYPE_INT, 0, 0, 0, NULL},
>  	{NULL, NULL, 0, 0, 0, 0, NULL}
>  };
>  #endif
> Index: DOCS/man/en/mplayer.1
> ===================================================================
> RCS file: /cvsroot/mplayer/main/DOCS/man/en/mplayer.1,v
> retrieving revision 1.622
> diff -u -r1.622 mplayer.1
> --- DOCS/man/en/mplayer.1	13 Jul 2004 20:59:47 -0000	1.622
> +++ DOCS/man/en/mplayer.1	14 Jul 2004 06:03:27 -0000
> @@ -1926,6 +1926,10 @@
>  Quality factor [0\-100]
>  .IPs outdir=<value>
>  Directory to save the JPEG files
> +.IPs subdirs=<value>
> +Prefix of the subdirectories to which the JPEG files are saved.

This should be rather something like:
"If specified mplayer will create numbered subdirectories with the
specfied prefix. If unspecified no subdirectories are created." 
(someone should clean up my english ^^')

> +.IPs maxfiles=<value>
> +Maximum number of JPEG files to be saved per subdirectory.

Mention the default value here

>  .RE
>  .PD 1
>  .
> Index: libvo/vo_jpeg.c
> ===================================================================
> RCS file: /cvsroot/mplayer/main/libvo/vo_jpeg.c,v
> retrieving revision 1.11
> diff -u -r1.11 vo_jpeg.c
> --- libvo/vo_jpeg.c	25 Apr 2003 20:37:26 -0000	1.11
> +++ libvo/vo_jpeg.c	14 Jul 2004 06:03:28 -0000
> @@ -36,6 +36,8 @@
>  int jpeg_smooth = 0;
>  int jpeg_quality = 75;
>  char * jpeg_outdir = ".";
> +char * jpeg_subdirs = "";

char * jpeg_subdirs = NULL would be better;

> +int jpeg_maxfiles=1536;
>  
>  static int framenum=0;
>  
> @@ -91,10 +93,34 @@
>  
>  static uint32_t draw_frame(uint8_t * src[])
>  {
> - char buf[256];
> + static uint32_t framecounter=0, subdircounter=0;
> + char buf[512];

move the 512 to a #define so it needs to changed only at one
place the next time.
(dont forget to undef it after it's last use)

>   uint8_t *dst= src[0];
> -    
> - snprintf (buf, 256, "%s/%08d.jpg", jpeg_outdir, ++framenum);
> + static char subdirname[256] = "";
> +
> +/* Create jpeg_outdir at first call; if it already exists, no harm is done */
> + if (framecounter==0 && subdircounter==0) {
> +  snprintf (buf, 512, "%s", jpeg_outdir);
> +  mkdir (buf, 0755);

MPlayer should complain here if anything else but 0 or EEXIST is
returned. On EEXIST a check for write permission should be performed
IMHO.

> + } 
> +
> +/* Start writing to new subdirectory after a certain amount of frames */
> + if ( framecounter == jpeg_maxfiles ) {
> +  framecounter = 0;
> + }
> +
> +/* If framecounter is zero (or reset to zero), increment subdirectory number.
> + * If jpeg_subdirs is not set, resort to old behaviour. */
> + if ( framecounter == 0 && strcmp(jpeg_subdirs, "") ) {

if jpeg_subdirs is initialized to NULL only a
if( !framecounter && jpeg_subdirs)
would be enough (beside that it's easier to read.

> +  snprintf (subdirname, 256, "%s%08d", jpeg_subdirs, subdircounter++);
> +  snprintf (buf, 512, "%s/%s", jpeg_outdir, subdirname);
> +  mkdir (buf, 0755);

Again check of return value and complain if != 0|EEXIST

> + }
> +
> +/* snprintf the full pathname of the outputfile */
> + snprintf (buf, 512, "%s/%s/%08d.jpg", jpeg_outdir, subdirname,++framenum);
> +
> + framecounter++;
>  
>   return jpeg_write( buf,src[0] );
>  }


Otherwise i would say it looks ok, but i havent read the code of vo_jpeg
itself, so i cannot say for sure.

			Attila Kinali