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

[Ruslan Savchenko]

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


-- 
Regards,
savrus