Re: [FFmpeg-devel] [PATCH 1/1] avutil/csp: create public API for colorspace structs

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Leo Izen:
> On 5/20/22 12:01, Andreas Rheinhardt wrote:
>> Leo Izen:
>>> This commit moves some of the functionality from avfilter/colorspace
>>> into avutil/csp and exposes it as a public API so it can be used by
>>> libavcodec and/or libavformat. It also converts those structs from
>>> double values to AVRational to make regression testing easier and
>>> more consistent.
>>> ---
>>> +
>>> +#include <stdlib.h>
>>> +
>>> +#include "attributes.h"
>>> +#include "csp.h"
>>> +#include "pixfmt.h"
>>> +#include "rational.h"
>>> +
>>> +#define AVR(d) { (int)(d * 30000), 30000 }
>>
>> Does this really lead to the intended values? After all, the cast does
>> not round to nearest, instead it just discards the fractional part (i.e.
>> rounds towards zero). You should probably use (int)(d * 30000 + 0.5) to
>> compensate for that.
>>
> 
> I could change it to do that. That said, I modeled this after the FIX
> macro in libavcodec/mpegaudio.h on line 60, which doesn't do that. Is
> that macro also incorrect, or is there a caveat here that makes these
> scenarios different?
> 

The values for mpegaudio.h are surely not rounded to nearest. I don't
know whether this is intended or not, but AFAIK the codecs using this
are not intended to be bitexact (as in: the concept of bitexactness does
not apply to these codecs because the output of a decoder is not exactly
defined anyway).

While some values are indeed rounded down when converting to double,
multiplying by 30000 seems to produce the intended values somehow. I
would nevertheless add 0.5.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".