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

[GLES] Check for UNPACK_ROW_LENGTH support on renderer instantiation #15684

Merged
merged 1 commit into from Apr 6, 2019

Conversation

Projects
None yet
8 participants
@kszaq
Copy link
Contributor

commented Mar 6, 2019

Description

Check for UNPACK_ROW_LENGTH support on renderer instantiation not on every LoadPlane call. Additionally check for GL_EXT_unpack_subimage extension even if headers define HAS_GLES == 3, e.g. Android or Mali-450 running mainline Linux + Lima.

How Has This Been Tested?

LibreELEC running on Amlogic S905 and S912.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@kszaq kszaq changed the title [GLES] Check for UNPACK_ROW_LENGTH support on renderer instantiation,… [GLES] Check for UNPACK_ROW_LENGTH support on renderer instantiation Mar 6, 2019

@kszaq

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

@peak3d Would you mind having a look?

@peak3d

peak3d approved these changes Mar 6, 2019

Copy link
Contributor

left a comment

Looks reasonable for me, thx1

@kszaq

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2019

Sorry to nag - is there anything else required for this PR to be merged?

@fritsch fritsch merged commit 3e75320 into xbmc:master Apr 6, 2019

1 check passed

default You're awesome. Have a cookie
Details

@Rechi Rechi added this to the Leia 18.2-rc1 milestone Apr 6, 2019

@kszaq

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2019

Thanks!

@kszaq kszaq deleted the kszaq:gles_simplify branch Apr 6, 2019

@rainman74

This comment has been minimized.

Copy link

commented Apr 8, 2019

With this change, software decoding is felt slower than before (Sony Bravia TV)...

@a1rwulf

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

With this change, software decoding is felt slower than before (Sony Bravia TV)...

How can decoding "feel" slower?
I think it would be good if you open an issue and properly describe your problem.

@rainman74

This comment has been minimized.

Copy link

commented Apr 8, 2019

My test video ran smoothly with software decoding and now with the last nightly build it jerks slightly.

Test video

@lrusak

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

@rainman74 did you actually install a build that had this change included? It's not available in an release builds right now.

@rainman74

This comment has been minimized.

Copy link

commented Apr 8, 2019

the last nightly build

@fritsch

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

As this is SW rendered, we can easily "see":

Please install the nightly before this version, enable debug logging via the GUI. Now play your video and make a recording from the debug display.

And a second one with the nightly at hand, each for one minute pleasse without touching keyboard / control.

@peak3d

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

I see one thing in this PR: The original version saves the actual value for PixelStoreI and after the image operation restores it. This PR has changed this and always set it to 0 (what is wrong IMO)

@peak3d peak3d referenced this pull request Apr 8, 2019

Merged

[LinuxRendererGLES] Fix pixelstore usage #15880

1 of 7 tasks complete
@peak3d

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

@rainman74 pls. try the build which is autogenerated here: #15880

@kszaq

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

@peak3d Setting GL_UNPACK_ROW_LENGTH to 0 follows code from LinuxRendererGL:

glPixelStorei(GL_UNPACK_ROW_LENGTH, stride / bps);
glBindTexture(m_textureTarget, plane.id);
glTexSubImage2D(m_textureTarget, 0, 0, 0, width, height, type, datatype, data);
/* check if we need to load any border pixels */
if (height < plane.texheight)
glTexSubImage2D( m_textureTarget, 0
, 0, height, width, 1
, type, datatype
, (unsigned char*)data + stride * (height-1));
if (width < plane.texwidth)
glTexSubImage2D( m_textureTarget, 0
, width, 0, 1, height
, type, datatype
, (unsigned char*)data + bps * (width-1));
glPixelStorei(GL_UNPACK_ROW_LENGTH, 0);

@peak3d

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2019

Yes, I remember when I forgot to reset it to 0 the GUI overlays were destroyed. Question is what happens if someone uses this in the future for GUI (for reasons we don't know now). This was the reason I saved / restored the value, Does not cost much but could save lot of issue tracking time in the future. GL implementation is not OK for me, too, because it is risky.

But yes, this will not be the reason for the slowness, it will be most likely that he has movies with matching stride where we haven't called glPixelStoreI before, but after thie PR we do.

@fritsch

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

Please check his logs / video of Debug Overlay first. There are other bugs open with his linked video file ... I want to have proof first, before searching a bogger that does not exist in a way we assume.

@rainman74

This comment has been minimized.

Copy link

commented Apr 9, 2019

@rainman74 pls. try the build which is autogenerated here: #15880

can't find that apk? Do you mean the next nightly build? I don't have my own compilation environment for Android Kodi :(

@rainman74

This comment has been minimized.

Copy link

commented Apr 9, 2019

As this is SW rendered, we can easily "see":

Please install the nightly before this version, enable debug logging via the GUI. Now play your video and make a recording from the debug display.

And a second one with the nightly at hand, each for one minute pleasse without touching keyboard / control.

I just made a strange discovery during the recording:

With debug mode and overlay active -> the video runs super smooth:
https://mega.nz/#!TyB0lShQ!LOK071z3AbgVuxkyvx4epYz3tlB2eCFMKmIPplsfUm4

Normal playback without the debug mode -> the video jerks slightly as already described:
https://mega.nz/#!vrRSBAaC!A-tMevOHFTyPFdbi66osxogHqmg-6HAc6kBIG7N4dMw

What could be the reason for that?

@peak3d

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

@rainman74 you'll find the atifacts here:

image

click on Show all checks / then Jenkins details and select the OS you need (Android Arm??)

@rainman74

This comment has been minimized.

Copy link

commented Apr 9, 2019

kodi-20190408-1e3031ec-PR15880-merge-armeabi-v7a.apk
... tested and the jerking is still there (and gone in debug mode)

@rainman74

This comment has been minimized.

Copy link

commented Apr 24, 2019

Status?

@rainman74

This comment has been minimized.

Copy link

commented May 20, 2019

To all Kodi Devs: I don't understand how to start with a new Kodi milestone (v19) and still have so many bugs left in v18? When I look at all the playback problems, but also all the hundreds of open issues in the tracker!

I don't think that's a good strategy to leave all the users out in the rain and let them wait for a new stable version in maybe 2 years.

I'm very disappointed by Kodi, which got better and better over the years. Unfortunately this is no longer the case with v18 and now this chapter is simply ticked off.

@kszaq

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

@rainman74 Kodi 18 is still open for PRs with bug fixes for a next dot release. If someone has a solution to your bug, you will see a PR for sure.

@rainman74

This comment has been minimized.

Copy link

commented May 20, 2019

@kszaq thanks for your feedback, I know that of course.

But it's already being heavily developed on v19 and v18 gets only very rudimentary support, although there are still a lot of bugs included, all of which will reappear on v19.

-> 322 issues still open!

So I would have tried to get the v18 as stable as possible first and then start with the v19 later. What use is v19 to anyone who has a problem right now?

So please stay with v18 for now and make it as stable as v16 and v17 were.

@ksooo

This comment has been minimized.

Copy link
Member

commented May 20, 2019

v18 gets only very rudimentary support

Support? Are we a company selling a product? No.

Cannot repeat this often enough. We are a (small) group of volunteers, doing 100% of this project in our free time. So, we do what we want to do, not, what "customers" want us to do.

If you like what we do / have done you are welcome to use what we "offer" to the world for free. If it doesn't fit your needs, you can look for alternatives.

@rainman74

This comment has been minimized.

Copy link

commented May 20, 2019

@ksooo That's all clear, but doesn't change the fact that you're starting to develop a new version and the "old" one isn't even finished yet ;-)

Don't get me wrong, I love Kodi for years and you do a great job in your spare time! That's undisputed. There are also very few alternatives that are at your level.

But v17 was just much more stable and error-free than the current v18, so I think it's the wrong way to ignore that and start other new features. For me, you lost track of your fanbase.

I have already started to go back to v17 and it's not just me. Many of my friends still don't think the v18 is mature. Is that your claim that your fans have to use an older version again?

If you think I'm just an isolated case, why don't you make a simple poll in your forum about how the quality of v18 is rated? If you're not interested in all this, well, I'd just like to give you a thought essay with my post. Nothing more.

@a1rwulf

This comment has been minimized.

Copy link
Member

commented May 20, 2019

There is no such thing like finished software.
We've addressed issues that we think are blockers or affected lots of people and still continue to do so for v18.
As any other software, we also have to move on and work on the next version, as we can't hold back contributions, just because you want us to work on v18 for another 6 months.
The number of open issues, doesn't say anything btw - you included every single issue, confirmed or not. And if you look at other big projects - they have thousands of open issues and still work on new releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.