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

[Michael Niedermayer]

On Mon, Dec 01, 2008 at 07:42:34AM +0300, Ruslan Savchenko wrote:
> On Mon, Dec 1, 2008 at 1:19 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Tue, Nov 25, 2008 at 11:40:09PM +0300, Ruslan Savchenko wrote:
> >> On Tue, Nov 25, 2008 at 10:44 PM, Uoti Urpala <uoti.urpala at pp1.inet.fi> wrote:
> >> > On Sun, 2008-11-23 at 19:59 +0300, Ruslan Savchenko wrote:
> > [...]
> >
> >> >
> >> >> 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.
> >> >
> >>
> >> Yes, but the question is what length should be priv->fname? I used 256
> >> in very the first version of vf_screenshot patch but it seemed this
> >> number didn't satisfy everyone. Even I once had a video file with file
> >> name length more than 256 chars.
> >
> > question is
> > why is it in the context instead of a local var?
> >
> 
> That's how it was originally. But I don't see how storing it in a
> local var would simplify the code.

putting stuff that is just local semantically in the context is confusing
even if not simpler i think it would be more readable


> Do you mean just by changing
> malloc() to alloca() (but man pages say it's usage is discouraged)?
> Maybe I don't understand something again.

i did not mean alloca(), if i suggested something like that it would be
a variable sized ISO C array, but iam a little afraid of these with user
controlable sizes

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

It is not what we do, but why we do it that matters.
-------------- 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/20081201/730a2f0a/attachment.pgp>