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

[Ruslan Savchenko]

On Tue, Nov 25, 2008 at 10:19 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sun, Nov 23, 2008 at 07:59:26PM +0300, Ruslan Savchenko wrote:
>
>> @@ -265,6 +273,7 @@ static void uninit(vf_instance_t *vf)
>>      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);
>>      free(vf->priv->outbuffer);
>>      free(vf->priv);
>>  }
>
> i dont see how it can still be allocated here
>

I don't know mplayer design so i was cautious if control would be
taken off from gen_fname() right after priv->fname allocation and then
given to vf uninitialisze code. But if you say this never happens then
that free() is futile.

>
>
> [...]
>
>> diff --git a/libmpcodecs/vf_screenshot.c b/libmpcodecs/vf_screenshot.c
>> index afc6770..eb122bb 100644
>> --- a/libmpcodecs/vf_screenshot.c
>> +++ b/libmpcodecs/vf_screenshot.c
>> @@ -26,10 +26,12 @@
>>  #include "m_struct.h"
>>
>>  #define SHOT_FNAME_LENGTH 102
>> +#define TIMESTAMP_LENGTH 20
>>
>>  struct vf_priv_s {
>>      int frameno;
>>      char *fname;
>> +    int style;
>>      /// shot stores current screenshot mode:
>>      /// 0: don't take screenshots
>>      /// 1: take single screenshot, reset to 0 afterwards
>> @@ -97,8 +99,37 @@ static int fexists(char *fname)
>>      else return 0;
>>  }
>>
>> -static void gen_fname(struct vf_priv_s* priv)
>> +static void gen_fname(struct vf_priv_s* priv, double pts)
>>  {
>> +    char *base = "shot";
>> +    char tstamp[TIMESTAMP_LENGTH];
>> +    char ts_sep;
>
>> +    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;
>
> this code does not look like it could work reliably
> double is not a real number in the mathematical sense its not exact,
> the way it rounds can be different in each calculation

Is the following code ok? ipts handles 2^32 msecs which is more than
490 days video length.

    long int ipts;
    uint32_t hour, min, sec, msec;
    ipts = lrint(pts * 100);
    hour = (uint32_t) (ipts/360000);
    min  = (uint32_t) (ipts/6000) % 60;
    sec  = (uint32_t) (ipts/100) % 60;
    msec = (uint32_t) (ipts) % 100;

>
> [...]
>> @@ -119,6 +122,20 @@ static void gen_fname(struct vf_priv_s* priv, double pts)
>>                    "%02" PRIu32 "%c%02" PRIu32 "%c%02" PRIu32 ".%02" PRIu32,
>>                     hour, ts_sep, min, ts_sep, sec, msec);
>>
>> +        if (priv->basename)
>> +           base = priv->basename;
>> +        else {
>> +            if (strstr(filename, "://"))
>
> please dont indent things randomly, even if this is mplayer code

OK. My bad, didn't notice this.

-- 
Regards,
savrus