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

[Michael Niedermayer]

On Sat, Nov 15, 2008 at 02:28:03AM +0300, Ruslan Savchenko wrote:
> On Sat, Nov 15, 2008 at 1:16 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Fri, Nov 14, 2008 at 10:24:47PM +0300, Ruslan Savchenko wrote:
[...]
> >
> > also does this code work when there are several files on the command line?
> > or does it always end up using the first filename?
> 
> It works with several files. You can even specify different options as in here:
> mplayer video1.mkv -vf screenshot=1 video2.mkv video3.mkv -vf screenshot
> I entered 's' once in each file, this produced:
> *** screenshot 'video1.mkv.[00:00:00.46].png' ***
> *** screenshot 'video2.mkv.[00:02:30.38].png' ***
> *** screenshot 'shot0002.png' ***
> 
> (shot0002.png is because i've already had shot0001.png in my directory)

Why shot000?.png instead of video3.mkv.[00:XX:YY.ZZ].png ?


> 
> >> +        /* 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?
> 
> I believe i do free() in uninit():

is uninit() called after each screenshot ?

[...]

> >
> >
> > [...]
> >> 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?
> 
> Because in mencoder.c "filename" is declared inside main(). In
> mplayer.c "filename" is global and I moved it to global in mencoder.c.

right, mplayer doesnt use indention, how could i forget, sorry.
the filename moving is ok IMHO, mplayer & mencoder should either both
or none have a global filename.

also spliting is always better than not spliting.
And the more independant patches are the quicker some will be applied

[...]
> --- libmpcodecs/vf_screenshot.c	2008-11-15 01:50:22.000000000 +0300
> +++ ../mplayerb/libmpcodecs/vf_screenshot.c	2008-11-15 02:09:52.000000000 +0300
> @@ -157,7 +157,7 @@
>          /* allocate space to handle "shotXXXX.png" filenames */
>          priv->fname = malloc(SHOT_FNAME_LENGTH);
>      do {
> -	snprintf (priv->fname, 100, "shot%04d.png", ++priv->frameno);
> +	snprintf (priv->fname, SHOT_FNAME_LENGTH, "shot%04d.png", ++priv->frameno);
>      } while (fexists(priv->fname) && priv->frameno < 100000);
>      if (fexists(priv->fname)) {
>  	priv->fname[0] = '\0';

if that didnt depend on the other patch, it would be ok

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

Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- 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/20081115/8117ad3e/attachment.pgp>