diff mbox series

[FFmpeg-devel,v2,1/2] libavutil/dovi_meta: Add nlq_pivots to AVDOVIDataMapping

Message ID 20220617193436.78-2-tcChlisop0@gmail.com
State New
Headers show
Series DOVI: Add NLQ pivots to AVDOVIDataMapping | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

quietvoid June 17, 2022, 7:34 p.m. UTC
The NLQ pivots are not documented but should be present
in the header for profile 7 RPU format.
It has been verified using Dolby's verification toolkit.

Also implemented the parsing in libavcodec/dovi_rpu.c.
And added the info to ffprobe and showinfo.

Signed-off-by: quietvoid <tcChlisop0@gmail.com>
---
 fftools/ffprobe.c         | 4 ++++
 libavcodec/dovi_rpu.c     | 7 +++++++
 libavfilter/vf_showinfo.c | 8 ++++++++
 libavutil/dovi_meta.h     | 1 +
 4 files changed, 20 insertions(+)

Comments

Andreas Rheinhardt June 17, 2022, 8:19 p.m. UTC | #1
quietvoid:
> The NLQ pivots are not documented but should be present
> in the header for profile 7 RPU format.
> It has been verified using Dolby's verification toolkit.
> 
> Also implemented the parsing in libavcodec/dovi_rpu.c.
> And added the info to ffprobe and showinfo.
> 
> Signed-off-by: quietvoid <tcChlisop0@gmail.com>
> ---
>  fftools/ffprobe.c         | 4 ++++
>  libavcodec/dovi_rpu.c     | 7 +++++++
>  libavfilter/vf_showinfo.c | 8 ++++++++
>  libavutil/dovi_meta.h     | 1 +
>  4 files changed, 20 insertions(+)
> 
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index 4e2fdbaec8..bc4f7ec0a1 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -2001,6 +2001,10 @@ static void print_dovi_metadata(WriterContext *w, const AVDOVIMetadata *dovi)
>              break;
>          }
>  
> +        if (mapping->nlq_method_idc != AV_DOVI_NLQ_NONE) {
> +            print_list_fmt("nlq_pivots", "%"PRIu16, 2, mapping->nlq_pivots[idx]);
> +        }
> +
>          print_int("num_x_partitions",          mapping->num_x_partitions);
>          print_int("num_y_partitions",          mapping->num_y_partitions);
>  
> diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
> index a87562c8a3..17837eb845 100644
> --- a/libavcodec/dovi_rpu.c
> +++ b/libavcodec/dovi_rpu.c
> @@ -315,7 +315,14 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
>          }
>  
>          if (use_nlq) {
> +            int nlq_pivot = 0;
>              vdr->mapping.nlq_method_idc = get_bits(gb, 3);
> +
> +            for (int i = 0; i < 2; i++) {
> +                nlq_pivot += get_bits(gb, hdr->bl_bit_depth);
> +                vdr->mapping.nlq_pivots[i] = av_clip_uint16(nlq_pivot);
> +            }
> +
>              /**
>               * The patent mentions another legal value, NLQ_MU_LAW, but it's
>               * not documented anywhere how to parse or apply that type of NLQ.
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index 6efcafce28..f8c04fb2fa 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -535,6 +535,14 @@ static void dump_dovi_metadata(AVFilterContext *ctx, const AVFrameSideData *sd)
>      av_log(ctx, AV_LOG_INFO, "mapping_color_space=%"PRIu8"; ", mapping->mapping_color_space);
>      av_log(ctx, AV_LOG_INFO, "mapping_chroma_format_idc=%"PRIu8"; ", mapping->mapping_chroma_format_idc);
>      av_log(ctx, AV_LOG_INFO, "nlq_method_idc=%d; ", (int) mapping->nlq_method_idc);
> +
> +    if (mapping->nlq_method_idc != AV_DOVI_NLQ_NONE) {
> +        av_log(ctx, AV_LOG_INFO, "nlq_pivots={ ");
> +        for (int i = 0; i < 2; i++)
> +            av_log(ctx, AV_LOG_INFO, "%"PRIu16" ", mapping->nlq_pivots[i]);
> +        av_log(ctx, AV_LOG_INFO, "}; ");

Don't use four calls to av_log for something that is so simple. Besides
having higher overhead and binary size it also adds the risk that these
av_logs are disturbed by another av_log in the middle, breaking the
output (at least with the default log callback).
(The same btw applies to other av_logs in this very function. Is it
actually intended for this to be one big line?)

> +    }
> +
>      av_log(ctx, AV_LOG_INFO, "num_x_partitions=%"PRIu32"; ", mapping->num_x_partitions);
>      av_log(ctx, AV_LOG_INFO, "num_y_partitions=%"PRIu32"\n", mapping->num_y_partitions);
>  
> diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
> index 3d11e02bff..6afc7b055a 100644
> --- a/libavutil/dovi_meta.h
> +++ b/libavutil/dovi_meta.h
> @@ -147,6 +147,7 @@ typedef struct AVDOVIDataMapping {
>      uint32_t num_x_partitions;
>      uint32_t num_y_partitions;
>      AVDOVINLQParams nlq[3]; /* per component */
> +    uint16_t nlq_pivots[2]; /* nlq_pred_pivot_value */
>  } AVDOVIDataMapping;
>  
>  /**
quietvoid June 17, 2022, 8:37 p.m. UTC | #2
On Fri, Jun 17, 2022 at 4:20 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> quietvoid:
> > The NLQ pivots are not documented but should be present
> > in the header for profile 7 RPU format.
> > It has been verified using Dolby's verification toolkit.
> >
> > Also implemented the parsing in libavcodec/dovi_rpu.c.
> > And added the info to ffprobe and showinfo.
> >
> > Signed-off-by: quietvoid <tcChlisop0@gmail.com>
> > ---
> >  fftools/ffprobe.c         | 4 ++++
> >  libavcodec/dovi_rpu.c     | 7 +++++++
> >  libavfilter/vf_showinfo.c | 8 ++++++++
> >  libavutil/dovi_meta.h     | 1 +
> >  4 files changed, 20 insertions(+)
> >
> > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> > index 4e2fdbaec8..bc4f7ec0a1 100644
> > --- a/fftools/ffprobe.c
> > +++ b/fftools/ffprobe.c
> > @@ -2001,6 +2001,10 @@ static void print_dovi_metadata(WriterContext *w, const AVDOVIMetadata *dovi)
> >              break;
> >          }
> >
> > +        if (mapping->nlq_method_idc != AV_DOVI_NLQ_NONE) {
> > +            print_list_fmt("nlq_pivots", "%"PRIu16, 2, mapping->nlq_pivots[idx]);
> > +        }
> > +
> >          print_int("num_x_partitions",          mapping->num_x_partitions);
> >          print_int("num_y_partitions",          mapping->num_y_partitions);
> >
> > diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
> > index a87562c8a3..17837eb845 100644
> > --- a/libavcodec/dovi_rpu.c
> > +++ b/libavcodec/dovi_rpu.c
> > @@ -315,7 +315,14 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
> >          }
> >
> >          if (use_nlq) {
> > +            int nlq_pivot = 0;
> >              vdr->mapping.nlq_method_idc = get_bits(gb, 3);
> > +
> > +            for (int i = 0; i < 2; i++) {
> > +                nlq_pivot += get_bits(gb, hdr->bl_bit_depth);
> > +                vdr->mapping.nlq_pivots[i] = av_clip_uint16(nlq_pivot);
> > +            }
> > +
> >              /**
> >               * The patent mentions another legal value, NLQ_MU_LAW, but it's
> >               * not documented anywhere how to parse or apply that type of NLQ.
> > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> > index 6efcafce28..f8c04fb2fa 100644
> > --- a/libavfilter/vf_showinfo.c
> > +++ b/libavfilter/vf_showinfo.c
> > @@ -535,6 +535,14 @@ static void dump_dovi_metadata(AVFilterContext *ctx, const AVFrameSideData *sd)
> >      av_log(ctx, AV_LOG_INFO, "mapping_color_space=%"PRIu8"; ", mapping->mapping_color_space);
> >      av_log(ctx, AV_LOG_INFO, "mapping_chroma_format_idc=%"PRIu8"; ", mapping->mapping_chroma_format_idc);
> >      av_log(ctx, AV_LOG_INFO, "nlq_method_idc=%d; ", (int) mapping->nlq_method_idc);
> > +
> > +    if (mapping->nlq_method_idc != AV_DOVI_NLQ_NONE) {
> > +        av_log(ctx, AV_LOG_INFO, "nlq_pivots={ ");
> > +        for (int i = 0; i < 2; i++)
> > +            av_log(ctx, AV_LOG_INFO, "%"PRIu16" ", mapping->nlq_pivots[i]);
> > +        av_log(ctx, AV_LOG_INFO, "}; ");
>
> Don't use four calls to av_log for something that is so simple. Besides
> having higher overhead and binary size it also adds the risk that these
> av_logs are disturbed by another av_log in the middle, breaking the
> output (at least with the default log callback).

Fixed in v3.
I've made it into a single log call:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297733.html

> (The same btw applies to other av_logs in this very function. Is it
> actually intended for this to be one big line?)

I'm not exactly sure how the showinfo filters usually look like.
I'm only basing this on the existing logging for this part.

>
> > +    }
> > +
> >      av_log(ctx, AV_LOG_INFO, "num_x_partitions=%"PRIu32"; ", mapping->num_x_partitions);
> >      av_log(ctx, AV_LOG_INFO, "num_y_partitions=%"PRIu32"\n", mapping->num_y_partitions);
> >
> > diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
> > index 3d11e02bff..6afc7b055a 100644
> > --- a/libavutil/dovi_meta.h
> > +++ b/libavutil/dovi_meta.h
> > @@ -147,6 +147,7 @@ typedef struct AVDOVIDataMapping {
> >      uint32_t num_x_partitions;
> >      uint32_t num_y_partitions;
> >      AVDOVINLQParams nlq[3]; /* per component */
> > +    uint16_t nlq_pivots[2]; /* nlq_pred_pivot_value */
> >  } AVDOVIDataMapping;
> >
> >  /**
>
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 4e2fdbaec8..bc4f7ec0a1 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2001,6 +2001,10 @@  static void print_dovi_metadata(WriterContext *w, const AVDOVIMetadata *dovi)
             break;
         }
 
+        if (mapping->nlq_method_idc != AV_DOVI_NLQ_NONE) {
+            print_list_fmt("nlq_pivots", "%"PRIu16, 2, mapping->nlq_pivots[idx]);
+        }
+
         print_int("num_x_partitions",          mapping->num_x_partitions);
         print_int("num_y_partitions",          mapping->num_y_partitions);
 
diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c
index a87562c8a3..17837eb845 100644
--- a/libavcodec/dovi_rpu.c
+++ b/libavcodec/dovi_rpu.c
@@ -315,7 +315,14 @@  int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size)
         }
 
         if (use_nlq) {
+            int nlq_pivot = 0;
             vdr->mapping.nlq_method_idc = get_bits(gb, 3);
+
+            for (int i = 0; i < 2; i++) {
+                nlq_pivot += get_bits(gb, hdr->bl_bit_depth);
+                vdr->mapping.nlq_pivots[i] = av_clip_uint16(nlq_pivot);
+            }
+
             /**
              * The patent mentions another legal value, NLQ_MU_LAW, but it's
              * not documented anywhere how to parse or apply that type of NLQ.
diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
index 6efcafce28..f8c04fb2fa 100644
--- a/libavfilter/vf_showinfo.c
+++ b/libavfilter/vf_showinfo.c
@@ -535,6 +535,14 @@  static void dump_dovi_metadata(AVFilterContext *ctx, const AVFrameSideData *sd)
     av_log(ctx, AV_LOG_INFO, "mapping_color_space=%"PRIu8"; ", mapping->mapping_color_space);
     av_log(ctx, AV_LOG_INFO, "mapping_chroma_format_idc=%"PRIu8"; ", mapping->mapping_chroma_format_idc);
     av_log(ctx, AV_LOG_INFO, "nlq_method_idc=%d; ", (int) mapping->nlq_method_idc);
+
+    if (mapping->nlq_method_idc != AV_DOVI_NLQ_NONE) {
+        av_log(ctx, AV_LOG_INFO, "nlq_pivots={ ");
+        for (int i = 0; i < 2; i++)
+            av_log(ctx, AV_LOG_INFO, "%"PRIu16" ", mapping->nlq_pivots[i]);
+        av_log(ctx, AV_LOG_INFO, "}; ");
+    }
+
     av_log(ctx, AV_LOG_INFO, "num_x_partitions=%"PRIu32"; ", mapping->num_x_partitions);
     av_log(ctx, AV_LOG_INFO, "num_y_partitions=%"PRIu32"\n", mapping->num_y_partitions);
 
diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
index 3d11e02bff..6afc7b055a 100644
--- a/libavutil/dovi_meta.h
+++ b/libavutil/dovi_meta.h
@@ -147,6 +147,7 @@  typedef struct AVDOVIDataMapping {
     uint32_t num_x_partitions;
     uint32_t num_y_partitions;
     AVDOVINLQParams nlq[3]; /* per component */
+    uint16_t nlq_pivots[2]; /* nlq_pred_pivot_value */
 } AVDOVIDataMapping;
 
 /**