[mythtv] CI Progress
linux.news at bucksch.org
Mon Aug 4 03:16:41 EDT 2003
Kenneth Aafl°y wrote:
>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
>>>> * Properties are lower case, without m_ (it makes no sense to only have half the properties start with m_), methods are upppercase.
>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
* 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
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
>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
>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.
More information about the mythtv-dev