Hi Carl, On Thu, Nov 17, 2016 at 3:10 PM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote:
> 2016-11-17 19:34 GMT+01:00 Ronald S. Bultje <rsbul...@gmail.com>: > > Carl, the reason the patch is wrong is that luma range does not scale up > > from 16<<n to (236<<n)-1, but from 16<<n to (236-1)<<n, where n is bpc-8. > > This is documented in numerous places, see e.g. > > https://msdn.microsoft.com/en-us/library/windows/desktop/ > bb970578(v=vs.85).aspx > > : > > > > "For example, if the white point of an 8-bit format is 235, the > > corresponding 10-bit format has a white point at 940 (235 × 4)." > > This is indeed very difficult to understand: How is this related? > > AV_PIX_FMT_GRAY was changed years ago after several users > protested that it did conform to above specification, since it doesn't > do now, the paragraph looks unrelated. > (AV_PIX_FMT_GRAY is full-scale as is AV_PIX_FMT_GRAY16) > > More important: > My patch is not related to 10-bit output format, so above calculations > are also not related afaict. The relevant field in the decoder is called bits_per_raw_sample. If this is 10, the format is 10-bit. The output format is therefore also 10-bit, otherwise it's not lossless. How you represent that in an uint16_t is up to you, obviously, but it's 10bit, right? So the patch relates to 10bit formats. The question then seems, what do you do with the ("filler") bits if we shift them to MSB in gray16, i.e. packed_at_lsb=0? My assumption (from the decision to represent the coding as gray16 while setting bits_per_raw_sample) is that you want it to represent a value in MSB of uint16_t that would be closest to what it would have been if the value was actually coded as 16bit, right? (I'm assuming the above is logical and we all agree on it, please let me know if you disagree, otherwise let's go on to the apparently controversial stuff.) > Your patch is therefore theoretically wrong. The correct behaviour in > > limited-range upscaling is indeed to leave the lower bits empty. For > > full-range, the lower bits might be filled if the intention is for the > > pixel value to be a representation of what the 16bit result would look > > like, but whether this belongs in a decoder or not is up for discussion. > > Decoders tend to be used by video players and if white looks gray on a > screen, it doesn't make much sense to say "the player should have > worked-around this". I'm fine with that, but it depends on the format. If the input was full-range, then yes. If the input was not full-range, then no. Assume I have a H264 file with a PPS range=limited and a chroma_idc=0 (4:0:0, i.e. grayscale). You agree from the H264 spec that this is a legal file, right? I could also generate source data by taking any (10bit) Bt709 YUV file (which almost universally use limited-range coding) and stripping the UV planes out and leaving the Y data untouched. How do I represent this data losslessly in ffv1 after your patch? I don't think I can, because a value of 0x3ac (235x4=940, i.e. white) would be upshifted to (0x3ac << 6) || (0x3ac >> 4) = 0xeb3a, which converting back (as e.g. swscale does) to 10bits is 940.9, so after rounding it would be 941. That is not 940, thus not lossless and therefore a bug (IMO). I agree that if the data was fullrange, your patch may be correct (it depends on some other stuff, but I don't want to get into the he-said-she-said stuff, I'm describing a bug here), but your patch breaks something that worked previously. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel