[FFmpeg-devel] avutil/pixfmt: improve definition of AVColorRange

As it was brought up that the current documentation leaves things
as specific to YCbCr only, ICtCp and RGB are now mentioned.
Additionally, the specifications on which these definitions of
narrow and full range are defined are mentioned.

This way, the documentation of AVColorRange should now match how
most people seem to read interpret it at this point, and thus
flagging RGB AVFrames as full range is valid not only according to
common sense, but also the enum definition.
---
 libavutil/pixfmt.h | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

On 9/18/2020 6:05 PM, Jan Ekström wrote:
> As it was brought up that the current documentation leaves things
> as specific to YCbCr only, ICtCp and RGB are now mentioned.
> Additionally, the specifications on which these definitions of
> narrow and full range are defined are mentioned.
> 
> This way, the documentation of AVColorRange should now match how
> most people seem to read interpret it at this point, and thus
> flagging RGB AVFrames as full range is valid not only according to
> common sense, but also the enum definition.
> ---
>  libavutil/pixfmt.h | 40 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 3 deletions(-)
> 
> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> index a46acf3c5e..68051bf4b9 100644
> --- a/libavutil/pixfmt.h
> +++ b/libavutil/pixfmt.h
> @@ -530,12 +530,46 @@ enum AVColorSpace {
>  };
>  
>  /**
> - * MPEG vs JPEG YUV range.
> + * Visual content value range.
> + *
> + * These values are based on definitions that can be found in multiple
> + * specifications, such as ITU-T BT.709 (3.4 - Quantization of RGB, luminance
> + * and colour-difference signals), ITU-T BT.2020 (Table 5 - Digital
> + * Representation) as well as ITU-T BT.2100 (Table 9 - Digital 10- and 12-bit
> + * integer representation). At the time of writing, the BT.2100 one is
> + * recommended, as it also defines the full range representation.
> + *
> + * Common definitions:
> + *   - For luminance planes such as Y in YCbCr and I in ICtCp, 'E' is the
> + *     original value in range of 0.0 to 1.0.
> + *   - For chrominance planes such as Cb,Cr and Ct,Cp, 'E' is the original
> + *     value in range of -0.5 to 0.5.
> + *   - 'n' is the output bit depth.
> + *   - For additional definitions such as rounding and clipping to valid n
> + *     bit unsigned integer range, please refer to BT.2100 (Table 9).
>   */
>  enum AVColorRange {
>      AVCOL_RANGE_UNSPECIFIED = 0,
> -    AVCOL_RANGE_MPEG        = 1, ///< the normal 219*2^(n-8) "MPEG" YUV ranges
> -    AVCOL_RANGE_JPEG        = 2, ///< the normal     2^n-1   "JPEG" YUV ranges
> +    AVCOL_RANGE_MPEG        = 1, ///< Narrow or limited range content.
> +                                 ///
> +                                 ///  For RGB and luminance planes:
> +                                 ///  (219 * E + 16) * 2^(n-8)
> +                                 ///  F.ex. the range of 16-235 for 8 bits
> +                                 ///
> +                                 ///  For chrominance planes:
> +                                 ///  (224 * E + 128) * 2^(n-8)
> +                                 ///  F.ex. the range of 16-240 for 8 bits

Is this parsed right by doxygen? Because if not you may want to use /**
*/ for multi-line descriptions, and placed above each value instead of
next to them.

> +
> +    AVCOL_RANGE_JPEG        = 2, ///< Full range content
> +                                 ///
> +                                 ///  For RGB and luminance planes:
> +                                 ///  (2^n - 1) * E
> +                                 ///  F.ex. the range of 0-255 for 8 bits
> +                                 ///
> +                                 ///  For chrominance planes:
> +                                 ///  (2^n - 1) * E + 2^(n - 1)
> +                                 ///  F.ex. the range of 1-255 for 8 bits
> +
>      AVCOL_RANGE_NB               ///< Not part of ABI
>  };
>  
>

On Sat, Sep 19, 2020 at 12:47 AM James Almer <jamrial@gmail.com> wrote:
>
> On 9/18/2020 6:05 PM, Jan Ekström wrote:
> > As it was brought up that the current documentation leaves things
> > as specific to YCbCr only, ICtCp and RGB are now mentioned.
> > Additionally, the specifications on which these definitions of
> > narrow and full range are defined are mentioned.
> >
> > This way, the documentation of AVColorRange should now match how
> > most people seem to read interpret it at this point, and thus
> > flagging RGB AVFrames as full range is valid not only according to
> > common sense, but also the enum definition.
> > ---
> >  libavutil/pixfmt.h | 40 +++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 37 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> > index a46acf3c5e..68051bf4b9 100644
> > --- a/libavutil/pixfmt.h
> > +++ b/libavutil/pixfmt.h
> > @@ -530,12 +530,46 @@ enum AVColorSpace {
> >  };
> >
> >  /**
> > - * MPEG vs JPEG YUV range.
> > + * Visual content value range.
> > + *
> > + * These values are based on definitions that can be found in multiple
> > + * specifications, such as ITU-T BT.709 (3.4 - Quantization of RGB, luminance
> > + * and colour-difference signals), ITU-T BT.2020 (Table 5 - Digital
> > + * Representation) as well as ITU-T BT.2100 (Table 9 - Digital 10- and 12-bit
> > + * integer representation). At the time of writing, the BT.2100 one is
> > + * recommended, as it also defines the full range representation.
> > + *
> > + * Common definitions:
> > + *   - For luminance planes such as Y in YCbCr and I in ICtCp, 'E' is the
> > + *     original value in range of 0.0 to 1.0.
> > + *   - For chrominance planes such as Cb,Cr and Ct,Cp, 'E' is the original
> > + *     value in range of -0.5 to 0.5.
> > + *   - 'n' is the output bit depth.
> > + *   - For additional definitions such as rounding and clipping to valid n
> > + *     bit unsigned integer range, please refer to BT.2100 (Table 9).
> >   */
> >  enum AVColorRange {
> >      AVCOL_RANGE_UNSPECIFIED = 0,
> > -    AVCOL_RANGE_MPEG        = 1, ///< the normal 219*2^(n-8) "MPEG" YUV ranges
> > -    AVCOL_RANGE_JPEG        = 2, ///< the normal     2^n-1   "JPEG" YUV ranges
> > +    AVCOL_RANGE_MPEG        = 1, ///< Narrow or limited range content.
> > +                                 ///
> > +                                 ///  For RGB and luminance planes:
> > +                                 ///  (219 * E + 16) * 2^(n-8)
> > +                                 ///  F.ex. the range of 16-235 for 8 bits
> > +                                 ///
> > +                                 ///  For chrominance planes:
> > +                                 ///  (224 * E + 128) * 2^(n-8)
> > +                                 ///  F.ex. the range of 16-240 for 8 bits
>
> Is this parsed right by doxygen? Because if not you may want to use /**
> */ for multi-line descriptions, and placed above each value instead of
> next to them.
>

The parser in my editor only finds the first line, which in an editor is fine.

Finally got doxygen to generate it, and it gets the whole explanation,
but it is basically in a code block after the main line. Which is OK,
but could be better.

Will switch to the multi-line thing and regenerate & check.

(Also apparently `make doc/doxy/html` is broken in separate build
roots since it goes to src, but then refers to the Doxyfile as
src/doc/Doxyfile. Oh well, at least it gave out the command required
for generation when called with V=1).

Jan