Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ability to disable subscription when mapping #106

Closed
wants to merge 1 commit into from
Closed

add ability to disable subscription when mapping #106

wants to merge 1 commit into from

Conversation

linniksa
Copy link
Contributor

No description provided.

@john-tornblom
Copy link
Contributor

I've been trying to figure out the reason for disabling the subscription when mapping. Could you perhaps explain a bit what your problem is and how this pull request fixes your problem?

@linniksa
Copy link
Contributor Author

when using cam module with limited number of connections/request per time, many channels is not mapped and mapping takes a long time

@linniksa
Copy link
Contributor Author

linniksa commented Aug 3, 2012

@john-tornblom
what you think about merge this pull request

@john-tornblom
Copy link
Contributor

I'm not sure yet. I have to think about it and run it by the other guys and see what they think. I'll get back to you.

@ghost ghost assigned john-tornblom Aug 17, 2012
@linuxstb
Copy link
Contributor

I've only had a quick look at the patch itself, but I would like this to be merged. My use case is UK DVB-T where there are a number of part-time channels. tvh currently only maps channels on-air at the time of mapping, but with this option would be able to map them all, regardless of whether they are currently broadcasting.

bow and I discussed this in IRC just now, and would suggest renaming the option from "Check subscription when mapping" to "Include currently unavailable services when mapping". An end-user may not know tvh's meaning of "subscription", and the option doesn't say what happens with the result of that check.

@adamsutton
Copy link
Contributor

@linuxstb I think as you discussed with bow, there needs to be a bit more to it that this.

I like the idea it will map all the services regardless of whether they're currently available, but I wouldn't want it to map non FTA channels for example.

This might also relate: https://www.lonelycoder.com/redmine/issues/548

@linniksa
Copy link
Contributor Author

@linuxstb
if this is named "Include currently unavailable services when mapping", does this mean it would be a try to real switch to that channel when mapping?

@linuxstb
Copy link
Contributor

@Partugal

I'm not suggesting any changes to the behaviour of your patch, simply changing the text in the web UI. So no, it wouldn't test the availability when mapping, as both available and unavailable services would be mapped.

Do you have a suggestion for clearer text? (or do you prefer your original?)

@adamsutton

I agree there is a lot more that can be added around this feature, but I think this would be a good start, and shouldn't interfere with any future work - e.g. a FTA filter.

However, the problem with a FTA filter is that services can lie about their scrambling status in the PMT, and also some channels may be partly FTA... Also, perhaps the FTA filter should be applied earlier - when finding services in the multiplex, rather than when mapping to channels? So that would be implemented in a different place to this feature.

@adamsutton
Copy link
Contributor

@Partugal

If you can update the comment as suggested by @linuxstb and cleanup the code so it will merge against master (ideally it'll be rebase'd). Then I'll see about getting it merged.

@linniksa
Copy link
Contributor Author

@adamsutton
i'm update pull request

imho "Check services when mapping" or "Switch to service when mapping" is more clear names

@adamsutton
Copy link
Contributor

I'm going to merge this, but actually I think @Partugal is right. I still think the text could be better, but I'll give it some thought when I merge.

@adamsutton
Copy link
Contributor

This has now been merged.

I've fixed a couple of compilation errors and renamed the variables to fit in better. Also made a final change on the text.

@adamsutton adamsutton closed this Aug 31, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants