[MPlayer-dev-eng] [PATCH] vf_screenshot, output filename corresponding to the played file

[Michael Niedermayer]

On Fri, Nov 14, 2008 at 10:24:47PM +0300, Ruslan Savchenko wrote:
> Any comments? (sorry if I didn't wait long enough)
> Attached patch also updates man page, but the code part is the same.

[...]
> +    uint32_t hour = (uint32_t) (pts/3600);
> +    uint32_t min  = (uint32_t) (pts/60) % 60;
> +    uint32_t sec = (uint32_t) pts % 60;
> +    uint32_t msec = (uint32_t) (pts*100) % 100;

the sec would look nicer with 1 more space



> +    size_t n;
> +
> +    if (priv->fnametype == 0) {
> +        /* allocate space to handle "shotXXXX.png" filenames */
> +        priv->fname = malloc(SHOT_FNAME_LENGTH);
>      do {
>  	snprintf (priv->fname, 100, "shot%04d.png", ++priv->frameno);
                               ^^^
should be replaced by SHOT_FNAME_LENGTH, and actually ideally
the 102->SHOT_FNAME_LENGTH could be a seperate patch


>      } while (fexists(priv->fname) && priv->frameno < 100000);
> @@ -101,6 +133,32 @@
>  	priv->fname[0] = '\0';
>  	return;
>      }
> +    }
> +    else if ((priv->fnametype == 1) || (priv->fnametype == 2)) {

maybe a switch/case would be slightly cleaner than this if, though its
more an issue if more cases get added.


> +        /* TIMESTAMP_LENGTH is enough for 2^32 seconds */
> +        if (priv->fnametype == 1)
> +            snprintf(tstamp, TIMESTAMP_LENGTH, "%02" PRIu32 ":%02" PRIu32 ":%02" PRIu32 ".%02" PRIu32, hour, min, sec, msec);
> +        else if (priv->fnametype == 2)
> +            snprintf(tstamp, TIMESTAMP_LENGTH, "%02" PRIu32 "-%02" PRIu32 "-%02" PRIu32 ".%02" PRIu32, hour, min, sec, msec);
> +            
> +        if (!priv->fnamebase) {
> +            if (strstr(filename, "://") != NULL)
> +                priv->fnamebase = strdup("shot");
> +            else {
> +                s = strrchr(filename, '/');
> +                if (s == NULL) 
> +                    s = filename;
> +                else
> +                    s++;

> +                priv->fnamebase = malloc(strlen(s) + 1);
> +                strcpy(priv->fnamebase, s);

strdup()

also does this code work when there are several files on the command line?
or does it always end up using the first filename?


> +            }
> +        }
> +        /* 8 is length of ".[].png" plus '\0' */
> +        n = strlen(priv->fnamebase) + strlen(tstamp) + 8;

> +        priv->fname = malloc(n);
> +        snprintf(priv->fname, n, "%s.[%s].png", priv->fnamebase, tstamp);

did i miss the free() or is this a memleak?


[...]
> Index: mencoder.c
> ===================================================================
> --- mencoder.c	(revision 27909)
> +++ mencoder.c	(working copy)
> @@ -115,6 +115,7 @@
>  char* audio_lang=NULL;
>  char* dvdsub_lang=NULL;
>  static char* spudec_ifo=NULL;
> +char* filename=NULL;
>  
>  static char** audio_codec_list=NULL;  // override audio codec
>  static char** video_codec_list=NULL;  // override video codec
> @@ -397,7 +398,6 @@
>  double v_timer_corr=0;
>  
>  m_entry_t* filelist = NULL;
> -char* filename=NULL;
>  
>  int decoded_frameno=0;
>  int next_frameno=-1;

why?


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is not what we do, but why we do it that matters.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20081114/60445a9b/attachment.pgp>