Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add support more formats to DXVA/DXVA-HD renderers. #4201

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Member

afedchin commented Feb 13, 2014

As alternative to #4192. This needs to be tested with various formats. I've tested this with YUV420P and YUV420P10 only.

@elupus, @Shine-, @wsoltys, @Voyager1, @a11599, please take a look.

Edit: YUYV422 also works.

Also DXVA and DXVAHD needs to be refactored. It is work for G+1

Member

elupus commented Feb 13, 2014

This seem a bit silly don't it? dvdplayer will perform that exact
conversion if we don't report them as supported in the Supports() call.

On Thu, Feb 13, 2014 at 2:24 PM, Anton Fedchin notifications@github.comwrote:

As alternative to #4192 #4192. This
needs to be tested with various formats. I've tested this with YUV420P and
YUV420P10 only.

@elupus https://github.com/elupus, @Shine- https://github.com/Shine-,
@wsoltys https://github.com/wsoltys, @Voyager1https://github.com/Voyager1,
@a11599 https://github.com/a11599, please take a look.

Also DXVA and DXVAHD needs to be refactored. It is work for G+1

You can merge this Pull Request by running

git pull https://github.com/afedchin/xbmc dxva_formats

Or view, comment on, or merge it at:

#4201
Commit Summary

  • Add support more formats to DXVA/DXVA-HD renderers.

File Changes

  • M xbmc/cores/VideoRenderers/WinRenderer.cpphttps://github.com/xbmc/xbmc/pull/4201/files#diff-0(18)
  • M xbmc/cores/dvdplayer/DVDCodecs/Video/DXVA.cpphttps://github.com/xbmc/xbmc/pull/4201/files#diff-1(171)
  • M xbmc/cores/dvdplayer/DVDCodecs/Video/DXVA.hhttps://github.com/xbmc/xbmc/pull/4201/files#diff-2(18)
  • M xbmc/cores/dvdplayer/DVDCodecs/Video/DXVAHD.cpphttps://github.com/xbmc/xbmc/pull/4201/files#diff-3(87)

Patch Links:


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/4201
.

Member

elupus commented Feb 13, 2014

I even think the NV12 conversion can be dropped as well by just reporting
NV12 as supported in the renderer. (as long as we have a conversion to
ffmpeg pixfmt for that)

On Thu, Feb 13, 2014 at 3:35 PM, Joakim Plate elupus@ecce.se wrote:

This seem a bit silly don't it? dvdplayer will perform that exact
conversion if we don't report them as supported in the Supports() call.

On Thu, Feb 13, 2014 at 2:24 PM, Anton Fedchin notifications@github.comwrote:

As alternative to #4192 #4192. This
needs to be tested with various formats. I've tested this with YUV420P and
YUV420P10 only.

@elupus https://github.com/elupus, @Shine- https://github.com/Shine-,
@wsoltys https://github.com/wsoltys, @Voyager1https://github.com/Voyager1,
@a11599 https://github.com/a11599, please take a look.

Also DXVA and DXVAHD needs to be refactored. It is work for G+1

You can merge this Pull Request by running

git pull https://github.com/afedchin/xbmc dxva_formats

Or view, comment on, or merge it at:

#4201
Commit Summary

  • Add support more formats to DXVA/DXVA-HD renderers.

File Changes

  • M xbmc/cores/VideoRenderers/WinRenderer.cpphttps://github.com/xbmc/xbmc/pull/4201/files#diff-0(18)
  • M xbmc/cores/dvdplayer/DVDCodecs/Video/DXVA.cpphttps://github.com/xbmc/xbmc/pull/4201/files#diff-1(171)
  • M xbmc/cores/dvdplayer/DVDCodecs/Video/DXVA.hhttps://github.com/xbmc/xbmc/pull/4201/files#diff-2(18)
  • M xbmc/cores/dvdplayer/DVDCodecs/Video/DXVAHD.cpphttps://github.com/xbmc/xbmc/pull/4201/files#diff-3(87)

Patch Links:


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/4201
.

Member

elupus commented Feb 13, 2014

Seems it's missing at the moment. It's the "static const EFormatMap g_format_map" inside DVDCodecUtils.cpp. It needs a mapping from NV12 format to the same format in the ffmpeg world (PIX_FMT_NV12).

Shine- commented Feb 13, 2014

I think NV12 will never arrive at CProcessor::Add anyway, because all previous processing will happen in YV12 (and therefore be converted to YV12 earlier on anyway). The exception was YUV420P10/P16, for which @elupus added the native processing code.

So all that needs to be handled in CProcessor::Add is YV12, YV12@10bit and YV12@16bit . Neither NV12, UYVY nor YUY2 should be advertised in Pre-Init() when DXVA rendering is active. It won't harm playback, but only because ffmpeg fails to deliver and falls back to YV12 on its own - while spamming your debug log with multiple error messages for each single frame.

Could be wrong, but that's what I remember seeing while regression-testing #4163 and #4192 with YUV422 source material (MJPEG).

Contributor

a11599 commented Feb 13, 2014

Does this work with libmpeg2 decoded frames (DVD menus) also?

Member

elupus commented Feb 13, 2014

@a11599 indeed no. It has no color space conversion, forgot that.

@elupus elupus commented on an outdated diff Feb 13, 2014

xbmc/cores/dvdplayer/DVDCodecs/Video/DXVA.cpp
@@ -1207,6 +1222,39 @@ bool CProcessor::Open(UINT width, UINT height, unsigned int flags, unsigned int
return true;
}
+bool CProcessor::InitSWScaleContext(UINT width, UINT height, unsigned int srcFormat, unsigned int dstFormat)
+{
+ PixelFormat pxFormat = PIX_FMT_NONE;
+
+ if (srcFormat == RENDER_FMT_YUV420P) pxFormat = PIX_FMT_YUV420P;
+ else if (srcFormat == RENDER_FMT_YUV420P10) pxFormat = PIX_FMT_YUV420P10;
+ else if (srcFormat == RENDER_FMT_YUV420P16) pxFormat = PIX_FMT_YUV420P16;
+ else if (srcFormat == RENDER_FMT_NV12) pxFormat = PIX_FMT_NV12;
+ else if (srcFormat == RENDER_FMT_UYVY422) pxFormat = PIX_FMT_UYVY422;
+ else if (srcFormat == RENDER_FMT_YUYV422) pxFormat = PIX_FMT_YUYV422;
@elupus

elupus Feb 13, 2014

Member

Use helper function for this.

@elupus

elupus Feb 13, 2014

Member

CDVDCodecUtils::PixfmtFromEFormat

Member

afedchin commented Feb 13, 2014

Updated. Now work with libmpeg2.

Member

afedchin commented Feb 13, 2014

This seem a bit silly don't it? dvdplayer will perform that exact conversion if we don't report them as supported in the Supports() call.

As I understand from sources the CXBMCRenderManager firstly trying to add the picture to the renderer then perform a conversion if renderer don't accept it. CWinRenderer pass picture to DXVA::CProcessor(HD) if m_renderMethod == RENDER_DXVA. Also dxva render has own buffer of pictures (IDirect3DSurface9 surfaces).

Member

elupus commented Feb 15, 2014

dvdplayer will first ask for supported pixel formats, then Configure() with one that is supported, then AddProcessor().

The libavcodec (ffmpeg) based decoder will automatically do pixel format conversion to a supported format that was reported in Supports(). libmpeg2 will however not, but will always just output YV12.

So renderers only every need to support YV12, everything else will be converted before it get's to the renderer by just reporting proper data in the Supports function.

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Feb 17, 2014

Member

afedchin commented Feb 24, 2014

Not sure whether this still has a chance of getting merged after #4192. But still, is there any chance to be merged?

Member

elupus commented Feb 25, 2014

It would be for G+. But the description need to be updated to say why this is needed.

Owner

MartijnKaijser commented May 21, 2014

@afedchin care to update

Member

afedchin commented May 21, 2014

sure. I'll update this asap.

On Wed, May 21, 2014 at 8:40 PM, Martijn Kaijser
notifications@github.comwrote:

@afedchin https://github.com/afedchin care to update


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/4201#issuecomment-43782666
.

Best Regards,
Anton Fedchin

http://www.ruswizards.com

Mobile: +7 918-506-8-506 (Russia)
Skype: anightik

Member

afedchin commented May 27, 2014

rebased and updated.

@afedchin afedchin closed this Mar 15, 2015

@afedchin afedchin deleted the afedchin:dxva_formats branch Mar 15, 2015

@MartijnKaijser MartijnKaijser modified the milestones: Pending for inclusion, Abandoned, obsolete or superseeded Mar 21, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment