[FFmpeg-devel] [PATCH 3/4] avformat/mov: add support for reading Mastering Display Metadata Box

[Michael Niedermayer]

On Sat, May 27, 2017 at 11:58:05AM -0300, James Almer wrote:
> On 5/27/2017 11:05 AM, Michael Niedermayer wrote:
> > On Sat, May 27, 2017 at 01:54:18AM -0300, James Almer wrote:
> >> On 5/26/2017 8:05 PM, Michael Niedermayer wrote:
> >>> On Wed, May 17, 2017 at 09:49:40PM -0300, James Almer wrote:
> >>>> As defined in "VP Codec ISO Media File Format Binding v1.0"
> >>>> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
> >>>>
> >>>> Partially based on Matroska decoder code.
> >>>>
> >>>> Signed-off-by: James Almer <jamrial at gmail.com>
> >>>> ---
> >>>>  libavformat/isom.h |  2 ++
> >>>>  libavformat/mov.c  | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  2 files changed, 67 insertions(+)
> >>>>
> >>>> diff --git a/libavformat/isom.h b/libavformat/isom.h
> >>>> index d9956cf63a..426f732247 100644
> >>>> --- a/libavformat/isom.h
> >>>> +++ b/libavformat/isom.h
> >>>> @@ -27,6 +27,7 @@
> >>>>  #include <stddef.h>
> >>>>  #include <stdint.h>
> >>>>  
> >>>> +#include "libavutil/mastering_display_metadata.h"
> >>>>  #include "libavutil/spherical.h"
> >>>>  #include "libavutil/stereo3d.h"
> >>>>  
> >>>> @@ -194,6 +195,7 @@ typedef struct MOVStreamContext {
> >>>>      AVStereo3D *stereo3d;
> >>>>      AVSphericalMapping *spherical;
> >>>>      size_t spherical_size;
> >>>> +    AVMasteringDisplayMetadata *mastering;
> >>>>  
> >>>>      uint32_t format;
> >>>>  
> >>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >>>> index afef53b79a..0b5fd849f3 100644
> >>>> --- a/libavformat/mov.c
> >>>> +++ b/libavformat/mov.c
> >>>> @@ -4612,6 +4612,60 @@ static int mov_read_tmcd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>>>      return 0;
> >>>>  }
> >>>>  
> >>>> +static int mov_read_smdm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>>> +{
> >>>> +    MOVStreamContext *sc;
> >>>> +    const int chroma_den = 50000;
> >>>> +    const int luma_den = 10000;
> >>>> +    int version;
> >>>> +
> >>>> +    if (c->fc->nb_streams < 1)
> >>>> +        return AVERROR_INVALIDDATA;
> >>>> +
> >>>> +    sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
> >>>> +
> >>>> +    if (atom.size < 5) {
> >>>> +        av_log(c->fc, AV_LOG_ERROR, "Empty Mastering Display Metadata box\n");
> >>>> +        return AVERROR_INVALIDDATA;
> >>>> +    }
> >>>> +
> >>>> +    version = avio_r8(pb);
> >>>> +    if (version) {
> >>>> +        av_log(c->fc, AV_LOG_WARNING, "Unsupported Mastering Display Metadata box version %d\n", version);
> >>>> +        return 0;
> >>>> +    }
> >>>> +    avio_skip(pb, 3); /* flags */
> >>>> +
> >>>> +    sc->mastering = av_mastering_display_metadata_alloc();
> >>>> +    if (!sc->mastering)
> >>>> +        return AVERROR(ENOMEM);
> >>>> +
> >>>
> >>>> +    sc->mastering->display_primaries[0][0] =
> >>>> +        av_make_q(lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den), chroma_den);
> >>>
> >>> this is not optimal, precission wise
> >>> av_d2q() should produce closer rationals
> >>> alternativly av_reduce() can be used directly
> >>>
> >>> but iam not sure why a fixed chroma_den and luma_den is fixed
> >>> maybe iam missing something
> >>
> >> Does
> >>
> >> for (i = 0; i < 3; i++)
> >>   for (j = 0; j < 2; j++)
> >>     av_reduce(&sc->mastering->display_primaries[i][j].num,
> >>               &sc->mastering->display_primaries[i][j].den,
> >>               lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den),
> >>               chroma_den, chroma_den);
> > 
> > Why do you use
> > lrint(((double)avio_rb16(pb) / (1 << 16)) * chroma_den
> > and
> > chroma_den
> > 
> > instead of
> > avio_rb16(pb)
> > and
> > 1 << 16
> > 
> > ?
> > 
> 
> To follow the origin spec the vp9 in mp4 spec and the
> AVMasteringDisplayMetadata API quote and are based on.
> 
> ---------------
> display_primaries_x[ c ] and display_primaries_y[ c ] specify the
> normalized x and y chromaticity coordinates, respectively, of the colour
> primary component c of the mastering display in increments of 0.00002,
> according to the CIE 1931 definition of x and y as specified in ISO
> 11664-1 (see also ISO 11664-3 and CIE 15). For describing mastering
> displays that use red, green and blue colour primaries, it is suggested
> that index value c equal to 0 should correspond to the green primary, c
> equal to 1 should correspond to the blue primary and c equal to 2 should
> correspond to the red colour primary (see also Annex E and Table E.3).
> The values of display_primaries_x[ c ] and display_primaries_y[ c ]
> shall be in the range of 0 to 50 000, inclusive.
> 
> white_point_x and white_point_y specify the normalized x and y
> chromaticity coordinates, respectively, of the white point of the
> mastering display in normalized increments of 0.00002, according to the
> CIE 1931 definition of x and y as specified in ISO 11664-1 (see also ISO
> 11664-3 and CIE 15). The values of white_point_x and white_point_y shall
> be in the range of 0 to 50 000.
> 
> max_display_mastering_luminance and min_display_mastering_luminance
> specify the nominal maximum and minimum display luminance, respectively,
> of the mastering display in units of 0.0001 candelas per square metre.

[...]

> Notice how all values break the increments of 0.00002 (50000
> denominator) and 0.0001 (10000 denominator) constrains when i just take
> the 0.16, 24.8 and 18.14 fixed point values stored in mp4 and dump them
> into AVMasteringDisplayMetadata as is, like you suggest.

ok, i mistakely assumed the specifications where consistent or at
least one a superset of others.

Please ignore my comment about the patch


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170527/e3089d96/attachment.sig>