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

imx6: update kodi patches, second pass #1119

Merged
merged 1 commit into from
Jan 2, 2017
Merged

Conversation

vpeter4
Copy link
Contributor

@vpeter4 vpeter4 commented Jan 2, 2017

No description provided.

@vpeter4 vpeter4 added the UPDATE label Jan 2, 2017
@CvH CvH merged commit 12565b0 into LibreELEC:master Jan 2, 2017
@vpeter4 vpeter4 deleted the imx6-rc2.1 branch January 2, 2017 15:34
@mk01
Copy link

mk01 commented Jan 8, 2017

@vpeter4
patch to rendererimx breaks all deint methods but BOB.
(this is proper refactor - mk01/xbmc@e1cac5f)

@vpeter4
Copy link
Contributor Author

vpeter4 commented Jan 8, 2017

Actually I already have applied all patches from @warped-rudi like https://github.com/OpenBricks/openbricks/blob/0e0d33112608f421f5bd8e60849a881f6a063d1e/packages/multimedia/kodi/patches/0042-IMXRender-Refactor-to-improve-readability-and-perfor.patch. If I see correctly.

Why doesn't such kode come to kodi upstream? Seems to me just a big mess all over.

Update: Now I see that it is not the same code. So would would be the perfect one?

@mk01
Copy link

mk01 commented Jan 8, 2017

@vpeter4

I'm commenting indeed the fact that you applied and broke deinterlacing.

@vpeter4
Copy link
Contributor Author

vpeter4 commented Jan 8, 2017

Obviously I don't understand anything of this but from what I see it is broken in kodi krypton upstream anyway. So I didn't broke anything - the code was/is already broken.

@fritsch
Copy link
Contributor

fritsch commented Jan 8, 2017

The big question is: Why does a distribution like LibreELEC that wants to ship official kodi pick multiple external patches from elsewhere != upstream, without even knowing if they fix something or if the do something good?

@mk01 is the official imx maintainer of kodi and it's sad that this single dev now needs to go towards downstream and fix the pickery issues.

@mk01
Copy link

mk01 commented Jan 8, 2017

@vpeter4

  • first two screens are from advanced(half)/advanced resp. - as they should look correctly (upstream or the simplified code in commit in 1st comment)

  • then two screenshots following are advanced(half) and adv(half) zoomed. the last one is advanced - patched.

(video is burosch1.mpg (720x576x25) played at 1920x1080x50 from @fritsch 's pool of test files)

upstream - advanced(half) ((yadif 1x))

screenshot021

upstream - advanced ((yadif 2x))

screenshot022

patched - advanced(half)

screenshot025

patched - advanced(half)

screenshot026

patched - advanced

screenshot023

@fritsch
Copy link
Contributor

fritsch commented Jan 8, 2017

@mk01 you don't really use yadif right? Just reusing the name :-) (Yeah: https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoPlayer/DVDCodecs/Video/DVDVideoCodecIMX.cpp#L638)

@fritsch
Copy link
Contributor

fritsch commented Jan 8, 2017

@mk01 while having you here. I asked @vpeter4 for the reason to pick the stuff from openbricks. It seems there are some bugs left, he mentioned "seeking" would cause issues. Is that okay to file these bugs on trac.kodi.tv or where do you want them?

I want to make sure that no distro has to pick valid fixes from elsewhere, but making sure upstream works as it should - features are something else, obviously.

@vpeter4 could you name the issues you have with "standard v17"? - then we can track them and fix them.

@vpeter4
Copy link
Contributor Author

vpeter4 commented Jan 8, 2017

@warped-rudi
Copy link

@fritsch: Upstream had issues and did not even compile cleanly from august to december...

Besides the problem @vpeter4 mentioned, I see here hangs on exit (usually the video plane gets switched on again so that the last frame of the previously played video is shown while Kodi waits infinitely for CIMXContext::Process() to exit).

Most of the other patches are attempts to make the code better understandable for someone who has not written it, remove unneeded stuff and tweak performance. They are not really new as I sent a link to the repo that contains them to @mk01 by the end of september. But I did not get feedback on that. I quickly adapted them to the december commit and probably broke something. S**t happens. The current source can be found here https://github.com/warped-rudi/xbmc/commits/Krypton-iMX6 . Maybe someone can comment.

@fritsch
Copy link
Contributor

fritsch commented Jan 8, 2017

@warped-rudi:

Upstream had issues and did not even compile cleanly from august to december...

Could you perhaps think of the fact, that upstream - with only one imx dev left - would have been happy to get that compile fix from you?

@warped-rudi
Copy link

@fritsch:
Well, I thought it would be sufficient to report the issue to the maintainer. BTW, where can I find your pool of test files?

@fritsch
Copy link
Contributor

fritsch commented Jan 8, 2017 via email

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.

5 participants