FernetMenta / vdr-plugin-vnsiserver

VDR plugin to handle XBMC clients.
GNU General Public License v2.0
16 stars 33 forks source link

Cam improvements #74

Closed FernetMenta closed 7 years ago

FernetMenta commented 7 years ago

@glenvt18 mind a review?

glenvt18 commented 7 years ago

@FernetMenta

m_Receiver->SetPriority(MINPRIORITY);
if (!m_Device->AttachReceiver(m_Receiver))
  return false;
if (m_camSlot)
  m_camSlot->StartDecrypting();
m_Receiver->SetPriority(m_Priority);

What a hack! :-)

I suggest adding a comment like the one below not to forget and, accidentally, remove that code.

// HACK cDevice::AttachReceiver() doesn't start scrambling timer if priority == MINPRIORITY

And for ClrChecked()

// HACK Undo ChannelCamRelations.SetChecked() - see cDevice::Action().
// Note: m_Device->CamSlot() returns NULL after SetChecked() is called. Use m_camSlot here.

Everything works fine. Either option helps (and both too). I've tested with 5 second CAM delay. The error is "16, no data". When both options are not set, zapping is not possible at all.

After you have merged this, I'll submit a PR with re-tune fix.

Thanks a lot!

EDIT. Add po/de_DE.po and po/lt_LT.po files to the blacklist commit (they have been generated by the build), and this cosmetic change:

diff --git a/videoinput.c b/videoinput.c
index d11dbe4..c4322d6 100644
--- a/videoinput.c
+++ b/videoinput.c
@@ -487,7 +487,7 @@ cVideoInput::cVideoInput(cCondWait &event)
   m_Device = NULL;
   m_camSlot = nullptr;
   m_PatFilter = NULL;
-  m_Receiver = NULL;;
+  m_Receiver = NULL;
   m_Channel = NULL;
FernetMenta commented 7 years ago

What a hack! :-)

hehe. Could have been easier if VDR didn't make this member private

I suggest adding a comment like

good idea. will do tomorrow, if still an open topic :)

After you have merged this, I'll submit a PR with re-tune fix.

ok, you have control.

glenvt18 commented 7 years ago

I've found an issue with DisableScrambleTimeout. I tested it with 5 sec CAM delay. A failed recording or streamdev session (they don't use that MINPRIORITY hack) leaves startScrambleDetection set to the last time(NULL). There is no way to reset it to zero again. As a result, VNSI is unable to zap until VDR is restarted (unless you are very very lucky). In other words, VDR remains a useless piece of s**t until restart. Re-tunes with DisableCamBlacklist work, but recordings and streamdev don't use it.

To summarize, if a TS stream can't be descrambled in TS_SCRAMBLING_TIMEOUT seconds, which is hard-coded as 3 seconds, we have the following issues:

  1. Recordings don't work. Tested.
  2. Streaming with streamdev doesn't work. Tested.
  3. VNSI can stream after a series of re-tunes using a dirty hack.

I think those are quite strong arguments for submitting a patch to VDR. Here are some options:

  1. virtual int cCamSlot::ScramblingTimeout() which can return 0.
  2. VDR option for the scrambling timeout.
  3. Both 1 and 2.
  4. Increase TS_SCRAMBLING_TIMEOUT to 10 seconds. Almost the same as 5.
  5. Remove this code completely.

What do you think of it?

FernetMenta commented 7 years ago

I think a VDR option that defaults to disable it would be best because it would also work for recordings. btw. I have a test system here with a real cam that also suffers from this silly scramble timeout. IMO this code was added a workaround for some failing cam.

glenvt18 commented 7 years ago

OK. I'll write to the VDR mailing list. Should we keep the MINPRIORITY hack?

FernetMenta commented 7 years ago

Should we keep the MINPRIORITY hack?

Currently it is the only way to i.e. activate an expired subscription or VOD.

glenvt18 commented 7 years ago

But to record you have to stream the channel with VNSI first. The only way to recover is a successfully started recording or streamdev stream or restart. So, it's dangerous.

FernetMenta commented 7 years ago

There is 2 different settings now: "DisableScrambleTimeout" and "DisableCamBlacklist". Users can select the most appropriate for their purposes. I think for the majority "DisableScrampleTimeout" won't harm because they either watch tv or the system wakes up for a recording, records a show, and shuts down again.

But to record you have to stream the channel with VNSI first.

I don't follow here. Why?

glenvt18 commented 7 years ago

Because recording can't use the hack. What is unexpected is that when both options are set (have just tested) re-tunes don't occur. That's because when cLiveReceiver is detached m_PmtChange is 1 (see this).

BTW. Keep an eye. https://linuxtv.org/pipermail/vdr/2016-September/029153.html

FernetMenta commented 7 years ago

Note that cLiveStreamer::RetuneChannel has to force a retune. This is called by VDR when pids changes or some cam related parameters changed. This is important info for the cam. If we don't detach/attach, the cam won't get started.

I think we should drop m_PmtChange completely as you suggested.

glenvt18 commented 7 years ago

m_PmtChange == 1 means that no TS packet has arrived yet - Receive() hasn't been called. I don't see why it is wrong to re-tune in this case. My guess was that m_PmtChange was left from the old code which used to detach/attach cLiveReceiver to change pids. If this is the case, we are free to remove it.

FernetMenta commented 7 years ago

Yes, pmtChange is a left-over form the days videoInput did not close/reopen and detected changes of pat/pmt itself.

glenvt18 commented 7 years ago

And I suspect Pat/Pmt filter (cLivePatFilter) won't work. m_PmtChannel = *m_Channel in Open(). When m_PmtChannel is modified re-tune is requested, but m_PmtChannel is reset every time - it doesn't store the new pids. I'll test it.

FernetMenta commented 7 years ago

@glenvt18 I just had a look on m_PmtChange. Should be ok like it is now.

m_PmtChannel is reset every time - it doesn't store the new pids

it gets the new pids from the channel on open

glenvt18 commented 7 years ago

Yes, I see. VDR 2.1.4 added a notification with cStatus::ChannelChange(), and cLivePatFilter is used instead for VDR < 2.1.4. That should work.

FernetMenta commented 7 years ago

@glenvt18 does vdr 2.3.3 consider your request?

glenvt18 commented 7 years ago

If there is only one (master) cam slot in the system, yes, there is no timeout anymore. VNSI reports 'no data (16)' error and starts streaming immediately as soon as the stream gets decrypted. The only drawback is that if a recording doesn't receive decrypted packets within 30 seconds, an emergency exit is fired and VDR restarts. If, at that time, one has active FTA/IPTV/etc recordings/streaming, they will be broken. So, scrambling timeout VDR option could (still) be helpful :( For systems with only one (master) cam DisableScrambleTimeout/DisableCamBlacklist options are not needed anymore. But I would keep them for older (< 2.3.3) VDRs as it looks like a lot of people are still using VDR-2.2.0.