[FFmpeg-devel] [PATCH 10/14] swscale/range_convert: fix mpeg ranges in yuv range conversion for non-8-bit pixel formats

[Andrew Randrianasulu]

вс, 29 сент. 2024 г., 14:01 Christophe Gisquet <christophe.gisquet at gmail.com
>:

> Hi,
>
> Le sam. 28 sept. 2024 à 18:16, Ramiro Polla <ramiro.polla at gmail.com> a
> écrit :
> > I think you missed that the internal representation (the contents of
> > dst) is 15 bits for bit_depth <= 14, and 19 bits for bit_depth > 14.
> > It is confusing, but when I say bit depths > 14, I mean input bit
> > depths > 14, which have an internal representation of 19 bits (and
> > therefore the contents of dst are int32_t). Look at chrRangeToJpeg16_c
> > for example.
>
> Yes I did miss these, even I suspected it was there: I mentioned I
> suspect the discussion would end up about the amount of casting and
> function prototypes.
>
> > For bit depth > 14, I had to raise the precision of the calculation in
> > order to respect the mpeg range. In this case, the shift is 18 bits
> > instead of 14, and the offset becomes greater than 32 bits.
>
> I don't know what the dst buffers are for (or their dynamics), but I
> have no problem accepting this is the way.
>
> > lum from 16 coeff 224260 (0x036c04) offset   8589631520 ( 0x01fffb6020)
> > chr from 16 coeff 229380 (0x038004) offset   8589672480 ( 0x01fffc0020)
> > lum to   16 coeff 306429 (0x04acfd) offset -10041292800 (-0x025681f800)
> > chr to   16 coeff 299589 (0x049245) offset  -9817128960 (-0x0249258000)
>
> Yes as you said, only > 14 requires int32_t dst buffers, int32_t coeff
> and int64_t offset.
> I can understand you didn't want to bother with this rare case beyond
> having offset this large.
>

As far as I understand our cinelerra-gg can use 16bit per channel buffer
when converting from internal floatingpoint format so something libavcodec
encoders can take as input. So it might be rare operation inside ffmpeg,
but not for bigger world.




> > The API is indeed a bit confusing because it is shared for all bit
> > depths. It should be:
> > (int16_t *dst, int width, uint16_t coeff, int32_t offset) for bit_depth
> <= 14
> > and
> > (int32_t *dst, int width, uint32_t coeff, int64_t offset) for bit_depth
> > 14
>
> My point, but cf. just above. Except maybe, use ptrdiff_t for width,
> to avoid the Win64 ABI woes.
>
> In any case, I didn't follow well enough the complete changes, and I
> didn't look for the corresponding SIMD changes I was also concerned
> with. But given the above, it must be fine.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>