[mythtv] CI Progress

[Kenneth Aafløy]

Ben Bucksch wrote:
> Kenneth Aafløy wrote:
> >
> >Can't understand why you 'mix and match' class and C functions
> >
> I just explained why. Please don't change the style (esp. if that
> results 1000 line changes), unless you asked first and made sure that
> it's OK or there's a good technical reason to do so. In this case, there
> is a good technical reason *not* to use your change.
>
> BTW: It's not a real C function, it uses e.g. references. It's just a
> normal function.
Final word: References are a part of the C standard, what you want to say is
that it uses (amongst others a class part of libmythtv) therefor it is not a
C function, which is why it belongs in the class!

> >Originally wanted to move it into DVBChannel, as it belongs there, basic
> >classwriting.
> >
> DVBSettings is not part of the class interface, it's part of the
> implementation, same for FetchDVBTuningOptions.
>
> >I was trying to move pid setting out of DVBChannel, it has no reference
to tuning at all. Besides, whats the point of using a vector to store it
when it actually will become a C array?
> >
> Because vectors are far more convient and saver to use (no fixed
> boundaries, no boundary checking, no mem management etc.), and we need
> it in other places as well, not just for ts_to_ps(). It's a very
> unfortunate fact that I have to use C for ts_to_ps (lib boundary), not
> something to extend through the whole code.
But what about hardware/driver limits (#define DMX_FILTER_MAX 16), why
didn't you follow this when you did the class/functions?

I can drag the ts_to_ps+callback into the class if you want to, then the
only thing outside is the lib, same as any other place in libmythtv!

> >>    * Properties are lower case, without m_ (it makes no sense to only
> >>      have half the properties start with m_), methods are upppercase.
> >>
> >>
> >Agree!
> >
> (But you added Channel and m_settings properties.)
I've seen m_* used other places!

I'm trying to make a point here!!

What about starting to clean up everything (put guidelines somewhere), make
the code readable by (almost) everyone!

> >>    * What is that "version"? Is that a replacement for "bool
> >>      pid_changed;". Why is the latter insufficient?
> >>
> >>
> >So where do I unset it
> >
> When you are sure that you made all necessary changes. If you want to
> keep your solution (it would make some sense, if you need to make
> changes in several, independent functions), please change the name to
> |settings_change| or similar and make a comment what it's intended for.
Fair enough!

> >>    * Why did you change the way the PID processing works?
> >>
> >Out of DVBChannel, planning on writing a new demux function, because I'll
> >need it if I'm going to write a Table parser (EPG/PMT...).
> >
> I'll need a much more detailed explanation.

Sorry, i'm planning (as you know) to write support for downloading the
Program Map Table and other tables from the ts. And also to make it support
recording to different files (if we can stop arguing and start coding)!

I will make it poll multiple fds, and filter as required.

Ok?

> >Then comment before I get close to releasing, gave a chance last time and
> >gave a chance this time, and not with one-liners explaining notting!
> >
> I don't understand. You know that I was on vacation. I just took one
> hour to look over your patch. I tried to make comments as early as
> possible, so that you don't invest time into stuff that will be
> rejected. If you want me to hold back my comments until you're done,
> that's even easier for me, costs me less time, but please understand
> that I'll decline changes that I think are worse or no better than the
> current solution.

If you care enough to read my patches, I appreciate it a lot. If you don't,
well then there is not much I can do about that.

I will, most of the time, release a test/viewing version of my patch (when I
feel I've done something right). If I don't get much feedback I clean it up,
and submit it.

I don't want to argue over this, there's no point, and get's us nowhere.

I have to say that my patch work a hell of a lot better than what was in CVS
before I started reading docs/sources.


Kenneth