vdr-plugin-osdteletext

The patch adds a lot of code and some complexity but does not document any benefits for using MySQL as the storage system. Please add some words somewhere in the patch why one would like to use it over the other options.

There are some xxv references in the patch that should probably be changed to osdteletext.

The "top pages" feature doesn't seem dependent on MySQL or the rest of the patch, could you submit that separately first?

Sorry, i forget to tell about small improvements

This small patch let the plugin don't save "unviewable" pages (pages with hexadecimal number A-F)
It's reduce the load to store pages, otherwise with option --toptext the plugin store pages with hexadecimal number.

void cTelePage::save()
   if(g_StoreTopText ||
      ((page.page & 0xFF) <= 0x99 
      (page.page & 0x0F) <= 0x9 )) {

txtrecv.h:
Follow member of cTxtReceiver was removed

tChannelID chan;

bool running;

All other changes around class "storage" are required, to overload functional pointers for mysql-storage backend.

There are some xxv references in the patch that should probably be changed to osdteletext.

This reference are only inside sample file /mysql/osdteletext.cnf to defined a working database setup.
Replace this text inside /mysql/osdteletext.cnf like

#
# Copy this file $ETCVDR/PLUGINS/osdteletext.cnf
#
# to enable privileges into database DBNAME
#
# USE DBNAME;
# GRANT select,insert,update,create,delete ON DBNAME.* TO osdteletext@'localhost';
# SET PASSWORD FOR osdteletext@'localhost'=PASSWORD('alobComhab7');
# FLUSH PRIVILEGES;
#

[client]
user=osdteletext
password=alobComhab7
port = 3306
host = localhost
database = DBNAME

anbr wrote:

[...]
---> Already defined into cTelePage :

That's mean cReceiver
cTxtReceiver::chan = cReceiver::ChannelID()

...

cTxtReceiver::running = cThread::Running()

In which way does it improve the osdteletext plugin, if a mysql backend is added? IMHO the existing caching mechanism should be improved to work better and there shouldn't be added a second one, which also requires a mysql daemon to be running to work.

OK, for external usage and for fast full text search, a SQL database would be an improvement. As long as this is disabled by default, it doesn't matter (but that's only my opinion and I'm not a osdteletext developer, but just someone who is interested in the plugin). I just know that my and probably many other VDRs out there are too slow to be able to run that big mysql daemon, so it would be definetly bad if the built-in file-based cache gets replaced by mysql in future.

I can understand your XXV usecase, but personally I wouldn't be happy with MySQL code in the OSDTeletext sources.

What about using a separate plugin and just refactoring the code, so that the common parts in both plugins can easily be shared? Xeatre.TV also has a "teletextgrabber" plugin (based on osdteletext - looks very much like your code, so it seems Andreas P. borrowed from you or vice versa).

This would have the benefit, that you can add extra functionality, like externally triggered updates of VTX pages.

Or: Making osdteletext just trigger an event when something changes, which an external application can capture to update the database?

On the other side: Your Patch isn't so much intrusive and injecting the storage is a good idea anyway (from a software designe perspective...)

etobi wrote:

I can understand your XXV usecase, but personally I wouldn't be happy with MySQL code in the OSDTeletext sources.

The storage is disabled by default, you must it enable with "make all HAVE_MYSQL=1" at build time. MySQL code reflected only on mysqlstorage.c, so that an plain build have'nt any depends at build and runtime to mysql.

It's add only an third methods. Any other storages methods (packed, single file) are unchanged and full usable.

What about using a separate plugin and just refactoring the code, so that the common parts in both plugins can easily be shared? Xeatre.TV also has a "teletextgrabber" plugin (based on osdteletext - looks very much like your code, so it seems Andreas P. borrowed from you or vice versa).

Sorry, i've never see any public code from xeatre.tv, so that i can only provide and share my own development.
Your refered about a plugin that have a different target (recording based historical teletext logging) that's is'nt really not usable for osdteletext-plugin.

After taking a deeper look into this patch, I have a tiny wish. Please split it a little bit more:

• Refactoring: Inject storage rather then refering to it statically
• Refactoring: Use ChannelID() instead of chan
• Refactoring: Use Running() instead of running instance variable
• New: Added TopText option
• New: Add MySQL storage

I also don't like that a static storageOption is introduced. The storageOption / maxStorage is only used for the LegacyStorage. The other storage classes don't need this. My suggestion is: Instantiate the storage classes from cPluginosdtelextc and provid maxStorage for LegacyStorage only. This means getting rid of the static Storage::instance() factory method (and the static system member) and moving the storage instantiation into the cPluginosdtelext or a separate factory class used by cPluginosdtelext. So there should be another refactoring step before adding the mysql storage:

• Refactoring: Moved storage instantiation into main class

BTW: There seems to be a bug in your patch regarding this issue anyway: You left a Storage::instance() call in the cTxtReceiver ctor. This would mean, two storages get instantiated.

Thanks! Much more Git-friendly and easier to understand. I'll try to add your patches ASAP, before making a new release.

anbr wrote:

Here a simple patch to call housekeeping regularly.
Without this patch housekeeping will only once at start up executed.

• Cleanup remove once at startup if filesystem over limit.
• if memory counted pages over limit
• if filesystem really full and write failed

What about using sqlite instead of mysql ? This would make configuration much easier.

gnapheus wrote:

What about using sqlite instead of mysql ? This would make configuration much easier.

At moment osdteletext-source include only changes for a more generic storage interface. There none mysql-depends changes merged at upstream release.

You chould create a own new class based on Storage (txtrecv.h)
e.g. for mysql - http://projects.vdr-developer.org/attachments/141/osdteletext-0.8.3-6th-add-mysql.diff

But some people on web-blogs say :
"SQLite is very fast on read operations and slow on write operations comparing to databases like MySQL." ;-)

anbr wrote:

gnapheus wrote:

What about using sqlite instead of mysql ? This would make configuration much easier.

At moment osdteletext-source include only changes for a more generic storage interface. There none mysql-depends changes merged at upstream release.

You chould create a own new class based on Storage (txtrecv.h)
e.g. for mysql - http://projects.vdr-developer.org/attachments/141/osdteletext-0.8.3-6th-add-mysql.diff

Yes, I know that the mysql backend isn't implemented yet. But I think osdteletext needs i new storage backend with more possibilities and maybe a more lightweight solution like sqlite would rather be implemented.

But some people on web-blogs say :
"SQLite is very fast on read operations and slow on write operations comparing to databases like MySQL." ;-)

Do you really think this is of importance for this application?