obsproject / obs-amd-encoder

AMD Advanced Media Framework Encoder Plugin for Open Broadcaster Studio
https://obsproject.com/forum/threads/amd-advanced-media-framework-encoder-plugin-for-obs-studio.52305/
GNU General Public License v2.0
458 stars 85 forks source link

Full Range color broken in 25.0.1 #400

Closed Glitchvid closed 5 months ago

Glitchvid commented 4 years ago

Description

NV12 709/Full encodes broken (clipping/crushing/stretched) colors. This was not an issue on 24.0.3 Included are video files (and logs) demonstrating regression.

Expected Behavior

NV12 709/Full should not clip white colors, or crush black colors.

Reproduction Steps

  1. Record color bars in 709/Full
  2. Play back color bars in media player and compare to image

System Information

Checklist:

Addendum:

Video/Image/Logs: amf_bug.zip I'm not sure if this is simply the AMD SDK/HW not ever supporting 709/Full in the first place, and something doing fixup along the way, with later versions deciding this fixup isn't needed/desired (thus forcing users to correct their input to 709/Partial) – or an actual regression.

I've tried comparing the new 709/Partial to the old 709/Full, and I do see less banding and dither noise on the gray gradients, so that leads me to believe 709/Full was working, and is now broken.

Here's a image diff between the old 709/Full and new 709/Partial: diff Notice the bands in the top section.

as-com commented 4 years ago

I can confirm this is happening on my system (with a Vega 56) - I was wondering why my videos suddenly became too dark.

as-com commented 4 years ago

I found that downgrading this specific plugin to version 2.5.0 fixes the problem. Version 2.5.1 is the version where this issue started happening. So, the problem is in these commits: https://github.com/obsproject/obs-amd-encoder/compare/2.5.0...2.5.1

But it makes no sense that such a change would break full color range support... especially after tracing through the code a few times.

e00E commented 4 years ago

I can confirm this on my system with Windows 10 64 bit, Rx 5700 XT.

My obs config basic.ini contains:

[Video]
ColorRange=Full
ColorSpace=709

[AdvOut]
RecEncoder=amd_amf_h265

My recordings are correct (colospace is full) in obs version 24.0.3 and incorrect in versions 25.0.8 and 26.0.0 (didn't test other releases). I determine the colorspace of a recording with ffprobe which either reports yuvj420p(pc) (good) or yuv420p(pc) (bad).

e00E commented 4 years ago

I've looked into this more: in amf-encoder-h265.cpp we set NominalRange. I cannot find any reference to this property in the amf documentation. Based on amf-encoder-h264 it looks like this used to be supported in an older version of amf. So this property, that is trying to set the color range is wrong in the current plugin code. The h264 version switched to AMF_VIDEO_ENCODER_FULL_RANGE_COLOR. I cannot find this in the amf hevc documentation either. Maybe setting the color range is not possible with amf hevc?

Edit: Maybe AMF_VIDEO_ENCODER_HEVC_INPUT_COLOR_PROFILE and AMF_VIDEO_ENCODER_HEVC_OUTPUT_COLOR_PROFILE are relevant combined with AMF_VIDEO_CONVERTER_COLOR_PROFILE_FULL_709 https://github.com/GPUOpen-LibrariesAndSDKs/AMF/blob/802f92ee52b9efa77bf0d3ea8bfaed6040cdd35e/amf/public/include/components/ColorSpace.h#L53 . However the header bundled here seems to be of an older version and doesn't include this enum value.

Xaymar commented 4 years ago

Sorry for jumping in late, as I wrote the encoder implements, I'll give some insight on this:

  1. When I originally wrote the plugin, AMD did not document a lot of the options necessary to actually perform a proper encoding. Most of the behavior you see in the plugin is taken from reverse engineering, and was targeting an older SDK and driver version.
  2. Over the years, AMD has made increasingly version incompatible changes repeatedly. This even happened multiple times during the development, where I had to redo step 1 as the documentation wasn't updated with the change.
  3. This entire thing needs to be burned to the ground and rewritten - it can be done with way less overhead if one actually tried to. Most of the code here is actually just legacy support.

What you suggested is correct, the new method is actually setting input and output color profiles. However I'd suggest starting from scratch instead of continueing this if you do intend to implement it - you'll save yourself lots of pain with all the abstractions I wrote.

e00E commented 3 years ago

Like Xaymar suggested I ended up rewriting this plugin:https://github.com/e00E/obs-amf It works and is usable but not as easy to use as this one because I do not provide builds or easy configuration.

RytoEX commented 5 months ago

This repository is being archived. All Issues are being closed. Thank you for understanding.