[mythtv] MythChannels checkin review (was: DVB todo list)

[Ben Bucksch]

Ramon Roca wrote:

>however I have no problem in being patient repeating them:
>
Please don't. We both wasted enough time on that already.

>>    * All the PID stuff
>>    
>>
>Currently mytchannels only feeds distinct types of PIDs when are known.
>Doesn't do nothing with playback ... I have no problem if you come with a distinct format to put all of them in a single column or whatever format of your preference, but in the meantime I coded in a certain way, you don't.
>
I think we *really* discussed this enough. I'm talking about recording 
(not playback), which your patch broke. IIRC, I did propose how you can 
do it, the change can be done within less than 5 minutes. Call it "under 
development" or whatever you want, but you need to fix this before it 
can go into CVS. Otherwise, you'd cause a regression for no good reason, 
which is not acceptable.

>>    * The freq conversion (not) in the backend
>>    
>>

>there was a bug introduced because a user can keep trying to tune to an invalid frequency that he thinks is ok without success
>
It's not a bug, if the user manually inserts invalid data in the 
database and it doesn't work. I didn't specify a unit in the DB docs I 
wrote, that's my fault, sorry, I'll do that with the next update.

>So I've fixed this on that way, you don't.
>
Yes, I wrote it intentionally in a certain way, with a reason (which I 
explained a lot), and you just go and change it, because you like it 
differently. You shouldn't be surprised, if I say no. I am not willing 
to discuss this (or the PID stuff) further.

>the only possibility that I do see here is to load those
>tables in separate scripts instead of cvs.sql/mc.sql etc to make them
>optional (i.e., only for DVB setup), not sure if it is a good solution since it will add an additional required step for some setups ans therefore introducing risks...
>
Some of the information (e.g. about satellites) can most likely be read 
from config files, which have to be read anyways to get the channel/freq 
information.

Maybe you can store some information in the code (or translation files) 
directly, e.g. the "de" -> "Deutsch" mapping.

Also, maybe you can roll several tables into one without losing 
information or flexibility.

Alternatively, maybe you could at least prefix the (really) dvb-specific 
tables with "dvb_" in their table name, that would make using and 
understanding the DB much easier IMHO.

And I'd really appreciate some documentation about what the tables are 
for, exactly. It may be obvious to you, but not to me, and it has to be 
obvious *without* reading any code.
For example (why docs are so important), there are some "sharable" field 
in some tables, but I have no idea what exactly they mean or of they are 
functional at all, and that although I did find an old post about in on 
the mailing list archive. Maybe that field would actually be 
applicatible for DVB with its parallel recordings, but I don't know.

But in general, I didn't look close enough at the tables to say that 
they must be changed before checkin.

[direct MythChannels module code]

>I do recommend you to do so, I'm sure that you'll find more issues there
>that can be helpful.
>
Honestly, with your reactions in this case, I am inclined to not care at 
all about MythChannels code and just put myself in user mode.

I'd be still glad to use it (apart from mplayer ;-P ).

>Again, I don't see any *stop issue* on any of this.
>
But I do, and to get my approval for checkin, you have to fix it.