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

LinuxRenderer fixes #1408

Merged
merged 2 commits into from Oct 10, 2012
Merged

LinuxRenderer fixes #1408

merged 2 commits into from Oct 10, 2012

Conversation

FernetMenta
Copy link
Contributor

This fixes/improves de-interlacing method weave when upscaling. Both fields are rendered into a fbo of original size first, then the fbo gets rendered and upscaled.

@elupus
this is follow-up from our discussion here: FernetMenta@975d03b

@FernetMenta
Copy link
Contributor Author

@elupus ping, do you have time to review this?

@elupus
Copy link
Contributor

elupus commented Sep 16, 2012

Could you explain a bit more in detail what it improves?

@elupus
Copy link
Contributor

elupus commented Sep 16, 2012

The first commit we can be merged as soon as merge window open. It could be improved I suppose by create a struct for holding the fbo properties, and having RenderFromFbo taking that as a parameter. But it is better than current code anyway.

@FernetMenta
Copy link
Contributor Author

There's two things it improves when doing a weave:

Quality:
It renders both fields to the fbo at the same size as source. Then it renders the fbo (holding both fields) and does upscaling.

Performance:
It first collects both fields into the fbo, then renders the fbo only once.

@elupus
Copy link
Contributor

elupus commented Sep 16, 2012

How does that work. You can't scale a weaved image.. Each field need to be scaled separate.

@FernetMenta
Copy link
Contributor Author

Ok, then we need to distinguish between weaving/scaling of interlaced fields and fields of a progressive frame. The latter needs scaling after the fields have been combined.

@elupus
Copy link
Contributor

elupus commented Sep 16, 2012

Yes, if we get separate fields for a progressive source we need to do that yes. It's only vdpau interrop that does so currently right?

@FernetMenta
Copy link
Contributor Author

Yes. Then I keep the first and 3rd commit in this PR and move the second (progressive weave) to #870?

@bobo1on1
Copy link
Member

For VDPAU interop I would write a shader that directly weaves the fields, most of the slower gpu's can't handle going through an fbo first at 1080p.

@FernetMenta
Copy link
Contributor Author

@bobo1on1
It won't go through the fbo it no upscaling is needed. Nevertheless I have written a prototype for this kind of shader (see link above), but then dropped this idea. If you think using a shader is better I have to put the pieces together.

@bobo1on1
Copy link
Member

Ah right, in that case it would be faster to render to an fbo first.

@ghost ghost assigned elupus Sep 24, 2012
@FernetMenta
Copy link
Contributor Author

@elupus
I dropped the change on weave and created a struct for the fbo related parameters.

@ghost
Copy link

ghost commented Oct 7, 2012

ping

@fritsch
Copy link
Member

fritsch commented Oct 10, 2012

Can we please include at least the second commit: "proper cleanup"?

Yesterday there was somebody on IRC, that saw the scene from the played video before in his "black screensaver" - cause of the Renderer is not cleaned up properly ...

Afaik today (10th) the merge window is ending, right?

@elupus
Copy link
Contributor

elupus commented Oct 10, 2012

The total diff looks okay, so it's fine to be merged.

FernetMenta added a commit that referenced this pull request Oct 10, 2012
@FernetMenta FernetMenta merged commit 77a49e8 into xbmc:master Oct 10, 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