catalinii / minisatip

minisatip is an SATIP server for linux using local DVB-S2, DVB-C, DVB-T or ATSC cards
https://minisatip.org
327 stars 82 forks source link

Change PCMCIA speed to 96 Mbps whenever a CI+ CAM is inserted #936

Closed Jalle19 closed 2 years ago

Jalle19 commented 2 years ago

I stumbled upon a CAM that always ended up taking the "ai_version == 1" path, meaning the CAM data rate would never get bumped.

The spec says:

CICAMs shall support 96 Mbit/s. Hosts shall support 72 Mbit/s, support for 96 Mbit/s is optional.

...so it should be safe to always use 96 Mbit/s for CI+ CAMs.

There's another tiny change that I couldn't easily split to a separate commit and that is to log that the "date time" resource is connected.

Yuri666 commented 2 years ago

you false. check log

[08/02 23:09:37.626 CA0]: setting CI+ CAM data rate to 96 Mbps

Jalle19 commented 2 years ago

I'm not getting that line in my logs without this change.

Yuri666 commented 2 years ago

so, your CAM not support 96 mbps. I have such CAM. It ci+ 1.1

Jalle19 commented 2 years ago

Well, it still works even when I force it to 96 Mbps 🤷 And the spec says it's mandatory for any CI+ CAM (unless that's a more recent addition).

Yuri666 commented 2 years ago

Ok, probally you right. date_rate APDU just inform CICAM about supported data rate by HOST. HOST always support 96 mbps, so we shall always to send 96 mbps APDU to ci+ CAM.

Jalle19 commented 2 years ago

Yeah that's my thinking as well. I don't know what circumstances could cause the host to not support 96 Mbps? Maybe some really old laptop with a native PCMCIA slot? I think it's safe to assume users use DD, TBS or Enigma STBs, so full speed should be taken for granted.

@Yuri666 can you try this with your 1.1 CAM?

Yuri666 commented 2 years ago

@Yuri666 can you try this with your 1.1 CAM?

No difference behavior between 72 and 96 mbps. Such as expected - this CAM can decode only one channel, so 72 mbps is enough at any way and don't matter what speed support host.

Jalle19 commented 2 years ago

Alright, thanks for testing! Do you think this is okay to merge?

Yuri666 commented 2 years ago

yes, all ok

Yuri666 commented 2 years ago

Ok, I re-read the spec again. Application information resource version less than 3 don't support data_rate_info APDU. It may hang some CAMs. So, this PR not valid, sorry.

Jalle19 commented 2 years ago

Ok, I re-read the spec again. Application information resource version less than 3 don't support data_rate_info APDU. It may hang some CAMs. So, this PR not valid, sorry.

Hmm, that's unfortunate. Should we try to find a CAM that actually hangs before reverting?

Yuri666 commented 2 years ago

I hear about it in tvheadend forum, but didn't seen in real. I don't think it's very important, and not needs to reverting before anybody claim.

Jalle19 commented 2 years ago

Alright. It's strange that AI version 3 seems to be mandatory for CI+ 1.4 compatible hosts to announce, yet my CI+ 1.4 CAM chooses to use AI version 1 🤷 I originally came up with this change because I thought it was a bug, but it seems the original code was correct, even though some comments were technically wrong.

I'll whip up a revert with some clarifications in the near future, don't want to deal with potential support issues 😄