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

in-accurate color with qsv_hevc #341

Closed
alatteri opened this issue Sep 20, 2023 · 23 comments
Closed

in-accurate color with qsv_hevc #341

alatteri opened this issue Sep 20, 2023 · 23 comments
Assignees

Comments

@alatteri
Copy link

Hello,

When using QSV_HEVC, the color is quite off. I'm wondering if this is a result of the RGB->YUV->RGB conversion.

Please watch the attached screen recording. Do you think there is any improvement that can be made?

Thanks.

Encoder:
uv -m 1316 -t decklink:codec=R12L --audio-filter delay:0:frames -c libavcodec:encoder=hevc_qsv:rc=qvbr:bitrate=24M:cqp=20 -s embedded --audio-capture-format channels=8 --audio-codec=AAC:bitrate=192K --param incompatible -P 5004

Receiver:
./UltraGrid.AppImage --tool uv -d decklink:synchronized:drift_fix --audio-delay -20 -r analog --audio-channel-map 0:0,1:1,2:2,3:3,4:4,5:5,6:6,7:7 --audio-scale none -P 5004 --param use-hw-accel,resampler=soxr,decoder-use-codec=R12L,force-lavd-decoder=hevc_qsv

qsv_hevc.mp4
@MartinPulec
Copy link
Collaborator

Hi, would you be able to provide a minimal command demonstrating the problem? I've tried:
uv -t testcard:c=R12L -c lavc:enc=hevc_qsv -d gl
and the testcard didn't visually look to be any different than without the compression. Using QSV for decompress and forcing R12L output as well didn't seem to make a difference. UltraGrid selected x2rgb10le for the compression but it looks like QSV is converting it to YUV internally.

I am using a Raptor Lake-P card, anyways, if it makes some difference.

@MartinPulec
Copy link
Collaborator

MartinPulec commented Sep 21, 2023

Ok, after little evaluation (using -c <compress> -t testcard:c=R12L:pattern=blank=0x000000aa -d dummy -p color) it looks like uncompressed and libx265 compressed video has YUV values [46,110,202], while hevc_qsv with X2RGB [74,101,202][59,104,202]¹.

I've also recorded the stream and looked at the metadata with MediaInfo and it looks like QSV converts to full-range YCbCr – this is slightly unfortunate, because UG cannot currently handle that and treats it as limited-range.

¹ UPDATED 20-10-2023: original values may not have been correct, re-measured with current appimage 962ce6b and cmd -c lavc:e=hevc_qsv -t testcard:c=R12L:pattern=blank=0x000000aa -d dummy -p color --param lavc-use-codec=x2rgb10le

@alatteri
Copy link
Author

Thanks for taking a look. I can also post source TIFF images of my patterns if it helps. Is there a fix possible for the issue?

Thanks,
Alan

it looks like QSV converts to full-range YCbCr – this is slightly unfortunate, because UG cannot currently handle that and treats it as limited-range.

@alatteri
Copy link
Author

attached are images sequences of both the RGB and mono luma ramps.

ramps.zip

@MartinPulec
Copy link
Collaborator

MartinPulec commented Sep 22, 2023

Well, I am not sure if QSV does the color-space conversion correctly, anyways, because:

ffmpeg -f lavfi -i color=#aa0000 -t 1 -pix_fmt xv30le -c:v hevc_qsv out.mp4

and

ffmpeg -f lavfi -i color=#aa0000 -t 1 -pix_fmt x2rgb10le -c:v hevc_qsv out.mp4

produce different levels - [170,0,0] for the first (correct) and [165,15,15] for the second when extracted to JPEG by FFmpeg (ffmpeg -i out.mp4 -frames 1 %d.jpg) and evaluated by ImageMagick (convert 1.jpg -resize 1x1 txt:-).[1] So it would be perhaps best to report to Intel. Also I could imagine that it won't be converted to YCbCr at all.

Anyways, I will disable X2RGB – the conversions will be done in SW by UG, so it would be less efficient but correct.

[1] but I am not 100% sure if FFmpeg handles correctly full-range YCbCr as well.... but the second case doesn't seem to me as full-range YCbCr interpreted as limited... actually it looks like the opposite - perhaps out.mp4 is actually limited but in container is incorrectly full-range?

@alatteri
Copy link
Author

alatteri commented Sep 22, 2023

Hi Martin,
Inline

So it would be perhaps best to report to Intel.

Sorry to ask this, but would you be able to do that? I don't believe I would have the proper technical knowledge to report that in a sensible concise manner to them.

Anyways, I will disable X2RGB – the conversions will be done in SW by UG, so it would be less efficient but correct.

Hmm... so what colorspace will be used for R12L --> QSV? I wonder if the SW conversion will still be realtime?

[1] but I am not 100% sure if FFmpeg handles correctly full-range YCbCr as well.... but the second case doesn't seem to me as full-range YCbCr interpreted as limited... actually it looks like the opposite - perhaps out.mp4 is actually limited but in container is incorrectly full-range?

I think using JPEG as the output format is not ideal for these testings as that is limited to 8bit, and once we goto 8bit the whole endeavor is useless anyway.

@MartinPulec
Copy link
Collaborator

I think using JPEG as the output format is not ideal for these testings as that is limited to 8bit, and once we goto 8bit the whole endeavor is useless anyway.

I've used it just to check if the values are shifted... you can use pnm, in which case it will use 16-bit samples. But then ImageMagick shows the values in percents, which seemed less clear to me.

@alatteri
Copy link
Author

Hi Martin..

I had the formatting wrong in my last reply.... I'll post it here again just to make sure you saw it. Sorry if this is annoying.

So it would be perhaps best to report to Intel.

Sorry to ask this, but would you be able to do that? I don't believe I would have the proper technical knowledge to report that in a sensible concise manner to them.

Anyways, I will disable X2RGB – the conversions will be done in SW by UG, so it would be less efficient but correct.

Hmm... so what colorspace will be used for R12L --> QSV? I wonder if the SW conversion will still be realtime?

@MartinPulec
Copy link
Collaborator

would you be able to do that?

Maybe, but I don't want to make any commitment here – I am not 100% sure if the problem is really in QSV or somewhere between FFmpeg and QSV. Also if it were fixed, the true is that UG doesn't yet support full-range YCbCr, so it depends what will be the decoded output. Even the debugging might consume some time with uncertain result. So I may eventually try something but without promises.

Hmm... so what colorspace will be used for R12L --> QSV?

XV30 - it is a packed 10-bit 4:4:4 YCbCr format.

I wonder if the SW conversion will still be realtime?

Don't know - as always, it depends on combination of actual video properties and processing power. If it won't perform for you, it might be a motivation to get the x2rgb pixfmt working. If it will, I won't be really interested in solving it, also because if it is really a bug (outside UG), it may be fixed independently then.

@MartinPulec
Copy link
Collaborator

MartinPulec commented Oct 20, 2023

Well, it was quite difficult to tackle but I think I got it!

The problem is very simple in the end, see above. After struggling with it for few days, I finally got it – QSV uses BT.601 for RGB->YCbCr conversion! I've computed by hand the values for RGB=[170,0,0] and in limited YCbCr. It is BT.709=[47,110,202] and BT.601=[61,102,202]. Although there is a difference 2 for Y and Cb, I think that it is likely clear that Intel is using BT.601.

Observation # 2 is that the encoded stream uses actually limited-range YCbCr; but it (incorrectly?) writes to metadata value of AVCodecContext::color_range (JPEG - full, MPEG - limited), which UG sets to full-range (I believe that correctly, because at the codec input there is full-range RGB; limited range conversion makes just the codec). But this is currently no issue for UG, because it doesn't honor full-range flag for YCbCr in decoder (yet).

I'll may to look into it next week – either UG doesn't set the color-space or QSV doesn't honor the setting.

@MartinPulec MartinPulec reopened this Oct 20, 2023
@MartinPulec MartinPulec self-assigned this Oct 20, 2023
@alatteri
Copy link
Author

Interesting. Thank you for looking into this. 10bit Full Range is the ideal goal. 0-1023.

@MartinPulec
Copy link
Collaborator

reported

Interesting. Thank you for looking into this. 10bit Full Range is the ideal goal. 0-1023.

It seems to me, that Intel is fixed to a limitted-range YCbCr BT.601, so I guess that we will end with this, anyways, but with correct metadata. This won't help for now, because UG YCbCR<->RGB conversions are currently expecting BT.709. hevc_qsv decoder doesn't offer RGB output, unfortunately (I thought that QSV could have converted to RGB back).

@alatteri
Copy link
Author

alatteri commented Nov 7, 2023

Hu Martin,
any thoughts on Intel's reply?
intel/cartwheel-ffmpeg#281

MartinPulec added a commit that referenced this issue Oct 2, 2024
If converting from YCbCr to RGB, use BT.601 coefficients.

refer to GH-341
@MartinPulec
Copy link
Collaborator

This pull request seems to be for a query API. It looks like that it would allow also negotiation/setting parameters. For us, in the best case we would like to keep RGB unconverted and compress it directly. Anyways, it isn't merged yet and even though it would, it is a question when would be the consequent stuff.

Anyways, to move this slightly forward, I've made a workaround that has 2 parts:

  1. UltraGrid on the sender marks the stream explicitly as limited YCbCr BT.601 (commit e6179ec)
  2. the information then is extracted by the UltraGrid receiver and the YCbCr->RGB coefficients according to BT.601 are used

There are some limitations - obviously when the output is displayed as a YCbCr, the display would need to treat it as BT.601 by itself. This can be toggled on for GL/SDL2 but not for DeckLink (I believe that it can be set to metadata but I doubt that the displaying device would honor that). Nevertheless, converting postprocessor shall be possible.

@alatteri
Copy link
Author

alatteri commented Oct 2, 2024

Hi Martin,

thanks for taking a look at this again.

Would this actually affect the image data?. ie. Go from RGB Full --> limited YCbCr BT.601 --> RGB Full or is this just tricking things with metadata? In the first case, then it wouldn't matter anyway, as such a manipulation of the image data would likely corrupt the integrity of the image beyond acceptable quality levels.

@alatteri
Copy link
Author

alatteri commented Oct 2, 2024

also, would the new Vulkan encoder capabilities in FFMPEG 7.1 overcome this issue while still providing hardware acceleration?

@MartinPulec
Copy link
Collaborator

Would this actually affect the image data?. ie. Go from RGB Full --> limited YCbCr BT.601 --> RGB Full or is this just tricking things with metadata? In the first case, then it wouldn't matter anyway, as such a manipulation of the image data would likely corrupt the integrity of the image beyond acceptable quality levels.

It shouldn't differ from converting over (from/to) limited BT.709. No color shift should occur. In terms of precision loss certainly 10-bit limited can hold less values than 10-bit full range (not talking about 12-bits) but I'd there should be at most -/+1 sample difference.

@MartinPulec
Copy link
Collaborator

also, would the new Vulkan encoder capabilities in FFMPEG 7.1 overcome this issue while still providing hardware acceleration?

Hard to tell, I think it has the same backend. But there may be some difference in the workflow (mainly if RGB won't be converted). Nevertheless, Vulkan encode cannot be used with UltraGrid right now (it would require some effort).

@alatteri
Copy link
Author

Intel recently released oneVPL 24.3.4 which includes a new parameter MFX_EXTBUFF_VIDEO_SIGNAL_INFO which amount other things contains a value that can be set for VideoFormat, VideoFullRange and other characteristics. Is this what is needed to properly handle this situation?

https://intel.github.io/libvpl/latest/API_ref/VPL_structs_cross_component.html#mfxextvideosignalinfo

Screenshot 2024-10-24 at 4 53 00 AM

@MartinPulec
Copy link
Collaborator

Is this what is needed to properly handle this situation?

Why do you think it is new? If you look at this line and around, it is already used by FFmpeg and the commit from 2021. But yes, if implemented (which seemingly isn't), it should work.

Anyways, can you confirm if the workaround works for you? As the issue is not directly UltraGrid one and we have a workaround, I am rather about to close the issue.

@alatteri
Copy link
Author

alatteri commented Nov 8, 2024

I won't have the ability to look at this workaround for a little while. I understand that this limitation is outside the scope of UG. Thank you for taking the time to try and address this issue.

@alatteri alatteri closed this as completed Nov 8, 2024
@MartinPulec
Copy link
Collaborator

Ok, thanks for the info, anyways. We can return to it later again - I believe that the proper solution would be great, anyways.

We can monitor the situation if it won't change. I'll perhaps try to re-test this if I am correct about it in my comment above. But IIRC it was just setting just in the metadata but the conversion was limited BT.601 in any case.

@MartinPulec
Copy link
Collaborator

Unfortunately, no update, indeed. Tested with these commands, Intel Arc A770, Arch Linux, ffmpeg-git (9ec042c1aa), intel-media-driver/vpl 24.4.2 - same results.

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

No branches or pull requests

2 participants