Bug 365083 - sundtek - missing initialization for dvb device + patch
Summary: sundtek - missing initialization for dvb device + patch
Status: RESOLVED INTENTIONAL
Alias: None
Product: kaffeine
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Mauro Carvalho Chehab
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-04 22:22 UTC by Georg Wolfram
Modified: 2017-03-06 13:25 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Georg Wolfram 2016-07-04 22:22:42 UTC
Hello Mauro,

I have 2 USB sticks. Both are not working if connected to my PC at the same time.

1. PCTV 290e DVB-T2
2. Sundtek MediaTV Pro III

The Sundtek device is special, because its driver is not part of the linux kernel.
Because of this, it is not detected by Solid (no sysfs entry).

The Sundtek guys wrote a patch (pre 2.0) to make their device appear in kaffeine. Mostly class DvbDeviceMonitor  and DvbLinuxDeviceManager::componentAdded (QString node, int adapter, int index).  This is just in case you are wondering why this code exists.

In componentAdded some device members are not initialized:

--- dvbdevice_linux.cpp 2016-07-04 23:51:43.718722419 +0200
+++ dvbdevice_linux.cpp 2016-07-04 23:54:48.161415080 +0200
@@ -1851,6 +1852,10 @@
        if (!device->demuxPath.isEmpty () && !device->dvrPath.isEmpty () &&
                        !device->frontendPath.isEmpty ())
        {
+               device->adapter = adapter;
+               device->index = index;
+               device->dvbv5_parms = NULL;
+
                device->startDevice ("");

Error without patch:
04-07-16 23:21:21.014 [Critical] No such file or directory while opening /dev/dvb/adapter2097184/frontend6094880
or

Patch quality: works for me.
Comment 1 Jonathan Riddell 2016-08-03 12:11:54 UTC
<mchehab> also, the hack to support mrec's devices will soon not work with newer versions of Kaffeine, as I'm planning to rely on some new methods at libdvbv5
Comment 2 Markus Rechberger 2016-08-03 17:41:37 UTC
We have a lot customers out there and also several new devices upcoming, if the plan is to make things incompatible this is not the right way to go.

Mauro is breaking things on purpose, just because he can't do better.

However in order to support DVB a quick way our streamingserver can be used instead of kaffeine:
http://support.sundtek.com/index.php/topic,2099.0.html

We're still working on it and it's an early version. The streamingserver also works on ARM, MIPS, PPC, SH4, etc. and is easy to install.
VLC is usually available everywhere so there should not be any problem with it.
Comment 3 Mauro Carvalho Chehab 2016-08-07 12:54:59 UTC
(In reply to Jonathan Riddell from comment #1)
> <mchehab> also, the hack to support mrec's devices will soon not work with
> newer versions of Kaffeine, as I'm planning to rely on some new methods at
> libdvbv5

My plans for the next Kaffeine version is to move all access to the hardware to libdvbv5. It means that device notification will come from the library, and the library will only work with the devices it knows, as, for local devices, it will rely on udev to get the device name (as they can be changed via udev rules).  This is meant to fix a long-standing issue on Kaffeine that got broken on version 1.0.0, related to dtvdaemon support.  That's why we can't add device-specific hacks at the Kaffeine's code. Once such changes were merged in Kaffeine, it shouldn't be hard for a vendor that doesn't provide open source drivers to develop other ways to integrate with libdvbv5 that won't require touching at the Kaffeine nor at the library code.
Comment 4 Markus Rechberger 2016-08-07 14:48:12 UTC
many distributions support it:
1. yavdr
https://github.com/yavdr/yavdr-hardware-sundtek
https://github.com/yavdr/yavdr-hardware-sundtek/blob/master/utils/scansundtek.c

2. openelec
https://github.com/antiprismca/OpenELEC-Antiprism/blob/master/packages/addons/driver/sundtek-mediatv/source/bin/sundtek-mediatv.start
3. libreelec
https://wiki.libreelec.tv/index.php?title=Add-ons
4. all kind of settopboxes support the tuners via plugins
5. Synology, QNAP, XPenology, etc NAS Systems also support it.

so instead of completely breaking working code fix it or kindly ask if someone can help you to get it done in a way that it won't break anything.
Many projects take care about it already.
Comment 5 Georg Wolfram 2016-08-07 17:59:30 UTC
(In reply to Mauro Carvalho Chehab from comment #3)
> My plans for the next Kaffeine version is to move all access to the hardware
> to libdvbv5. It means that device notification will come from the library,
> and the library will only work with the devices it knows, as, for local
> devices, it will rely on udev to get the device name (as they can be changed
> via udev rules).

I am all in for a technical superior solution, but until this solution exist please consider applying my patch.
Comment 6 Markus Rechberger 2016-08-09 13:42:29 UTC
This behaviour is absolutely rubbish, right now there's a bug and a 3 lines patch available.

Whatever Mauro touches is a disaster in terms of maintainance. The truth is he has nothing to say when it comes to our drivers because we implemented the entire linux TV stack from scratch in userspace and it's out of his control. It's his competition and he's afraid of that.
The compatibility is much higher of that the latest stack works until 2.6.16 while the project he's in charge of needs heavy workarounds and removes several drivers to achieve some kind of backward compatibility.

This is nothing but blocking a bugfix here.
Comment 7 Luigi Toscano 2016-08-09 21:10:11 UTC
A pri(In reply to Markus Rechberger from comment #6)
> This behaviour is absolutely rubbish, right now there's a bug and a 3 lines
> patch available.
> 
> Whatever Mauro touches is a disaster in terms of maintainance. The truth is
> he has nothing to say when it comes to our drivers because we implemented
> the entire linux TV stack from scratch in userspace and it's out of his
> control. It's his competition and he's afraid of that.
> The compatibility is much higher of that the latest stack works until 2.6.16
> while the project he's in charge of needs heavy workarounds and removes
> several drivers to achieve some kind of backward compatibility.

A proprietary closed userspace driver, for people who don't know.
Comment 8 Markus Rechberger 2016-08-09 21:15:02 UTC
That's right and that is why the drivers are in userspace and not in kernelspace.

And that being said most customers prefer the binary form anyway because they just use our script to deploy the drivers on ARM, PPC, SH4, MIPS, X86 (independent of the kernel version)
The package is compatible with all linux versions down to 2.6.16
Comment 9 Luigi Toscano 2016-08-09 21:16:37 UTC
(In reply to Markus Rechberger from comment #8)
> That's right and that is why the drivers are in userspace and not in
> kernelspace.
> 
> And that being said most customers prefer the binary form anyway because
> they just use our script to deploy the drivers on ARM, PPC, SH4, MIPS, X86
> (independent of the kernel version)
> The package is compatible with all linux versions down to 2.6.16

Apart from the issue of supporting the non-standard driver, as reported many times, a Solid backend would do the trick instead of relying on the application.
Comment 10 Nicolás Alvarez 2016-08-09 21:36:08 UTC
I don't even care what weird proprietary hardware would be fixed by this: the existing code looks buggy and the patch seems to fix it.

DvbLinuxDeviceManager::componentAdded(QString node, int adapter, int index) is creating a DvbLinuxDevice with heap garbage in the 'adapter', 'index', and 'dvbv5_parms' members. Unless I'm missing something, they are not initialized anywhere, not even in the constructor. How is that not a bug, and why not merge the patch?

If different future changes break support for this hardware again because the driver is doing things fundamentally wrong, we'll discuss it then, but it's orthogonal.
Comment 11 Mauro Carvalho Chehab 2016-08-10 03:50:29 UTC
The point is that the DVB support on version 2.x is partially done via libdvbv5. The new version of the library detects itself the DVB devices (and will allow to not only use local devices, but also remote ones).
The Kaffeine 2.0.1 implementation to support the closed-source userspace driver is a hack: as it seeks for userspace-created fake nodes under /dev/dvb and expects that the library would work fine with that. This won't be true anymore with newer versions of the library, as the library needs to do different things if the device is local or remote. So, it relies on udev for local devices
In other words, it doesn't use the device's path (/dev/dvb/..), but, instead, the name announced by the driver via sysfs.
It would still be possible to add support to the userspace driver, if someone with the hardware has interest on writing such code, but the approach will be different.
Anyway, my plan is to add support for the new version of the library in a couple of weeks. So, IMHO, it doesn't make sense to apply a patch that we know it will be broken real soon now.
Comment 12 Markus Rechberger 2016-08-10 08:13:35 UTC
Nicolás Alvarez has clearly pointed out that this should be fixed.

The person who wrote the fix is using those devices.

You broke this functionality, do you get that? Fix it!

Closed source or not doesn't matter there are people out there using it and our devices are available for nearly 10 years and we're also working on new hardware.
We do not block any existing hardware either. But you're trying to lock out our hardware.

In the future if you have another approach for detecting the hardware we can certainly talk about another approach.
Comment 13 Mauro Carvalho Chehab 2016-08-10 09:19:24 UTC
(In reply to Markus Rechberger from comment #12)

> In the future if you have another approach for detecting the hardware we can
> certainly talk about another approach.

Last time we mixed code from you on an open source project, it didn't end well, as you tried to relicense a co-authored code to MPL without permission of the other developers, and, after a while you closed the sources:
    https://lwn.net/Articles/306601/

So, I won't be actively working on other approach. You can do whatever you want, within the legal limits of the licenses applied on Kaffeine and libdvbv5, provided that you keep your code out of them.
Comment 14 Markus Rechberger 2016-08-10 09:24:56 UTC
This is from 2008, we now have 2016.

In the meanwhile you have stolen code from other ones (even from doubtful sources), took their credit etc. I won't get into detail too much about that but hey I can also do that!

Fix the bug which you have caused, this is 3 lines of code.
Comment 15 Markus Rechberger 2016-08-10 09:26:19 UTC
And by the way this is not about me you are disabling the support for people out there who are using devices which we manufacture.

You can be sure that if you continue to cause problems I will cause big trouble for you.
Comment 16 Markus Rechberger 2016-08-10 09:35:50 UTC
Ah by the way, we have rewritten everything from scratch, we are not even using those chipsets.

We have pure application level drivers and had to re-write everything, memory mapping has been rewritten to use shared memory. The whole thing works completely different when implementing them on application level.

So you can continue to play with wrong accuses. The relicensing was for BSD drivers and that was another topic.

You're just trying to justify wrong things with even more wrong arguments.
Comment 17 Markus Rechberger 2016-08-10 09:49:28 UTC
https://www.kde.org/code-of-conduct/

Mauro, did you ever read that? You're violating nearly every point there.
I wonder that the KDE Team really accepts Mauro, his patches should be accepted but he should not be allowed to maintain a project like that.
Comment 18 Luigi Toscano 2016-08-10 11:30:29 UTC
(In reply to Markus Rechberger from comment #15)
> And by the way this is not about me you are disabling the support for people
> out there who are using devices which we manufacture.
> 
> You can be sure that if you continue to cause problems I will cause big
> trouble for you.

(In reply to Markus Rechberger from comment #17)
> https://www.kde.org/code-of-conduct/
> 
> Mauro, did you ever read that? You're violating nearly every point there.
> I wonder that the KDE Team really accepts Mauro, his patches should be
> accepted but he should not be allowed to maintain a project like that.

I think this should stop now.
Comment 19 Markus Rechberger 2016-08-10 12:28:33 UTC
Ok I think I'm done with this project at the end.

First of all I have asked in a friendly way (at least 10 times also in IRC), however no one has helped here to fix this. The original patch isn't even from me.
And the fact is Mauro broke this support (thanks!).

Mauro is whatever he is (someone who shouldn't maintain anything, after 10 years he tries to cause more problems again! Just fantastic), he tries to protect his kernel drivers, thus he has to fight other projects - as a result you can see this bugreport which could have been closed after the patch has been submitted.

And yes we do support the Linux DVB standard - however the device detection is different and even sysfs changed over the years.

The only one who see's this in a sane way is Nicolás, the other ones are sticking on my so called "bad" behaviour - well that will not change because I don't want to waste any more time on this. 

No one will have a different behaviour after trying in a friendly way for so many times.

The fact remains that this project accepts obvious bugs (of the new maintainer), and bugreports are ignored. That's just awful.

I won't continue here anymore bye.
Comment 20 Luigi Toscano 2016-08-10 12:37:56 UTC
(In reply to Markus Rechberger from comment #19)
> Ok I think I'm done with this project at the end.
> 
> First of all I have asked in a friendly way (at least 10 times also in IRC),
> however no one has helped here to fix this. The original patch isn't even
> from me.
> And the fact is Mauro broke this support (thanks!).
> 
> Mauro is whatever he is (someone who shouldn't maintain anything, after 10
> years he tries to cause more problems again! Just fantastic), he tries to
> protect his kernel drivers, thus he has to fight other projects - as a
> result you can see this bugreport which could have been closed after the
> patch has been submitted.

Personal attach aside.
> 
> And yes we do support the Linux DVB standard - however the device detection
> is different and even sysfs changed over the years.
> 
> The only one who see's this in a sane way is Nicolás, the other ones are
> sticking on my so called "bad" behaviour - well that will not change because
> I don't want to waste any more time on this. 

There are no "others". You only discussed this on IRC, *I* raised issues on the way this was solved, and I'm not the KDE Community. I also clearly said to bring this to more appropriate places for long discussions, like mailing lists (or the upcoming QtCon).
Comment 21 Markus Rechberger 2016-08-10 12:48:24 UTC
By the way this bugzilla is lacking the option of removing an email account. I will just disable the notification here but 

if possible someone please remove our account completely. I don't want to deal with such a project.

It's worthless to discuss that bugs should be fixed, too ridiculous that it's only about 3 lines of code.
In the closed source world things have to work, in certain parts of the opensource world people prefer to waste time (luckily this only applies to kaffeine).
Comment 22 Valorie Zimmerman 2016-08-10 22:26:14 UTC
(In reply to Markus Rechberger from comment #15)
> And by the way this is not about me you are disabling the support for people
> out there who are using devices which we manufacture.
> 
> You can be sure that if you continue to cause problems I will cause big
> trouble for you.

This threat is completely unacceptable behavior. If you continue to threaten people in the KDE community you will not be able to continue to use our infrastructure. The well-being of our community members is more important than any technical issues.

Please write to the CWG mail list and explain your behavior and why you should be able to continue to use KDE infra. mailto:community-wg@kde.org

Valorie, for the Community Working Group
Comment 23 Markus Rechberger 2016-08-11 08:09:53 UTC
Valorie Zimmerman, delete my account if possible.

This is no community as you are locking out numerous potential users.
This is nothing but a joke.

We drop support for KDE now and near future. I haven't seen any such ridiculous behaviour for years. Everyone was happy about getting some official support from a manufacturer and everyone else. The only affected application is kaffeine anyway.

Our users are usually the first ones who report bugs of certain projects when they come up.
The streamingserver / VLC approach seems to be a better replacement anyway.

Begging that a bug should be fixed

04-07-16 23:21:21.014 [Critical] No such file or directory while opening /dev/dvb/adapter2097184/frontend6094880

Is so unbelievable ridiculous. Especially since it was caused by that lame Maintainer.

I think you guys should re-think what you are actually doing.

again delete my accounts I don't give a shit about this KDE project anymore. And you keep justifying why bugs are better than a fixed solution.
Comment 24 Mauro Carvalho Chehab 2017-03-06 13:18:43 UTC
(In reply to Mauro Carvalho Chehab from comment #3)
> (In reply to Jonathan Riddell from comment #1)
> > <mchehab> also, the hack to support mrec's devices will soon not work with
> > newer versions of Kaffeine, as I'm planning to rely on some new methods at
> > libdvbv5
> 
> My plans for the next Kaffeine version is to move all access to the hardware
> to libdvbv5. It means that device notification will come from the library,
> and the library will only work with the devices it knows, as, for local
> devices, it will rely on udev to get the device name (as they can be changed
> via udev rules).  This is meant to fix a long-standing issue on Kaffeine
> that got broken on version 1.0.0, related to dtvdaemon support.  That's why
> we can't add device-specific hacks at the Kaffeine's code. Once such changes
> were merged in Kaffeine, it shouldn't be hard for a vendor that doesn't
> provide open source drivers to develop other ways to integrate with libdvbv5
> that won't require touching at the Kaffeine nor at the library code.

As people pinged me on IRC about this BZ, let me write a better explanation about why this patch won't be applied.

Kaffeine 0.x had a feature of allowing to watch Digital TV with Kaffeine on a hardware without Digital TV hardware. That was done by accessing a remote Kaffeine instance connected to the hardware.

At Qt4 rework (Kaffeine 1.x), this feature got removed, and a prototype for a feature with a similar function was written, at the dtvdaemon/ directory.

Such feature is important, as it would allow someone to have cheap boards like Raspberry PI connected to DVB boards, allowing multiple computers to use the DVB hardware to watch TV. It would also allow using old PC hardware with PCI digital TV boards to provide digital TV streams to modern PC hardware (that's actually my main usecase: I have a very slow hardware with I use to test Kernel support for old PCI hardware).

I tried to play with Kaffeine's dtvdaemon code for a while, in order to extend to a functional code, on an experimental branch (not pushed upstream):
   https://git.linuxtv.org/mchehab/kaffeine.git/log/?h=dtvdaemon

Using dtvdaemon approach, each function at src/dvb/dvbcam_linux.cpp would need to be mapped into two TCP/IP messages (message, message reply) and a few ones also on a notify message. It would also require a complex change at dvbcam_linux, in order to use this code. For development purposes, a text mode application would also be required, in order to check if the API calls are working:


So, I came with another solution, with is a lot simpler: to implement this feature directly at libdvbv5 library, and have a daemon non-GUI application, running on the machine. As it depends only on libc, it is easier to build it on SoC hardware, like RPi. It also brings another advantage over dtvdaemon. With dtvdaemon, I had to write a non-GUI code to test the TCP/IP protocol during its development. Adding it via libdvbv5, there's no need for that, as the same tools there (dvbv5-zap, dvbv5-scan, dvb-fe-tool) can easily be modified to use a remote hardware. The daemon code is at:

    https://git.linuxtv.org/v4l-utils.git/tree/utils/dvb/dvbv5-daemon.c
Comment 25 Mauro Carvalho Chehab 2017-03-06 13:25:33 UTC
(In reply to Mauro Carvalho Chehab from comment #24)

My comment #24 is incomplete. Hit enter by mistake, before finishing it...

> So, I came with another solution, with is a lot simpler: to implement this
> feature directly at libdvbv5 library, and have a daemon non-GUI application,
> running on the machine. As it depends only on libc, it is easier to build it
> on SoC hardware, like RPi. It also brings another advantage over dtvdaemon.
> With dtvdaemon, I had to write a non-GUI code to test the TCP/IP protocol
> during its development. Adding it via libdvbv5, there's no need for that, as
> the same tools there (dvbv5-zap, dvbv5-scan, dvb-fe-tool) can easily be
> modified to use a remote hardware. The daemon code is at:
> 
>     https://git.linuxtv.org/v4l-utils.git/tree/utils/dvb/dvbv5-daemon.c

That solution require that the board detection and all DVB access to go via the library, as the library is responsible to route it to the local or remote hardware. This is incompatible with the hack added on libdvbv5 to support a proprietary DVB driver on userspace.