diff mbox series

[FFmpeg-devel,2/2] libavcodec/dpxenc: change transfer/primaries to BT.709

Message ID 20210519135933.2882-2-val.zapod.vz@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,1/2] avformat/pmpdec: AAC has been supported for a long time | expand


Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate fail Make fate failed
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate warning Make fate failed

Commit Message

Valerii Zapodovnikov May 19, 2021, 1:59 p.m. UTC
 libavcodec/dpxenc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


Valerii Zapodovnikov May 25, 2021, 6:59 p.m. UTC | #1
So, apparently two such patches were done before but not pushed (even
written by Paul, see patch 22747, it will supersede mine). I think at the
very least you must rename "linear colorimetric" BS, because there is no
such thing in the standard and it is not even defined for 2. Also one
little fact: ffplay does not support linear transfer (while mpv does). But
it does not mean you should use linear transfer on things that cannot be
linear (unless you actually select AVCOL_TRC_LINEAR), LOL, because that
will destroy colors in mpv! But whatever: ffplay does not even support some
combinations of BT.601 matrix and says that SMPTE 270M matrix is the same
as BT.601, while actually it is derived from SMPTE 170M/270M primaires and
white point! Hilarious. I also do not really wanna play with your FATE
system, maybe somebody can change metadata hashes of fate for me in Paul's
patch? I can also see that patchwork actions no longer print diff for fate
hashes, which is unfortunate (it did though print them before, like in that
Also FATE has samples without linear transfer, for example, as can be seen
in mediainfo's MediaTrace cyan.dpx uses value 3 for transfer which is new
"logarithmic" transfer (not sure it is the same as in ITU-T H.273's
though). Get rid of it. Now, lena_4x_concat.dpx uses linear transfer and is
oversaturated in mpv, get rid of it too (it is produced by ffmpeg for sure
as it has 2 as color primaries but that is a wrong value).
And lighthouse_rgb48.dpx was created by some very old version of

P.S. Paul's patch is almost perfect. Indeed AVCOL_PRI_SMPTE240M is the same
as AVCOL_PRI_SMPTE170M and thus it is the same value as in the SMPTE 268M
standard: i.e. value 9. No, ITU-R 601-5 system M (525) is NTSC 1953 with
Illuminant C and is actually defined in BT.470, you know that one BT.601
matrix was derived from! LOL. Now with GAMMA22 (which is actually real 2.2
(563/256) only in Adobe RGB 1998) it is System M actually, so should return
8. Also add AVCOL_TRC_GAMMA28 and return System B, G (value 7).
diff mbox series


diff --git a/libavcodec/dpxenc.c b/libavcodec/dpxenc.c
index fa8b7d5ddc..db5ed4e328 100644
--- a/libavcodec/dpxenc.c
+++ b/libavcodec/dpxenc.c
@@ -219,8 +219,8 @@  static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
     write32(buf + 772, avctx->width);
     write32(buf + 776, avctx->height);
     buf[800] = s->descriptor;
-    buf[801] = 2; /* linear transfer */
-    buf[802] = 2; /* linear colorimetric */
+    buf[801] = 6; /* BT.709 transfer */
+    buf[802] = 6; /* BT.709 primaries */
     buf[803] = s->bits_per_component;
     write16(buf + 804, (s->bits_per_component == 10 || s->bits_per_component == 12) ?
                        1 : 0); /* packing method */