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

Remember the last selected audio and/or subtitles for IPTV streams #3786

Closed

Conversation

DimitarCC
Copy link
Contributor

@DimitarCC DimitarCC commented Oct 4, 2023

This is for remembering the last selected audio or subtitle track for IPTV services.
I know there is a plugin for AutoAudioSelection but it relies on rules only and can not remember the actual user selection.

Where is needed that? Well for IPTV services that are DVB compliant and have multiple audio or subtitle tracks. Now it can not behave like the real DVB service regarding audio and subtitle user selections.

This change fix/add that functionality.
Forum thread here

@Taapat
Copy link
Member

Taapat commented Oct 4, 2023

Why are so many unused imports added here?
Each file contains at least one or more.

Why an unused variable selectionpng added to AudioSelection that points to a non-existent folder in enigma?

Why almost all the code in the avChange method placed in try?
If you are convinced that all this can cause GSOD in any place, maybe you can think about how to make it better?


def isIPTV(service):
path = service and service.getPath()
return path and not path.startswith("/") and service.type in [0x1, 0x1001, 0x138A, 0x1389]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, this iptv detection is wrong and should be moved to c++ by the way.
getPath() have not always the correct value depending on where you get the eServiceReference.

Copy link
Contributor Author

@DimitarCC DimitarCC Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its true. But in the place i use it is correct. But yes if it is in the enigma itself will be better. If you willing to add it why not add the required c++ code ;)


def queueChange(self):
self._waitForEventInfoTimer.stop()
self._waitForEventInfoTimer.start(50, True)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 50?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not? There should be a delay otherwise it will not enable the subtitles correctly.

else:
self.infobar.av_config[ref_str] = pickle_dumps(playinga_idx, 0).decode()

self.saveAVDict(self.infobar.av_config)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very bad if you call this every x seconds.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And how big will this file be if you start 1000 of different iptv streams?

Copy link
Contributor Author

@DimitarCC DimitarCC Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you think that is called every x seconds? And why you think it will ever be called often? You will open and select language or subtitles every second or what?!

setAVInfo is called only on change of the audio track or on pressing OK button. I dont see a reason that to be called often on purpose.

And how big will this file be if you start 1000 of different iptv streams?

Not more than the enigma database that stores track information for normal DVB services. But if you want to test it out make 1000 services and post here how big it is ;)

I use it with 50 services that have subtitles or audio tracks and the size is 3Kb

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry.. I have missread this part.

@DimitarCC
Copy link
Contributor Author

DimitarCC commented Oct 4, 2023

Why are so many unused imports added here? Each file contains at least one or more.

Why an unused variable selectionpng added to AudioSelection that points to a non-existent folder in enigma?

Why almost all the code in the avChange method placed in try? If you are convinced that all this can cause GSOD in any place, maybe you can think about how to make it better?

I think you become too picky and search any reasons that to not be merged...
But i will answer...

Yes there are unused imports since that left from the other functionality i stripped so to separate the code as @littlesat wanted.
In general that imports will not have such a great impact dont you think?

Regarding the try in avChange it is there because the pickle load can fail if someone corrupts the storage file and some of the values inside are not what is expected.
But if you have idea how to overcome this why no share it? It is nice that you point it out but i also can point a lot of issues in OpenPLi code just for the sport ;)

@littlesat
Copy link
Member

Not a good idea to do it this way. This is a work-a-round. Normally this is stored in lamedb. Better to do a real redesign and store it somehow in lamedb. Ensure or made something that it can be done via service ref.

@DimitarCC
Copy link
Contributor Author

DimitarCC commented Oct 4, 2023

@littlesat That means it will never be done because for last 10 year no one wants to mess with lamedb :(
And ofcource it's a workaround because unwilling from nobody to do it in c++.

Sometimes workarounds have to be accepted when there is no light in the tunnel to be done in the right way. ;)

At least with that workaround it will work vs not work at all without it.

@@ -271,12 +274,19 @@ def __init__(self):

self.__event_tracker = ServiceEventTracker(screen=self, eventmap={
iPlayableService.evStart: self.serviceStarted,
iPlayableService.evEnd: self.serviceEnded,
iPlayableService.evUpdatedInfo: self.queueChange,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a problem. How often do you get the evUpdatedInfo event?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why not using evStart?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well evStart can not be used because on that event the info for available audio and subtitle tracks is not available yet.
That happens on evUpdatedInfo for audio tracks and evSubtitleListChanged for subtitles. But since we have to call this twice in this case i use only evUpdatedInfo. Indeed it can be called multiple times but i dont think is more than 2 times.

@Huevos
Copy link
Contributor

Huevos commented Oct 4, 2023

Not a good idea to do it this way. This is a work-a-round. Normally this is stored in lamedb. Better to do a real redesign and store it somehow in lamedb. Ensure or made something that it can be done via service ref.

What do you mean. URLs are not stored in lamedb. Lamedb is for tuning information. Streaming is nothing to do with tuning. Please don't think of messing that up.

We have already had the abandoned lamedb5 project and everyone had to write new code for that. Now we have this strange suggestion.

@littlesat
Copy link
Member

And now again I should make it really right like all the stuff in the past to prove it.... sorry I pass! My notification is gone due to responses like those from you..

@littlesat
Copy link
Member

I know urls aren't stored there. And lamedb5 is also not really accepted yet. But I miss design and agreement here. Again just a lot of hacking code is created so the whole thing got huger hackier and bigger and worser maintable and introducing bugs and more unexpected behavior.... by lake of first discus need... brainstorm, design and create.... the enigma2 code is dead due to this....

@DimitarCC
Copy link
Contributor Author

In my opinion the enigma2 code becomes dead because there are no developers left to maintain it. If every developer faces so much denial, he will not want to work on this ofcource. Or simply will move to another team. Maybe that is the reason OpenPLi to stuck and not evolve anymore... :(

@littlesat
Copy link
Member

It are about 90 extra lines for skin features and the code is not really lean and clean as far I can see in a quick view. This is huge for just satisfy some features used in skins without any thoughts about the real need and/or can something be done also with the code already available and why/how you need something. Actually the whole stuff around skins is crazy already for years. In the past skins did add their own python code renderers when needed. Also a crazy method. Just to get something that looks a bit nicer for someone... making often things slower.... more difficulty maintainable. Without any design thoughts and clear discussions how something is done as it is just done and hacked by some single person.... and then suddenly it is threwes over the wall, other images are spoiled by it and we should follow.... in the past I changed a lot to prove things could made better but I do not have the time and patience anymore.... so this is not my call

Copy link
Member

@littlesat littlesat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone here exactly described the problem for something in events what is missing so we made code that work a round it instead of resolving it by finding a trigger or something else what is missing or we do not have. Having a delay or so is not done that is work a round something... exactly a mechanisme we should strive not to make prefer.... so first think about how we can create a kind of trigger/event when subs or audio channels becomes known with streams

@DimitarCC
Copy link
Contributor Author

But there is not such a trigger created yet from years. And in this way it will not be created next years as well.
We are going in circles.....
You do not have a developer willing to do any kind of c++ extensions, then you dont want to accepts workarounds (even if they work) and demand the c++ code to be fixed and again no developers that willing to do so... Where you are going? In that way i doubt it ever be fixed.

That is discouraging for every developer as i stated.

@littlesat
Copy link
Member

littlesat commented Oct 4, 2023 via email

@DimitarCC
Copy link
Contributor Author

Not. But have you got other options? Since you have no developers left who will fix it even if you discuss it for months or years.
At the end it will stay just one discussion, and nothing will be implemented because there will be nobody left to do so.

Isnt it better to have something working (ofcource if it not breaks anything) instead to have nothing working and just a long long discussion.

@littlesat
Copy link
Member

I think we should find a better method

@littlesat littlesat closed this Oct 18, 2023
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

5 participants