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

[Uoti Urpala]

On Sun, 2008-11-23 at 19:59 +0300, Ruslan Savchenko wrote:
> All attached patches except of 0001 and 0008 refer to vf_screenshot.c.
> Here is a brief explanation about each patch:
> 
>  0001-Move-filename-variable-to-global-in-mencoder.c.patch
>     Move 'filename' variable outside main() to global in mencoder.c.

I still think this is a bad idea. A filter shouldn't rely on such
globals existing. What if the "file" is an URL? Standard input? You'd
need some kind of policy for naming the output, and it does not really
belong in a filter.

If functionality like this is really required then I think there should
be top-level code deciding what subdirectory or prefix to use for all
output related to a file, whatever filter or other part generates it. 

> In mplayer.c 'filename' is already global.

I've already removed that global in my git tree.


> 0002-Replace-magic-number-102-by-SHOT_FNAME_LENGTH-macro.patch
>     Define SHOT_FNAME_LENGTH and use it instead of magic numbers.
> 
> 0003-Change-priv-fname-allocation-from-static-to-dynamic.patch
>     Allocate priv->fname dynamically in gen_fname() and free it in
> put_image(). Dynamic allocation is required by further patches.

Seems the filename is only used immediately after being generated and
there is no need to allocate permanent storage for it. It should be
possible to store it in a local variable with much simpler code.