Re: [FFmpeg-devel] [PATCH v3 08/13] avcodec/dovi_rpudec: make `enable` also affect decoding

[Niklas Haas <ffmpeg@haasn.xyz>]

On Thu, 18 Apr 2024 13:30:39 +0200 Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> Niklas Haas:
> > From: Niklas Haas <git@haasn.dev>
> > 
> > This could be used by codecs to selectively disable parsing Dolby Vision
> > RPUs, and is cheap to support.
> > ---
> >  libavcodec/av1dec.c      | 1 +
> >  libavcodec/dovi_rpu.h    | 2 ++
> >  libavcodec/dovi_rpudec.c | 6 ++++++
> >  libavcodec/hevcdec.c     | 1 +
> >  libavcodec/libdav1d.c    | 1 +
> >  5 files changed, 11 insertions(+)
> > 
> > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> > index 4c1405df77..20865b4f12 100644
> > --- a/libavcodec/av1dec.c
> > +++ b/libavcodec/av1dec.c
> > @@ -1551,6 +1551,7 @@ static void av1_decode_flush(AVCodecContext *avctx)
> >  static const AVOption av1_options[] = {
> >      { "operating_point",  "Select an operating point of the scalable bitstream",
> >                            OFFSET(operating_point), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, AV1_MAX_OPERATING_POINTS - 1, VD },
> > +    { "dolbyvision", "Decode Dolby Vision RPUs", OFFSET(dovi.enable), AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, VD },
> >      { NULL }
> >  };
> >  
> > diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
> > index 3e80647422..8f8905b96b 100644
> > --- a/libavcodec/dovi_rpu.h
> > +++ b/libavcodec/dovi_rpu.h
> > @@ -37,6 +37,8 @@ typedef struct DOVIContext {
> >       *
> >       * For encoding, FF_DOVI_AUTOMATIC enables Dolby Vision only if
> >       * avctx->decoded_side_data contains an AVDOVIMetadata.
> > +     *
> > +     * For decoding, FF_DOVI_AUTOMATIC has the same meaning as 1.
> >       */
> >  #define FF_DOVI_AUTOMATIC -1
> >      int enable;
> > diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c
> > index 7c7eda9d09..978d5dfc2b 100644
> > --- a/libavcodec/dovi_rpudec.c
> > +++ b/libavcodec/dovi_rpudec.c
> > @@ -37,6 +37,9 @@ int ff_dovi_attach_side_data(DOVIContext *s, AVFrame *frame)
> >      AVDOVIMetadata *dovi;
> >      size_t dovi_size, ext_sz;
> >  
> > +    if (!s->enable)
> > +        return 0;
> > +
> >      if (!s->mapping || !s->color)
> >          return 0; /* incomplete dovi metadata */
> >  
> > @@ -306,6 +309,9 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size,
> >      uint8_t use_nlq;
> >      uint8_t profile;
> >  
> > +    if (!s->enable)
> > +        return 0;
> > +
> >      if (rpu_size < 5)
> >          goto fail;
> >  
> > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> > index 7825efe2e6..c622a48b94 100644
> > --- a/libavcodec/hevcdec.c
> > +++ b/libavcodec/hevcdec.c
> > @@ -3694,6 +3694,7 @@ static const AVOption options[] = {
> >          AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, PAR },
> >      { "strict-displaywin", "stricly apply default display window size", OFFSET(apply_defdispwin),
> >          AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, PAR },
> > +    { "dolbyvision", "Decode Dolby Vision RPUs", OFFSET(dovi_ctx.enable), AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, PAR },
> >      { NULL },
> >  };
> >  
> > diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> > index 09fe767fb8..f9e1a181fc 100644
> > --- a/libavcodec/libdav1d.c
> > +++ b/libavcodec/libdav1d.c
> > @@ -674,6 +674,7 @@ static const AVOption libdav1d_options[] = {
> >      { "filmgrain", "Apply Film Grain", OFFSET(apply_grain), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, VD | AV_OPT_FLAG_DEPRECATED },
> >      { "oppoint",  "Select an operating point of the scalable bitstream", OFFSET(operating_point), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 31, VD },
> >      { "alllayers", "Output all spatial layers", OFFSET(all_layers), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VD },
> > +    { "dolbyvision", "Decode Dolby Vision RPUs", OFFSET(dovi.enable), AV_OPT_TYPE_BOOL, {.i64 = 1 }, 0, 1, VD },
> >      { NULL }
> >  };
> >  
> 
> Is parsing this stuff very expensive? If not, I don't really see a
> reason to add an option for it (we also do not add options for lots of
> other side data types).
> 
> - Andreas

It's not expensive, no. Well, having DV metadata present technically
alters the interpretation of the video quite substantially (e.g. SDR
streams now displaying as HDR), so users may want *some* way of
"globally" disabling it.

That said, it's probably better to relegate that to `vf_sidedata`.
I'll drop this commit from the series, then.

> 
> _______________________________________________
> 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".
_______________________________________________
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".