-
Notifications
You must be signed in to change notification settings - Fork 7
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
PC scale color range doesn't seem to work as expected #40
Comments
I'll leave the ffmpeg part to you to figure out, but here's the explanation about YUVRGB_Scale: it doesn't describe the video's colour range. YUVRGB_Scale describes the desired output range when converting to RGB. It's explained in this part of the manual: http://rationalqm.us/dgmpgdec/DGIndexManual.html#YUVtoRGB So when the manual says
what it means is
Huh?If you want VapourSynth filters to assume the video has limited range, use YUVRGB_Scale=1 (PC). If you want them to assume the video has full range, use YUVRGB_Scale=0 (TV). |
Edit: VapourSynth Editor converts to RGB, which is why the value seemed to be taking effect. |
Yes, limited range is also known as TV range, full range is also known as PC range. I know, it's confusing. YUVRGB_Scale is basically backwards compared to what one might reasonably expect. Read the explanation and that part of the manual again until it makes sense. And yes, if you're displaying the video on your computer screen there is YUV to RGB conversion happening somewhere even if you didn't write that in the script. VapourSynth Editor does it for you, taking into account the _ColorRange frame property, which D2V Source sets according to YUVRGB_Scale. That's why changing YUVRGB_Scale has an effect on what you see in the Preview window. |
It's not backwards/flipped. The issue here was the understanding of how YUVRGB_Scale should be used. It isn't an INPUT scale switch, it's an OUTPUT scale switch. YUVRGB_Scale does absolutely nothing until you want to convert YUV to RGB, as the name suggests. This is why it feels like it's backwards because it's for the output, not the input. Let's say the input was limited/TV (e.g. The IT Crowd) and you wanted to load that in and convert it to full/PC? You would do YUVRGB_Scale=1, not YUVRGB_Scale=0. It also did not help that VapourSynth Editor previewer (and other decoders) would convert to RGB, following the YUVRGB_Scale behind the scenes, without knowing it only applied because of that. If you wanted that same source converted to RGB as limited/TV (RGB [16, 235]) you would do YUVRGB_Scale=0. However, if you then decode that in software/player expecting RGB [0, 255], then the blacks will be brighter and the whites will be darker. If you decode it in software/player that can detect it as limited/TV (either by ffmpeg's -color_range flag, manual means, or such) then it would look correct.
|
It was wrong before this commit: 3a1b750 It's correct now. Maybe you have an old version (1.0 or older)? |
-snip- |
Please read again: http://rationalqm.us/dgmpgdec/DGIndexManual.html#YUVtoRGB |
We're reading what you write. You seem to think this option specifies the range of the input video. No! It defines the range you want for the RGB conversion. Consider a black in YUV. It has value 16. Now the question is do you want that mapped to 16 in the RGB (TV range) or to 0 (PC range). Suppose you pick PC and map it to 0. Then the black will be darker than if you had picked TV. Same goes for white. Is it clear? |
Yes, that explanation does clear it up a bit. I always assumed that the mapping for 16,235 to 0,255 is the one that causes the brighter blacks and darker whites, but it is the other way around, hence the inverted sense. Am I correct in saying all YUV videos are Y(16,235) UV(16,240) (TV/limited range) input? Meaning no matter what, I would want to use -color_range tv on ffmpeg? |
In practice you'll find all sorts of videos, so you'll have to check with a histogram filter every time. |
If it is a DVD then yes, the source is encoded per 601, so the YUV has limited range. Beyond that dubhater is correct -- you better check. (I omit consideration of blurays, etc., because you are talking about DGIndex et al.) |
Even with DVDs you can't assume anything. I've seen a few where the luma went all the way to 255 in most scenes. I had some screenshots, but it looks like I deleted those. 25 out of 26 episodes of a certain anime series needed something like this:
From another anime:
|
I see, how would I check though? I see you have a vapoursynth-histogram plugin, which I have now installed and taken a look at, but now entirely sure how to use this to check. |
Just stopped by to say that this is illuminating. And every encode I've done from a DVD rip in the last few years has been wrong... Edit: On further though, I've got a question. If you just read in the d2v and then encode to h264/5 without any YUV-to-RGB conversion in your script, is YUVRGB_Scale passed into the output file in any way? Does it do anything at all in that situation? Second question: Am I correct to think that, unless you do a YUV-to-RGB conversion in your script, you need to do a fmtc.bitdepth() conversion if you want to get full scale? And then a fmtc.matrix() conversion from BT601 to HT709 after that? Third question: Would anyone care to venture some best practices for dealing with the various color issues converting from DVD to h264/5? |
Setting aside the issue of whether it's flipped, I'm more concerned with whether this value even matters in the first place. It appears that x264/5 have their own explicit input and output levels parameters, so I'm very much doubting this value is getting passed to the encoder. If the only thing this value actually does is control how YUV-to-RGB conversions are done inside vapoursynth and implicitly in vsedit's preview, then it sounds to me like it's often just irrelevant. Converting YUV-to-RGB inside vapoursynth is pretty uncommon. Beyond that, it sounds like you generally want to keep it set to TV for purposes of keeping vsedit's preview behaving in a "what you see is what you get" manner. As for the images, I think maybe your monitor needs adjustment/replacement. It's 110% visually obvious to me that the top image has had its range squashed. |
Yes, as explained, it matters only for the internal YUV->RGB conversion. It is not passed in the served video in any way. However, VS/AVS+ also have a frame property called _ColorRange that can be set by a source filter to claim the range of the video. But this is a totally different thing. Again, as explained, the value is not flipped. Finally, yes, the images are obviously different. |
Two RGB png images, have almost identical green channel (except the waveform thing on the right, and text on the top left). The blue and red channels are different definitely. Inspected with ffmpeg and its filters. |
Yes, it is expected. Please do the right thing and close this issue. |
Then how would I correctly encode this for viewing as limited range. When I use -color_space default in ffmpeg, MPV and others do not seem to read it correctly. Not only that, but the histogram still shows (basically) no data within purple, but still has some going out of it. Which makes me think its still showing 0->255, yet that was not the case for dubhater's example. Dubhaters example also again, did not make any noticeable difference to blacks or whites, yet mine is. |
The difference in dubhater's example is obvious. You are the only person doubting that. How you correctly encode a video that lies between TV and PC range is not relevant to your claim that d2vsource is wrong. So again, I ask you to do the right thing and close the issue. Look at things objectively and lose your need to be right. |
Perhaps dubhaters example looks different on my end only? On my end, his example looks identical other than the histogram. The colors look identical. I'm using the latest version of Firefox. Perhaps it's a weird firefox thing? Also, I don't care if I'm right or wrong, I want to learn that's all. |
"And to further confuse everyone" We're not confused, you are. You claimed a specific issue with d2vsource. We have shown that there is no issue. Now you want to use this issue to "learn about things", but that is not the purpose of a bug tracker. Close the issue and then go post about your confusion at Doom9, or some other video forum. I would he happy to try to help you there. |
Thanks for closing. Hope to see you follow-up at Doom9. There's one more point I'd like to make here. In the DGIndex tools the YUVRGB_Scale option only affects the YUV->RGB conversion performed by vfapi. But vfapi is almost never used anymore. d2vsource may not even be using YUVRGB_Scale. I'll have a look at the source code. That may account for everything you are seeing. |
OK, d2vsource sets the Vapoursynth frame property _ColorRange based on the YUVRGB_Scale value. However, the tools subsequently used (e.g., encoder) must be coded to actually use that frame property, or to pass it through where applicable. That's something we can follow-up at Doom9 if you like. |
Ha ha! Turns out you were right. d2vsource maps directly YUVRGB_Scale to _ColorRange. But _ColorRange is defined as: 0=full range, 1=limited range, while YUVRGB_Scale is defined as 0=limited, 1=full range. So that is flipped. Please re-open this issue. :-) |
What? I always assumed YUVRGB_Scale was ignored (since no RGB conversion is ever done by d2vsource) and the range was flagged according to the mpeg stream. Insanity. |
Yes, but d2vsource does copy YUVRGB_Scale to _ColorRange, and they have different definitions. Easy fix. |
But there can still be a confusion. The YUVRGB_Scale is not intended to signal the range of the input video, but rather how to scale when converting to RGB. Maybe YUVRGB_Scale should be completely ignored, because not only is the syntax different, but the semantics are as well. That's a decision for the author of d2vsource. |
Did a little more looking. We should just ignore YUVRGB_Scale and get things from the stream. In DGDecodeNV I set _ColorRange from CUVID/NVDec: vs_full_range = pFormat->video_signal_description.video_full_range_flag; So you'd have to parse the stream. But for sure let's get rid of YUVRGB_Scale. If you can't parse then simply don't define _ColorRange. d2vsource shouldn't make up things that are not coded in the D2V file. People can manually set _ColorRange just as easily as manually setting YUVRGB_Scale. |
Wait what? So I was right in the end? Which issue? This one or my pull request from a year ago that fixed it? |
That would in fact be the main confusion I had with this issue, but not with the pull request I linked a while back in here (#41), which further got me confused it seems. |
If d2vsource is going to retain the connection to YUVRGB_Scale, which is a wrong thing IMHO, then it is backwards as you reported and should be fixed. But IMHO it is better to completely sever that connection, due to different intents (semantics). And as I mentioned, people can manually set _ColorRange just as easily as manually setting YUVRGB_Scale. They are two different things; there is no reason to connect them. The d2vsource author will make that decision. |
It would also be problematic to unflip the value if people are unaware this change occurred and were used to the flipped methodology. As in, they may have gotten used to providing the "wrong" value, for it to be flipped to the "right" value. |
Which is another good reason to get rid of YUVRGB_Scale usage entirely! That variable was intended only to affect YUV->RGB conversion when serving via vsapi, which is long dead and buried. myrsloik rightly points out that using it is insanity. |
Whatever! It's not relevant here. Make a thread at Doom9. |
If vfapi is not used anymore, why do I notice the change in VapourSynth Editor? Does that still use it even after the API 4 releases? Even with the VapourSynth Editor fork by YomikoR that fixed the COMPAT API4 deprecation errors? |
Because d2vsource has connected YUVRGB_Scale to _ColorRange, when it is insane to do so. Look, you are setting YUVRGB_Scale depending on what you think about the source video. It's a manual thing. I'm just saying, get rid of that and simply set _ColorRange depending on what you think about the source video. You can put the prop set line in your script after the d2vsource invocation. Then it won't matter what d2vsource is doing. |
Wouldn't this mean, if I were to just use pymediainfo to parse the range information directly from the MPEG stream, then I could just essentially do away with YUVRGB_Scale entirely, like what you seem to want to happen now with d2vsource? If so, that's kind of great. And to be clear, _ColorRange is also specifying the output range here, right? |
_ColorRange is intended to specify the range of the input video, to allow proper downstream conversions/encodings. Yes, I am suggesting to ignore YUVRGB_Scale completely and simply specify _ColorRange (after the d2vsource invocation). Then it actually becomes moot if the d2vsource author does or does not remove the connection to YUVRGB_Scale, although IMHO it should be removed. The downstream tools will provide the means to set the desired output range. For that video you mentioned that is halfway between PC and TV, you will be in the position of Buridan's ass when deciding what to set for _ColorRange. |
I just want to add this, trying to be simple, because it was a stopper for a while. Something is not right if working with vapoursynth and d2vwitch, I had to just use experience to have desired result: clip=core.ffms2.Source("source.m2t") clip = core.d2v.Source(d2v_file_indexed_by_d2vwitch_with_defaults) clip = core.d2v.Source(d2v_file_indexed_by_d2vwitch_with_range_full) It should be straightened out somehow, for it causes some headaches when using mpeg2 files, not sure what or who is wrong here, just mentioning it, not knowing really, maybe source plugin is at fault, which one? Or dv2which or vapoursynth flag setting? Which one? |
Now the scale is simply ignored and _ColorRange is unset. I'll leave this open for a bit in case someone has a better idea later but it's a lot less wrong. |
Agreed. If d2vsource cannot parse the video then nothing can be done. Unless, of course, the app generating the D2V wants to change the D2V format to include the range, which would be parsed from the stream. |
It may be possible to get the range from FFmpeg and properly set things but once again I'm missing a relevant sample to actually test things. |
I guess I am confused. I offered to provide a sample for the open GOP thing in the other thread about ffms. Do you now need an MPEG2 file to test parsing of the range from the ES syntax for this thread? Here we talk about d2vsource not ffms. I can give that but any old MPEG2 file should suffice. Please clarify if I am confused. |
Never mind. I'm not going to try to implement range parsing. |
A fine decision IMHO. Thank you. I'll provide the sample in the other thread. |
Is the range even in the stream? |
Well, I know NVDec returns it. I have to do some swim coaching right now but later tonight I'll check the MPEG2 spec and report back. |
Ha! I had time to look. It is not specified in the MPEG2 syntax, so hat-tip dubhater. But NVDec returns some fields for video signal description (one of which is a full range flag); they are all 0 for MPEG2. Got snookered because for the per frame info, NVDec returns codec-specific data, but not for the sequence info. I just assumed they were handled the same and if NVDec returned a value, it must be present in the stream. So, to try to get closure on all this: Nobody should try to set _ColorRange for MPEG2 (and definitely not from YUVRGB_Scale); it must be set manually in the script after the d2vsource invocation. It is valid for some other formats. I'll check that and in DGSource() only set _ColorRange for formats that specify it in the syntax. Does all that sound OK to you? |
When would this change go to the main branch (I presume that's the plan?) and eventually to a release? Currently, in my vsfunc glob of code, I just manually ignore the _ColorRange set by d2vsource. |
It isn't no. What I'm currently doing is: video_track = next(iter(MediaInfo.parse(self.d2v.videos[0]).video_tracks), None)
if video_track and getattr(video_track, "color_range", None):
color_range = {"Full": 0, "Limited": 1}[video_track.color_range]
else:
color_range = 1 # assume limited/TV as MPEGs most likely are
self.clip = core.std.SetFrameProp(self.clip, "_ColorRange", color_range) The only reason why I did the MediaInfo check at all, was in case it was able to be inferred by MediaInfo, and since it was already a dependency for my stuff. I may remove it if it really is clear that no MPEG/MPEG-2 stream can have color range information in the ES. |
This is because essentially d2vsource sets the _ColorRange to the opposite of what the YUVRGB_Scale is. So if YUVRGB_Scale is PC, then _ColorRange will be TV. Its using YUVRGB_Scale for an incorrect purpose essentially. What this issue is pointing out is exactly that, exactly the confusion you and I are facing a bit by it. We already have this fixed in the newffmpeg branch but need to wait for a new release. Once this new release is out, _ColorRange won't be set at all. This means after this change, you simply load the clip, and if it isn't in the right range (because VapourSynth had no information and assumed x/y) then simply set _ColorRange to either 0 or 1. Perhaps in the future core.d2v.Source will get a new range parameter that we could use to specify the source, perhaps even make it required, either way, it would make the manual range adjustment more apparent to users. The whole reason we need to manually set it now when we didn't before is that most MPEG content is limited/TV range, and the default YUVRGB_Scale is full/PC, so when d2vsource used the opposite of YUVRGB_Scale to get However, it would be wrongly set if let's say you had a full/PC MPEG, then d2vsource would still set _ColorRange to tv, which would still be wrong, unless YUVRGB_Scale was set to tv. |
"It isn't no." That's a strange way to say yes. |
Well, that was wild and informative.
Does that sound right? |
Your step 1 is not required because _ColorRange will be overwritten by your step 3. |
The Problem
d2v_source
YUVRGB_Scale=1
(PC) with ffmpeg-color_range pc
(PC) returns a limited/TV color range output, not the expected full/PC color range.You may be thinking, "Oh, maybe the source is just limited/TV range then?", if that's the case, then choosing the value for YUVRGB_Scale and -color_range in FFMPEG doesn't seem to matter and it just ignores the manually specified value, otherwise it should have given me an FFMPEG output where the blacks and whites are washed out, it shouldn't look "normal" if the source isn't PC color range.
Left:
YUVRGB_Scale=1
ffmpeg -color_range pc
| Right:YUVRGB_Scale=1
ffmpeg -color_range default
(default is TV?)Left has saturated blacks and saturated whites. Right has correct colors outputting at presumably PC range, yet ffmpeg used
default
color_range switch which is presumablytv
and the D2V used YUVRGB_Scale=1 (PC). What?Edit: It's been discovered that it essentially needs the -color_range of the opposite that YUVRGB_Scale is, because d2vsource sets _ColorRange to be the opposite of what YUVRGB_Scale is. See #41 for more information and why it may or may not have initially been intentional.
Here are the results of the combinations possible:
Here's a reminder that according to the d2v file and DGIndex manual, YUVRGB_Scale's options are 0 for TV and 1 for PC.
VapourSynth-Editor's Preview Window also only had the correct colors for YUVRGB_Scale=1.
None of the options where I matched YUVRGB_Scale and -color_range did the color output work correctly.
The text was updated successfully, but these errors were encountered: