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

VAAPI / VDPAU: Add deinterlacing methods to processInfo #404

Closed
wants to merge 10 commits into from

Conversation

fritsch
Copy link

@fritsch fritsch commented Aug 25, 2016

See topic.

VDPAU is a mess :-) concerning its capabilities.

@fritsch
Copy link
Author

fritsch commented Aug 25, 2016

The VDPAU commit is not tested (no hardware).

@fritsch fritsch changed the title VAAPI: Add deinterlacing methods to processInfo VAAPI / VDPAU: Add deinterlacing methods to processInfo Aug 25, 2016
@FernetMenta
Copy link
Owner

vdpau is not a mess at all. it served as a template for vaapi

@fritsch
Copy link
Author

fritsch commented Aug 25, 2016

Yeah - it depends on how or who looks at it. VAAPI is much more clean with the PreInit methods that are properly used for hw enumeration and stuff. Never the less I found a way in vdpau to also do it at the hopefully right position.

@fritsch
Copy link
Author

fritsch commented Aug 26, 2016

VAAPI:
v2: added AUTO_MODE <- what's the plan here? Users need a default
VDPAU:
v2: added AUTO_MODE and VDPAU_BOB

@@ -1875,6 +1875,12 @@ bool COutput::Init()
m_pp->PreInit(m_config, &m_diMethods);
delete m_pp;

std::list<EINTERLACEMETHOD> deintMethods;
deintMethods.assign(m_diMethods.diMethods, m_diMethods.diMethods + m_diMethods.numDiMethods);
deintMethods.push_back(VS_INTERLACEMETHOD_AUTO);
Copy link
Owner

Choose a reason for hiding this comment

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

auto is no deinterling mode and is not automatic either. hence crap.
should be handled similar to sw decoder. if mode is not supportd by vaapi, select default.
make sure to add mode none.

Copy link
Author

Choose a reason for hiding this comment

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

None is in, will remove auto.

@fritsch fritsch force-pushed the deint branch 2 times, most recently from 819977e to 3c2a741 Compare August 26, 2016 17:21
@MilhouseVH
Copy link

Looks good on Intel, all deinterlace modes avilable in Video Settings and appearing in Player Process Info dialog when selected.

However, nvidia (VDPAU) doesn't have any deinterlace option available in Video Settings:

s1

@MilhouseVH
Copy link

Latest version of this PR seems to be working great now @fritsch. The deinterlace options are available in Video Settings for both VAAPI and VDPAU, and all the correct deinterlace descriptions are appearing in Player Process Info. Thanks!

@@ -2450,6 +2486,9 @@ void CMixer::InitCycle()

m_processPicture.DVDPic = m_mixerInput[1].DVDPic;
m_processPicture.videoSurface = m_mixerInput[1].videoSurface;

// update deinterlacing method used in processInfo (none if progressive)
m_config.processInfo->SetVideoDeintMethod(deintStr);
Copy link
Owner

Choose a reason for hiding this comment

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

this should not be updated on every cycle, only when changed

Copy link
Author

Choose a reason for hiding this comment

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

You prefer a member tracking it or can I base on m_Deint? e.g. at the beginning of this method before calling SetDeinterlacing if (method != m_Deint) -> update?

Copy link
Owner

Choose a reason for hiding this comment

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

there is a method SetDeinterlacing(). why don't you set it there?

Copy link
Author

Choose a reason for hiding this comment

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

Thx - did so.

Copy link
Owner

Choose a reason for hiding this comment

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

but still set it in InitCycle

Copy link
Author

Choose a reason for hiding this comment

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

It's in Init()

Copy link
Owner

Choose a reason for hiding this comment

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

you still call m_config.processInfo->SetVideoDeintMethod in InitCycle, which is wrong

Copy link
Author

Choose a reason for hiding this comment

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

Ups. Leftover. Something for tonight.

Am 29.08.2016 5:46 nachm. schrieb "Rainer Hochecker" <
notifications@github.com>:

In xbmc/cores/VideoPlayer/DVDCodecs/Video/VDPAU.cpp
#404 (comment):

@@ -2450,6 +2486,9 @@ void CMixer::InitCycle()

m_processPicture.DVDPic = m_mixerInput[1].DVDPic;
m_processPicture.videoSurface = m_mixerInput[1].videoSurface;
+

  • // update deinterlacing method used in processInfo (none if progressive)
  • m_config.processInfo->SetVideoDeintMethod(deintStr);

you still call m_config.processInfo->SetVideoDeintMethod in InitCycle,
which is wrong


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/FernetMenta/xbmc/pull/404/files/9b3f4db0ce2a8efa2228def0dd191ba5c5e80e9e#r76631096,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABCfHVN-318HrsmHP-VPrmA6oRo3Xv3gks5qkv7jgaJpZM4JteUI
.

FernetMenta and others added 4 commits August 29, 2016 10:32
RBP: Add Pi specific deinterlace support reporting
v1: Initial
v2: add Auto mode
v3: Remove auto again - it's "crap"
v1: Initial
v2: Add Auto Mode and VDPAU_BOB
v3: Fallback
v4: Don't spam processInfo
@FernetMenta
Copy link
Owner

I cherry-picked the vdpau commit and squashed further changes into it. thanks.

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