Jan Ekström:
> On Sun, Aug 20, 2023 at 10:10 AM Andreas Rheinhardt
> <andreas.rheinha...@outlook.com> wrote:
>>
>> Jan Ekström:
>>> These two were added in 28e23d7f348c78d49a726c7469f9d4e38edec341
>>> and 3558c1f2e97455e0b89edef31b9a72ab7fa30550 for version 0.9.0 of
>>> SVT-AV1, which is also our minimum requirement right now.
>>>
>>> In other words, no additional version limiting conditions seem
>>> to be required.
>>>
>>> Additionally, add a FATE test which verifies that pass-through of
>>> the MDCV/CLL side data is working during encoding.
>>> ---
>>> libavcodec/libsvtav1.c | 70 ++++++++++++++++++++++++++++++++++
>>> tests/fate/enc_external.mak | 5 +++
>>> tests/ref/fate/libsvtav1-hdr10 | 14 +++++++
>>> 3 files changed, 89 insertions(+)
>>> create mode 100644 tests/ref/fate/libsvtav1-hdr10
>>>
>>> diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
>>> index f2b73361d8..d4f9fa14ba 100644
>>> --- a/libavcodec/libsvtav1.c
>>> +++ b/libavcodec/libsvtav1.c
>>> @@ -24,9 +24,11 @@
>>> #include <EbSvtAv1ErrorCodes.h>
>>> #include <EbSvtAv1Enc.h>
>>>
>>> +#include "libavutil/bswap.h"
>>> #include "libavutil/common.h"
>>> #include "libavutil/frame.h"
>>> #include "libavutil/imgutils.h"
>>> +#include "libavutil/mastering_display_metadata.h"
>>> #include "libavutil/opt.h"
>>> #include "libavutil/pixdesc.h"
>>> #include "libavutil/avassert.h"
>>> @@ -146,6 +148,72 @@ static int alloc_buffer(EbSvtAv1EncConfiguration
>>> *config, SvtContext *svt_enc)
>>>
>>> }
>>>
>>> +static void handle_mdcv(struct EbSvtAv1MasteringDisplayInfo *dst,
>>> + const AVMasteringDisplayMetadata *mdcv)
>>> +{
>>> + struct EbSvtAv1ChromaPoints *points[] = {
>>> + &dst->r,
>>> + &dst->g,
>>> + &dst->b,
>>> + };
>>> +
>>> + if (!mdcv->has_primaries)
>>> + goto skip_primaries;
>>> +
>>> + for (int i = 0; i < 3; i++) {
>>> + struct EbSvtAv1ChromaPoints *dst = points[i];
>>> + const AVRational *src = mdcv->display_primaries[i];
>>> +
>>> + dst->x =
>>> + AV_BSWAP16C(av_rescale_q(1, src[0],
>>> + (AVRational){ 1, (1 << 16) }));
>>
>> Can you explain what's the matter with the AV_BSWAP16C here? And if I am
>> not mistaken, then the av_rescale_q() above is equivalent to src[0] * (1
>> << 16) (if we had multiplications of AVRationals by integers).
>
> It's basically a way that I got the big-endian values written out to
> the uint16_t. Same goes for the uint32_t values and AV_BSWAP32C.
This would only work on a little endian system (on big endian systems,
this approach would write the values as little-endian); the correct way
to write BE is via AV_WB16/32.
Not
> sure if there is a better macro for handling this on both BE and LE.
>
> SVT-AV1 seems to just write these values into the bit stream as-is and
> the interface seems to be defined around that (f.ex. in the string
> based interface `x << 8 | x >> 8` is always done to the parsed value).
Honestly, I do not get why they are using intswap16/32 unconditionally
even for BE systems. It seems to contradict their own documentation of
always putting BE values into the struct.
>
> https://gitlab.com/AOMediaCodec/SVT-AV1/-/blob/7233e7ead5fe8a0f1dc33b91aba0c5a24cb9f673/Source/Lib/Encoder/Codec/EbEntropyCoding.c#L3626-3637
>
The correct reference would be
https://gitlab.com/AOMediaCodec/SVT-AV1/-/blob/7233e7ead5fe8a0f1dc33b91aba0c5a24cb9f673/Source/API/EbSvtAv1Enc.h#L93
("values are stored in BE format"); you should not rely on internals of
other projects.
The reason for using BE should be explicitly documented.
- 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-requ...@ffmpeg.org with subject "unsubscribe".
