[MPlayer-dev-eng] libvo changes

[Uoti Urpala]

On Sat, 2008-04-05 at 22:26 +0200, Reimar Döffinger wrote:
> On Sat, Apr 05, 2008 at 10:50:57PM +0300, Uoti Urpala wrote:
> > "struct vo" is appropriate at least for the way I think about the
> > variables. I think about those in an object-oriented manner so that each
> > "instance" of struct vo "is a VO".
> 
> My objection is that a name consisting of two characters is unsuitable
> for a global struct.

A plain "vo" would be bad for a global, but IMO "struct vo" isn't
really. It would be bad if it was in a header exported to other
programs, but I don't expect it to cause practical problems if used
inside MPlayer (so there would only be problems if a system header also
defines "struct vo").

> > I on the other hand dislike the overuse of typedefs in much of MPlayer
> > code. The struct name should be available to allow pointers without the
> > full definition, but if you also have a typedef then it's less obvious
> > which struct and typedef names correspond to each other.
> 
> If you e.g. give them the same name or the same name with _s and _t it
> can hardly get any more obvious that they correspond.

Maybe it would be obvious if that was the practice for all typedefs, but
currently it isn't.

> > IMO the
> > "struct" part makes parsing code easier as it's obvious what is a type
> > name and what is a variable name.
> 
> That IMO is fairly obvious by the place where they are. Though that you
> object to placing declarations always at a start of a block makes that
> less obvious I admit.

Even if you always group declarations together it's IMO not so obvious.
A block could still start with declarations or with other kinds of code,
and at some point it'll probably switch between the two.

> > If you've already parsed the code and
> > it doesn't make sense otherwise then it must be a type name and "struct"
> > would not give (much) extra information, but IMO that takes more effort.
> 
> The _t/_s that is used in other places in MPlayer makes it rather
> obvious too without quite as much explicitness.

It's not consistently used. And *_t is reserved (though admittedly the
probability of using reserved identifiers causing problems isn't very
high).

> >  I don't usually consider the struct part clutter. "struct vo" is in
> > fact shorter than your proposed "vo_context".
> 
> vo_context could of course be made vo_ctx or vo_ctx_t, but the
> difference is also that I don't see how the "struct" part provides any
> useful information whatsoever, there is not really anything it could be
> besides a struct.

Depends on the context. Without any context 'struct vo' does tell a lot
more than 'vo'. Somewhere in a program you may be able to tell that it
"really can't be anything else", but there's still a difference in how
obvious that is.

> > It does contain a space
> > but that's IMO mainly a matter of being used to reading code with struct
> > types and cannot really be considered more cluttery in absolute terms.
> 
> Nevermind the space, though even in that interpretation it seems very
> much like an argument against it, or do you have any reason to assume
> that the majority of current and/or future MPlayer developers would be
> "used to reading code with struct types"? Not to mention new developers
> that are hardly born "used" to it...

Why not? It's not like using typedefs for everything would be a standard
practice for all projects. You seem to assume that everyone coming from
outside MPlayer would be used to reading typedef names but not used to
reading "struct", but I see no reason for such an assumption.

> Not to mention that even assuming a developer "used" to it I think you
> will have a hard time arguing it does not probably make reading harder.

Depends... "struct" is an extra word (though the identifier might have
to be longer without "struct"), but OTOH it simplifies parsing the code.
How much harder do you think "const" makes reading? It's an extra word
with less parsing benefit that "struct" has.


Just to make it clear, even though I think MPlayer code overuses typedef
names I don't mean that I would always be opposed to any use of typedefs
for structs.