[FFmpeg-devel] [RFC] Bug in colorspace.c

[Soft Works]

Hi,

in colorspace.c, there are two functions with contradicting behavior
regarding reading/writing the max_luminance value from/to 
AVMasteringDisplayMetadata. The code seems to be unchanged since 
3 years:


1. ff_determine_signal_peak()

    sd = av_frame_get_side_data(in, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
    if (!peak && sd) {
        AVMasteringDisplayMetadata *metadata = (AVMasteringDisplayMetadata *)sd->data;
        if (metadata->has_luminance)
            peak = av_q2d(metadata->max_luminance) / REFERENCE_WHITE;
    }


2. ff_update_hdr_metadata()

    sd = av_frame_get_side_data(in, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
    if (sd) {
        AVMasteringDisplayMetadata *metadata = (AVMasteringDisplayMetadata *)sd->data;
        if (metadata->has_luminance)
            metadata->max_luminance = av_d2q(peak * REFERENCE_WHITE, 10000);
    }


The latter function writes the value as an AVRational with a denominator of
10000, but the former function doesn't multiply the value by 10000. 

So - both cannot be right.

In fact, there seems to be some confusion about the base/range of this value. 
I had found contradicting code somewhere in libva or Intel Media Driver
code a while ago, and I have also seen videos with both "kinds" of values.

This might be a chicken-egg issue, though: maybe those other files I've seen
were created by ffmpeg and subject to this bug. I'm not sure..

As I needed a practical rather than an academical solution, I ended up using a 
check on the value range to determine whether to multiply by 10000 or not
(function 1). I won't submit this as a patch because somebody would shout out
"hack" anyway, while I see it more as an adaption to reality :-)

Anyway, the second function for writing the value should definitely do it in
the right way - unfortunately it can't do without a sane input parameter, 
which in turn can only be ensured with said "hack".

For now, I just want to point at the problem as I don't have a clean solution
to offer, but I'd be interested in others' thoughts.


Best wishes,
softworkz