[MPlayer-dev-eng] [SURVEY] change mencoder's default ofilename extension?

[The Wanderer]

Oded Shimon wrote:

> On Tue, Nov 29, 2005 at 01:41:53PM -0500, The Wanderer wrote:
> 
>> Oded Shimon wrote:

>>> I do consider them equivalent, and i think my patch fixes both
>>> problems. Actually, I don't even consider the first one a
>>> problem. Someone does '-of mpeg' and gets a 'test.avi', he knows
>>> it's an mpg file since he expliticly asked, especially now with
>>> the big loud warning, and can rename it to 'test.mpg' himself...
>> 
>> However, the problem which the thread was discussing is the one
>> which you don't consider to be a problem. If you think that your
>> proposed solution to another problem makes the problem at hand
>> moot, then you can certainly argue that.
>> 
>> I think that your suggested patch fixes the problem you're aiming
>> it at, but counts as (at best) no more than a fairly ugly
>> workaround for the problem which was actually being discussed. I
>> would really prefer something which fixes both problems.
> 
> IMO this patch fixes both problems...

I suppose we'll just have to disagree on that - and since you're the
actual coding contributor, you probably have more say in the matter than
I do.

>>> Regarding your other remark - having the muxer choose the file
>>> extention isn't feasible either. muxer_mpeg has several
>>> extentions ("mpg", "mpeg", "vob"), muxer_lavf has many many more
>>> possible extentions, and the idea involves changing user's input
>>> which imo is just wrong.
>> 
>> Of course there are multiple valid extensions for some formats, but
>> surely choosing *one* of the known valid extensions is better than
>> leaving an unrelated and invalid extension in place.
> 
> OK, how about this idea, if the file is "test.avi", and -of mpeg was
> chosen, output "test.mpg" by default. it has the small advantage of
> overriding an explicit 'test.avi', but it would be silly to
> explicitly ask "-o test.avi -of mpeg". This is in addition to my
> patch, as it doesn't solve the other problem.

This is roughly what I was suggesting, except that I would have a) done
it for *all* muxers, not just the MPEG muxer, and b) done it only in the
case that no -o option was specified at all. (If the code is such that
there's no way to tell that other than by checking whether or not the
current filename is equal to the default, then the case is hopeless, but
otherwise...)

>>> I'm doing the "\n" pad thing and committing in a few days.. speak
>>> now or forever hold your peace?
>> 
>> Well, for one thing your patch assumes that the extension it is
>> trying to detect will always be lowercase, which is not necessarily
>> valid. For that matter, what happens with your patch if someone
>> specifies an output filename which *has* no extension, and thus
>> does not contain a period? Or if someone specifies an output
>> filename which contains multiple periods? (I've done a few of the
>> latter myself.)
> 
> About lowercase, easy fix by changing to 'strcasecmp'. (can anyone
> testify on it's portability? I think it should be ok..)

I didn't suggest strcasecmp exactly because I seem to recall that it is
not portable. If it is, then that does completely resolve that question.

> About 'no extention' and 'double extention', you obviously didn't
> read the e-mail I JUST send! I explained exactly how it was treated!

Hmm. Okay, you're right that I did somehow miss that explanation - but I
wrote the above having just looked at the code, and I didn't see how it
produces that effect. Looking at it again I see that I misinterpreted
the conditional in the for loop, so now I understand how it works;
objection withdrawn.

> Do you still have objections? Can I commit?

I don't think it's as good a solution to the first problem as would be
implementing a proper "choose the default extension based entirely on
the container format" mechanism, but other than that it seems to be okay
by me.

-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

Secrecy is the beginning of tyranny.