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

[Ruslan Savchenko]

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:
>> Any comments? (sorry if I didn't wait long enough)
>> Attached patch also updates man page, but the code part is the same.
>
>> +    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

What should i use instead svn diff to create a patch? I did one with
diff -Naur and attached, is it ok?

>
> 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)

>> +        /* 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():

@@ -288,18 +346,33 @@
     av_freep(&vf->priv->avctx);
     if(vf->priv->ctx) sws_freeContext(vf->priv->ctx);
     if (vf->priv->buffer) free(vf->priv->buffer);
+    if (vf->priv->fname) free(vf->priv->fname);
+    if (vf->priv->fnamebase) free(vf->priv->fnamebase);
     free(vf->priv->outbuffer);
     free(vf->priv);

>
>
> [...]
>> 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.
It is needed, because vf_screenshot is linked with mencoder.
In the thread opening message there was a separate patch for
mencoder.c doing this slip. Sorry if i distracted you by sending
incomplete patch (only for vf_screenshot.c file) in the middle of
discussion. I don't quite understand if i should split them or no (but
i see now i should always send all parts).


Thanks again for comments, fixed patch attached.

-- 
Regards,
savrus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vf_screenshot-filename.patch
Type: application/octet-stream
Size: 6669 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20081115/a8639b58/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vf_screenshot-shotfnamelength.patch
Type: application/octet-stream
Size: 555 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20081115/a8639b58/attachment-0001.obj>