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

[Michael Niedermayer]

On Sat, Nov 15, 2008 at 03:03:41AM +0300, Ruslan Savchenko wrote:
> On Sat, Nov 15, 2008 at 2:47 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > 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 ?
> 
> Because of the last -vf screenshot. Without it
> "video3.mkv.[00:XX:YY.ZZ].png" comes out.
> 
> >
> >
> >>
> >> >> +        /* 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 ?
> 
> Oops, my bad. I will start fixing this memleak right now.
> 
> >
> > 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
> >
> 
> Attached filename moving patch.
> 
> > [...]
> >> --- 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
> >
> 
> Sorry I don't understand you. Should I send this path before the main
> one? Attached in such case.

yes thats what ive meant

both attached patches look ok IMHO

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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/4ea1e3f7/attachment.pgp>