[mythtv] CI Progress

[Ben Bucksch]

Kenneth Aafløy wrote:

>Final word
>
aha.

>But what about hardware/driver limits (#define DMX_FILTER_MAX 16), why
>didn't you follow this when you did the class/functions?
>
These limits don't apply, if we filter the PIDs/streams ourselves using 
ts_to_ps(). This is one of the main reasons why that function exists at all.

>I can drag the ts_to_ps+callback into the class if you want to
>
No. I intentionally left it in dvbdev, because it heavily depends on the 
code in dvbdev. I ideally only wanted a single function call for the 
conversion/filtering (so that it's easy to replace with another, better 
lib, e.g.) - the fact that I have to set up the PID and ipack arrays in 
dvbchannel annoys me already. mpegtools (part of dvbdev) are horrible, 
and I'd prefer to have them replaced with better code one day, e.g. 
MPSYS <http://www.nenie.org/misc/mpsys/>, when it supports PS. That's 
only harder, if you extend the dvbchannel-code depending on 
mpegtools/dvbdev.

>>>>   * Properties are lower case, without m_ (it makes no sense to only have half the properties start with m_), methods are upppercase.
>>>>
>>>Agree!
>>>
>I've seen m_* used other places!
>
But not in dvb*, and not *consistently* in the rest. And it's only 
making things worse, if half of the class/code has m_ and the other half 
doesn't, because the reader will wrongly assume that vars without m_ are 
local vars (that assumption is the point of that m_). In that case, it's 
really an all-or-nothing question.

>I'm trying to make a point here!!
>
But you ended up making exclamation marks. ;-P

>What about starting to clean up everything (put guidelines somewhere), make the code readable by (almost) everyone!
>
OK, fine. Make a coding guideline for MythTV, get consensus or approval 
on it, submit a patch that changes the code to follow that guideline. I 
complain because

    * I personally don't agree with the new style
    * you mix these extensive style changes with (small?) bug fixes in a
      single patch, without appearant relation
    * you fix only half of the file, which makes things even worse than
      they are currently
    * you didn't document what you changed and why

Do you really think that |malloc|ed struct (defined in a header file) is 
an improvement over a local struct var (hidden in the impl) that already 
works fine? Sorry, I don't, and I am not happy, if you mess up my code 
that way.

If you want to submit a bugfix or a cool new feature, great. If you 
really need to change the code to make that possible, also OK. But 
please try to keep the changes small, and propose/submit the 
style-"cleanup" later.

>i'm planning (as you know) to write support for downloading the
>Program Map Table and other tables from the ts.
>
I know, and I am looking forward to it.

>And also to make it support recording to different files
>
You mean several, independent recordings at the same time from the same 
card? Great! :-)

>(if we can stop arguing and start coding)!
>
Hey! I did my share already, remember?

If I wrote some code, intentionally in a certain way, and you change 
that code in a way that's IMHO worse, then I may say so. I could do what 
others do and just say in 2 lines that I dislike your change, but I take 
the time to explain things in detail. Maybe I shouldn't. If you disagree 
with me, fine, but don't accuse me of arguing, because it's you who is 
arguing, not me. Same goes for Ramon, BTW.

[PID filtering changes]

>I will make it poll multiple fds, and filter as required.
>
I still don't see why you need to change the existing PID code for that. 
Why not just call SetPID() (maybe adding a version accepting a vector of 
PIDs)?

>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 really tried to (I spent one hour, now over 2 hours with the 
discussion), but the patch made so many changes that even most of the 
functions were off, so the patch was totally unreadable. There were no 
comments or docs either. And, from your replies, it seems that my guess 
was right, that most changes (e.g. the DVBSettings move) indeed did 
*not* have a good reason, so it would have been futile for me to search 
for a meaning.

>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.
>
Quite possible. That doesn't make *all* of your changes desirable.