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

Isengard: muxer - flush the queued packets with wrong pts on seek #146

Closed
wants to merge 7 commits into from

Conversation

perexg
Copy link
Contributor

@perexg perexg commented Dec 18, 2015

(only for Isengard branch)

@Jalle19
Copy link
Contributor

Jalle19 commented Dec 18, 2015

Can you change the PR to point against master directly from Github, or does it not apply cleanly?

It's really a fatal error. If the predicate is not set to zero,
then _PredicateCallbackDefault() function (see platform/mutex.h)
gives true return value. It means that Wait() returns ASAP.

This also means that m_seekTime must not be set to a zero value
in the ParseSubscriptionSkip().
@perexg perexg changed the title muxer - flush the queued packets with wrong pts on seek Isengard: muxer - flush the queued packets with wrong pts on seek Dec 18, 2015
@Jalle19 Jalle19 added the Fix label Dec 19, 2015
@perexg
Copy link
Contributor Author

perexg commented Dec 19, 2015

The commit 4793806 shows the purpose. Also, Flush() should be called on rewind or FF 4x + .

@Glenn-1990
Copy link
Contributor

Why is this only for Isengard?
Isengard will be obsolete soon as jarvis is almost in RC.

@perexg
Copy link
Contributor Author

perexg commented Dec 20, 2015

Sorry, I work only with precompiled binaries (using kodi 15.2 package in Fedora). The packaging is always delayed a bit. But I'll port my changes to master when I finish all tests.

@perexg perexg mentioned this pull request Dec 30, 2015
@Jalle19
Copy link
Contributor

Jalle19 commented Jan 5, 2016

Are all these changes part of #155?

@perexg
Copy link
Contributor Author

perexg commented Jan 5, 2016

Yes, they're.

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 5, 2016

Okay. @ksooo do we close this or do you want to make a new release for Isengard with applicable backported patches? I believe OE users would get the update since they use a repository for PVR addons.

@ksooo
Copy link
Member

ksooo commented Jan 5, 2016

What is Isengard? ;-) I do not see a need for an Isengard update- If you ask me, I would close this PR.

@opdenkamp
Copy link
Contributor

That you don't see the need doesn't mean that it's not useful for others. Not everyone is using the latest greatest version number :)

@ksooo
Copy link
Member

ksooo commented Jan 5, 2016

That's why I said "if you ask me". ;-) If others don't share my point of view, I'm fine with this. I will not block an Isengard update.

@opdenkamp
Copy link
Contributor

I've merged in PRs for older releases quite often when people sent updates. Only thing that needs to be taken care of is that you bump the add-on version numbers properly if the APIs or ABIs are incompatible, so you don't get clashes.

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 5, 2016

@seo will you accept addon bumps for OE 6 even though Isengard development has stoppd? If OE won't use a newer version no one else will since other platforms get the addons shipped with Kodi itself.

@opdenkamp
Copy link
Contributor

There are a lot more distributions out there that we don't control. There's more out there than just Ubuntu, Windows, etc. One of the big advantages of separating core from stuff like this is that add-ons can still be fixed, even though development of core has moved to a new version.

That being said, I don't care much if you merge this or not, just commenting on the reasons that you're giving for not merging ;)

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 5, 2016

@opdenkamp which platforms would that be? As far as I know, OE is the only one with a repository for binary addons.

@opdenkamp
Copy link
Contributor

e.g. any Linux distribution that's not called Ubuntu?

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 5, 2016

Okay. Let's wait a bit longer before we start backporting stuff in case any fixes show up at the last minute.

@CvH
Copy link

CvH commented Jan 6, 2016

@Jalle19
latest/rescent "official" Isengard version is shipped at OE, if there is an update here it would be updated at OE. At least this is the plan ;)

@CvH
Copy link

CvH commented Jan 17, 2016

@Jalle19
any news on this for Isengard ?

@Jalle19
Copy link
Contributor

Jalle19 commented Jan 28, 2016

Closing this since master has the fixes applied. I hope to be able to do some backports during the weekend.

@Jalle19 Jalle19 closed this Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants