diff mbox

[FFmpeg-devel] avcodec/dnxhddec: Do not overwrite colorspace if the container has set it.

Message ID 20171129004946.28211-1-steven@strobe.cc
State New
Headers show

Commit Message

Steven Robertson Nov. 29, 2017, 12:49 a.m. UTC
The existing logic overrides container metadata even in cases where the
container metadata must be trusted (e.g. HDR). The original spec had no
provision for specifying color volume, so many files rely on the
assumption of Rec. 709.

An update to the spec included a 'clv' field for explicitly signaling
that the container should be trusted in an existing bitfield in the
frame header, but the default of 0 from old encoders forces Rec. 709,
which would break any HDR stream. Because there is no place in DNxHR for
specifying a transfer function, DNxHR HDR files must include
container-level color information.

This patch maintains the existing behavior of choosing the 709 over the
601 matrix when container-level information is missing, and allows
container-level information to win if present.

Signed-off-by: Steven Robertson <steven@strobe.cc>
---
 libavcodec/dnxhddec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer Nov. 30, 2017, 1:41 a.m. UTC | #1
On Tue, Nov 28, 2017 at 04:49:46PM -0800, Steven Robertson wrote:
> The existing logic overrides container metadata even in cases where the
> container metadata must be trusted (e.g. HDR). The original spec had no
> provision for specifying color volume, so many files rely on the
> assumption of Rec. 709.
> 
> An update to the spec included a 'clv' field for explicitly signaling
> that the container should be trusted in an existing bitfield in the
> frame header, but the default of 0 from old encoders forces Rec. 709,
> which would break any HDR stream. Because there is no place in DNxHR for
> specifying a transfer function, DNxHR HDR files must include
> container-level color information.
> 
> This patch maintains the existing behavior of choosing the 709 over the
> 601 matrix when container-level information is missing, and allows
> container-level information to win if present.
> 
> Signed-off-by: Steven Robertson <steven@strobe.cc>
> ---
>  libavcodec/dnxhddec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/dnxhddec.c b/libavcodec/dnxhddec.c
> index f46e41a456..6f8c716412 100644
> --- a/libavcodec/dnxhddec.c
> +++ b/libavcodec/dnxhddec.c

will apply

thanks

[...]
diff mbox

Patch

diff --git a/libavcodec/dnxhddec.c b/libavcodec/dnxhddec.c
index f46e41a456..6f8c716412 100644
--- a/libavcodec/dnxhddec.c
+++ b/libavcodec/dnxhddec.c
@@ -93,7 +93,9 @@  static av_cold int dnxhd_decode_init(AVCodecContext *avctx)
 
     ctx->avctx = avctx;
     ctx->cid = -1;
-    avctx->colorspace = AVCOL_SPC_BT709;
+    if (avctx->colorspace == AVCOL_SPC_UNSPECIFIED) {
+      avctx->colorspace = AVCOL_SPC_BT709;
+    }
 
     avctx->coded_width  = FFALIGN(avctx->width,  16);
     avctx->coded_height = FFALIGN(avctx->height, 16);