[MPlayer-dev-eng] vo equalizer cleanup

[Reimar Döffinger]

On Mon, May 02, 2011 at 01:15:22PM +0300, Ivan Kalvachev wrote:
> On 5/2/11, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> > On 2 May 2011, at 01:35, Ivan Kalvachev <ikalvachev at gmail.com> wrote:
> >> However instead of creating new structures for GET and SET I decided
> >> to reused the single vf_equalizer structure from video filter system.
> >> This allows the structure to be directly passed through vf_vo.
> >> Because I use the very same structure, some files also need to include
> >> the vf.h .
> >> I could change that if you object on it, put please provide a clean
> >> way to ensure that the vo structure would be the same as the vf one.
> >> I picked the same structure because my plan is to convert vo to the vf
> >> api.
> >
> > In that case, why not add a vfcontrol.h header, include that from
> > video_out_internal.h and move things over there as needed?
> 
> Instead of creating new headers, I could just include vf.h from
> video_out_internal.h .

That includes a whole lot of other stuff, which I think is a really
bad idea. The vos have a lot of OS-specific code, thus there is a
particularly high risk of namespace collission and we should absolutely
avoid including half of MPlayer's header files within them.
Once you accidentially happen expand this inclusion mess so far that
the vos end up including loader/ headers you're almost certainly
at a "s..tuff hits the fan" stage.

> However this would break the rule for header files inclusion, that we
> ported from FFmpeg. Aka - each file should include the headers
> directly needed by the stuff it contains. So if I use vf_equalizer I
> should include the file where it is defined - vf.h .

Actually it was mostly that headers should compile stand-alone so that
changes in a .c file will not suddenly cause compilation errors in
an included .h file.
For anything else common sense should be used.
Here video_out.h defines VOCTRL_SET_EQUALIZER, so it really should
also provide the struct since without it that define can't be used.
And it's perfectly fine to assume that including whatever provides
VOCTRL_SET_EQUALIZER will also provide the struct necessary to use it.
However with you planned changes we might want to remove both from
video_out.h in the long term, but that's nothing that has to be handled
now.

> So do you still want me to include vf.h from video_out_internal.h, and
> remove the vf.h inclusions from my patch?

I mostly do not want all of vf.h to be pulled into (almost) all vos.
I think it would be reasonable to avoid extra includes, but I don't
have much of an opinion really.