[MPlayer-dev-eng] vo equalizer cleanup

Ivan Kalvachev ikalvachev at gmail.com
Mon May 2 12:15:22 CEST 2011


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 .

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 .
If vf.h needs another defines, then it should include just the files
that contain them, they should do the same on their own , etc. This
way it ensures that we don't rely on one header including other
header, thus if we don't need some structure anymore we can remove the
include and not break the build.
make checkheaders actually checks for header self-containment.

Well, the rule in FFmpeg was relaxed and checkheaders doesn't (and
can't) check for unduly (unneeded extra) headers. So I could do what
you ask, it however would break the point of using that system, even
if it doesn't break any code.

I didn't like the system when it was introduced in FFmpeg, but even if
it have drawbacks, it have its merits too.

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


More information about the MPlayer-dev-eng mailing list