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

VideoPlayer: fix and cleanup deinterlacing methods #10339

Merged
merged 12 commits into from
Sep 4, 2016

Conversation

FernetMenta
Copy link
Contributor

I wanted to do this for v18 but it is broken now and it makes no sense to implement another work-around for the old design flaw. Defining de-interlacing methods does not belong to renderer. It is defined by video decoder and platform. This change simplifies this:

  • SW decoding with ffmpeg provides the basic methods: none, yadif, yadif-half
  • platforms can override and add additional like render_bob, blend, etc.
  • hw decoders define their own

@Memphiz could you add platform overrides for osx/ios? OSX can add render bob and bland, ios, render_bob
@afedchin could you please do the required changes for Windows
@fritsch vaapi, vdpau
@popcornmix mmal
@mk01 imx

Android is same as iOS or other platform that use GLES renderer.

@FernetMenta FernetMenta added Type: Fix non-breaking change which fixes an issue v17 Krypton Component: Video labels Aug 25, 2016
@fritsch
Copy link
Member

fritsch commented Aug 25, 2016

There is some thing, that might confuse users: Whenever you play progressive only content, you only see Yadif and Yadif half, as those are the fallback methods we have when hw decoder does not overwrite. Is that intended?

@@ -150,21 +150,19 @@ enum AVPixelFormat CDVDVideoCodecFFmpeg::GetFormat( struct AVCodecContext * avct
const char* pixFmtName = av_get_pix_fmt_name(*fmt);

// if frame threading is enabled hw accel is not allowed
if(ctx->m_decoderState != STATE_HW_SINGLE)
// 2nd condition:

This comment was marked as spam.

This comment was marked as spam.

@Memphiz
Copy link
Member

Memphiz commented Aug 25, 2016

@FernetMenta i am unsure what needs to be overiddn ... Only the new method in processinfo ? (You already took care of the vtb hw decoder as i see)...

@FernetMenta
Copy link
Contributor Author

@Memphiz yes, this method

@@ -39,7 +39,6 @@ class CRendererVTB : public CLinuxRendererGLES

// Feature support
virtual bool Supports(EINTERLACEMETHOD method) override;
virtual EINTERLACEMETHOD AutoInterlaceMethod() override;

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@FernetMenta
Copy link
Contributor Author

I added a method to query the default de-interlacing method. That is used by the settings dialog in order to display something useful if current method is not supported. Codecs are supposed to set their desired default method.

@afedchin is there anything to be done for Windows?

@FernetMenta
Copy link
Contributor Author

I will check Windows tomorrow. Then we can get this in. @afedchin is back after Sept. 10th and can verify.

objections?

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@FernetMenta FernetMenta merged commit d87e384 into xbmc:master Sep 4, 2016
@FernetMenta FernetMenta deleted the deint branch September 4, 2016 13:45
@MilhouseVH
Copy link
Contributor

MilhouseVH commented Sep 4, 2016

No VAAPI hardware acceleration after this change, and all VAAPI deinterlace modes are missing (and those deinterlace modes that are present, don't work - everything is yadif=1:-1:1 in PlayerProcessInfo).

This had been working until a couple of days ago, before the cherry pick from FernetMenta#404.

Maybe this is important...

17:41:31  24.553089 T:140558567728896   ERROR: VAAPI::ConfigVAAPI - failed to init output

Debug log playing 720p: http://sprunge.us/CQNY

http://forum.kodi.tv/showthread.php?tid=269815&pid=2408161#pid2408161

Edit: VDPAU is working as expected (both HW acceleration and deinterlace).

@afedchin
Copy link
Member

@FernetMenta windows changes looks good, but. I didn't understand how it will work with sw decoding and dxva rendering. On some hw dxva deinterlacing are better than ffmpeg.

@FernetMenta
Copy link
Contributor Author

I didn't understand how it will work with sw decoding and dxva rendering

for that purpose you can override processInfo and provide different or additional methods.

@afedchin
Copy link
Member

@FernetMenta I still don't see how to solve this.
Only WinRenderer knows which methods are supported by itself. It has no access to processInfo.

  1. There is no issue with DXVA decoder and any renderer it works as before.
  2. with SW decoder there are three possible renderers: DXVAHD (which support deinterlacing) and PS/SW renderer which does not.

In my mind I see that in case SW decoding and DXVAHD renderer user should be able to select the following deinterlacing methods: None, Auto - which actually is DXVA (as before), DXVA and YADIFF. But I do not see how to access to processInfo from WinRenderer which knows actual rendering method.

@FernetMenta
Copy link
Contributor Author

I will think about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Type: Fix non-breaking change which fixes an issue v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants