[FFmpeg-devel] [PATCH v9 4/6] lavc: Implement Dolby Vision RPU parsing

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Mon Jan 3 07:42:15 EET 2022


Hendrik Leppkes:
> Andreas Rheinhardt <andreas.rheinhardt at outlook.com> schrieb am So., 2. Jan.
> 2022, 05:18:
> 
>> Niklas Haas:
>>> From: Niklas Haas <git at haasn.dev>
>>>
>>> Based on a mixture of guesswork, partial documentation in patents, and
>>> reverse engineering of real-world samples. Confirmed working for all the
>>> samples I've thrown at it.
>>>
>>> Contains some annoying machinery to persist these values in between
>>> frames, which is needed in theory even though I've never actually seen a
>>> sample that relies on it in practice. May or may not work.
>>>
>>> Since the distinction matters greatly for parsing the color matrix
>>> values, this includes a small helper function to guess the right profile
>>> from the RPU itself in case the user has forgotten to forward the dovi
>>> configuration record to the decoder. (Which in practice, only ffmpeg.c
>>> and ffplay do..)
>>>
>>> Notable omissions / deviations:
>>> - CRC32 verification. This is based on the MPEG2 CRC32 type, which does
>>>   not seem to be implemented in lavu. (And I don't care enough to do so)
>>> - Linear interpolation support. Nothing documents this (beyond its
>>>   existence) and no samples use it, so impossible to implement.
>>> - All of the extension metadata blocks, but these contain values that
>>>   seem largely congruent with ST2094, HDR10, or other existing forms of
>>>   side data, so I will defer parsing/attaching them to a future commit.
>>> - The patent describes a mechanism for predicting coefficients from
>>>   previous RPUs, but the bit for the flag whether to use the
>>>   prediction deltas or signal entirely new coefficients does not seem to
>>>   be present in actual RPUs, so we ignore this subsystem entirely.
>>> - In the patent's spec, the NLQ subsystem also loops over
>>>   num_nlq_pivots, but even in the patent the number is hard-coded to one
>>>   iteration rather than signalled. So we only store one set of coefs.
>>>
>>> Heavily influenced by https://github.com/quietvoid/dovi_tool
>>> Documentation drawn from US Patent 10,701,399 B2 and ETSI GS CCM 001
>>>
>>> Signed-off-by: Niklas Haas <git at haasn.dev>
>>> ---
>>>  configure             |   2 +
>>>  libavcodec/Makefile   |   1 +
>>>  libavcodec/dovi_rpu.c | 430 ++++++++++++++++++++++++++++++++++++++++++
>>>  libavcodec/dovi_rpu.h |  71 +++++++
>>>  4 files changed, 504 insertions(+)
>>>  create mode 100644 libavcodec/dovi_rpu.c
>>>  create mode 100644 libavcodec/dovi_rpu.h
>>>
>>> diff --git a/configure b/configure
>>> index 0ccd3bda11..68658a847f 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2434,6 +2434,7 @@ CONFIG_EXTRA="
>>>      cbs_vp9
>>>      dirac_parse
>>>      dnn
>>> +    dovi_rpu
>>>      dvprofile
>>>      exif
>>>      faandct
>>> @@ -2706,6 +2707,7 @@ cbs_mpeg2_select="cbs"
>>>  cbs_vp9_select="cbs"
>>>  dct_select="rdft"
>>>  dirac_parse_select="golomb"
>>> +dovi_rpu_select="golomb"
>>>  dnn_suggest="libtensorflow libopenvino"
>>>  dnn_deps="avformat swscale"
>>>  error_resilience_select="me_cmp"
>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>> index fb90ecea84..7364c7a91f 100644
>>> --- a/libavcodec/Makefile
>>> +++ b/libavcodec/Makefile
>>> @@ -77,6 +77,7 @@ OBJS-$(CONFIG_CBS_MPEG2)               += cbs_mpeg2.o
>>>  OBJS-$(CONFIG_CBS_VP9)                 += cbs_vp9.o
>>>  OBJS-$(CONFIG_CRYSTALHD)               += crystalhd.o
>>>  OBJS-$(CONFIG_DCT)                     += dct.o dct32_fixed.o
>> dct32_float.o
>>> +OBJS-$(CONFIG_DOVI_RPU)                += dovi_rpu.o
>>>  OBJS-$(CONFIG_ERROR_RESILIENCE)        += error_resilience.o
>>>  OBJS-$(CONFIG_EXIF)                    += exif.o tiff_common.o
>>>  OBJS-$(CONFIG_FAANDCT)                 += faandct.o
>>> diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
>>> new file mode 100644
>>> index 0000000000..fc2e1fb2a1
>>> --- /dev/null
>>> +++ b/libavcodec/dovi_rpu.c
>>> @@ -0,0 +1,430 @@
>>> +/*
>>> + * Dolby Vision RPU decoder
>>> + *
>>> + * Copyright (C) 2021 Jan Ekström
>>> + * Copyright (C) 2021 Niklas Haas
>>> + *
>>> + * This file is part of FFmpeg.
>>> + *
>>> + * FFmpeg is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * FFmpeg is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with FFmpeg; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> 02110-1301 USA
>>> + */
>>> +
>>> +#include "libavutil/buffer.h"
>>> +
>>> +#include "dovi_rpu.h"
>>> +#include "golomb.h"
>>> +#include "get_bits.h"
>>> +
>>> +enum {
>>> +    RPU_COEFF_FIXED = 0,
>>> +    RPU_COEFF_FLOAT = 1,
>>> +};
>>> +
>>> +/**
>>> + * Private contents of vdr_ref.
>>> + */
>>> +typedef struct DOVIVdrRef {
>>> +    AVDOVIDataMapping mapping;
>>> +    AVDOVIColorMetadata color;
>>> +} DOVIVdrRef;
>>> +
>>> +void ff_dovi_ctx_unref(DOVIContext *s)
>>> +{
>>> +    for (int i = 0; i < FF_ARRAY_ELEMS(s->vdr_ref); i++)
>>> +        av_buffer_unref(&s->vdr_ref[i]);
>>> +
>>> +    /* Preserve the user-provided fields explicitly, reset everything
>> else */
>>> +    *s = (DOVIContext) {
>>> +        .logctx = s->logctx,
>>> +        .config = s->config,
>>> +    };
>>> +}
>>> +
>>> +int ff_dovi_ctx_replace(DOVIContext *s, const DOVIContext *s0)
>>> +{
>>> +    int ret;
>>> +    s->logctx = s0->logctx;
>>> +    s->config = s0->config;
>>> +    s->mapping = s0->mapping;
>>> +    s->color = s0->color;
>>> +    for (int i = 0; i < DOVI_MAX_DM_ID; i++) {
>>> +        if ((ret = av_buffer_replace(&s->vdr_ref[i], s0->vdr_ref[i])) <
>> 0)
>>> +            goto fail;
>>> +    }
>>> +
>>> +    return 0;
>>> +
>>> +fail:
>>> +    ff_dovi_ctx_unref(s);
>>> +    return ret;
>>> +}
>>> +
>>> +int ff_dovi_attach_side_data(DOVIContext *s, AVFrame *frame)
>>> +{
>>> +    AVFrameSideData *sd;
>>> +    AVBufferRef *buf;
>>> +    AVDOVIMetadata *dovi;
>>> +    size_t dovi_size;
>>> +
>>> +    if (!s->mapping || !s->color)
>>> +        return 0; /* incomplete dovi metadata */
>>> +
>>> +    dovi = av_dovi_metadata_alloc(&dovi_size);
>>> +    if (!dovi)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    buf = av_buffer_create((uint8_t *) dovi, dovi_size, NULL, NULL, 0);
>>> +    if (!buf) {
>>> +        av_free(dovi);
>>> +        return AVERROR(ENOMEM);
>>> +    }
>>> +
>>> +    sd = av_frame_new_side_data_from_buf(frame,
>> AV_FRAME_DATA_DOVI_METADATA, buf);
>>> +    if (!sd) {
>>> +        av_buffer_unref(&buf);
>>> +        return AVERROR(ENOMEM);
>>> +    }
>>> +
>>> +    memcpy(av_dovi_get_header(dovi), &s->header, sizeof(s->header));
>>> +    memcpy(av_dovi_get_mapping(dovi), s->mapping, sizeof(*s->mapping));
>>> +    memcpy(av_dovi_get_color(dovi), s->color, sizeof(*s->color));
>>
>> These are potentially problematic due to trailing padding in the
>> structures. This trailing padding might become valid fields in newer
>> versions of this structure. E.g. av_dovi_metadata_alloc() might
>> intentionally set the default values of these new fields to invalid
>> values. Yet lavc (with the old structure) sees them as padding, allowing
>> the compiler to trash these values on every write to these structures.
>> More likely though is that they will never be touched at all and
>> therefore retain their default (zero) initialization. Which might
>> conflict with the default value of these fields.
>> This can be fixed by only copying everything up to and including the
>> last field that we know of for every of these structures. This will
>> unfortunately increase the maintainence burden a bit.
>>
> 
> This code is the primary source of any data in this struct. If it ever gets
> extended, this parser wanting to store more fields would be why, so
> initialization doesn't seem overly worrysome.
> 

It is indeed reasonable to expect that any extension to any of these
structs would be necessitated by this code and patches to this code
making use of these new fields will be part of the same patchset adding
the new fields. But even then it is still possible to use a new lavu
with an old lavc in which case the above mentioned problem exists.
This scenario could be precluded by forbidding mixing libraries from
different snapshots.

- Andreas


More information about the ffmpeg-devel mailing list