| Message ID | 20200124141430.8675-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 1/24/2020 11:14 AM, Paul B Mahol wrote: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavcodec/dnxhdenc.c | 11 ++++++++++- > libavcodec/dnxhdenc.h | 1 + > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c > index 2feb8baf21..03835fa794 100644 > --- a/libavcodec/dnxhdenc.c > +++ b/libavcodec/dnxhdenc.c > @@ -406,6 +406,15 @@ static av_cold int dnxhd_encode_init(AVCodecContext *avctx) > } > > ctx->is_444 = ctx->profile == FF_PROFILE_DNXHR_444; > + > + switch (avctx->colorspace) { > + case AVCOL_SPC_UNSPECIFIED: Isn't 3 meant to be unspecified? If not, what is it for? > + case AVCOL_SPC_BT709: ctx->colorspace = 0; break; > + case AVCOL_SPC_BT2020_NCL: ctx->colorspace = 1; break; > + case AVCOL_SPC_BT2020_CL: ctx->colorspace = 2; break; > + default: ctx->colorspace = 3; break; > + } > + > avctx->profile = ctx->profile; > ctx->cid = ff_dnxhd_find_cid(avctx, ctx->bit_depth); > if (!ctx->cid) { > @@ -576,7 +585,7 @@ static int dnxhd_write_header(AVCodecContext *avctx, uint8_t *buf) > buf[0x21] = ctx->bit_depth == 10 ? 0x58 : 0x38; > buf[0x22] = 0x88 + (ctx->interlaced << 2); > AV_WB32(buf + 0x28, ctx->cid); // CID > - buf[0x2c] = (!ctx->interlaced << 7) | (ctx->is_444 << 6) | (avctx->pix_fmt == AV_PIX_FMT_YUV444P10); > + buf[0x2c] = (!ctx->interlaced << 7) | (ctx->is_444 << 6) | (ctx->colorspace << 1) | (avctx->pix_fmt == AV_PIX_FMT_YUV444P10); > > buf[0x5f] = 0x01; // UDL > > diff --git a/libavcodec/dnxhdenc.h b/libavcodec/dnxhdenc.h > index 7b0d862e28..4d5f457e33 100644 > --- a/libavcodec/dnxhdenc.h > +++ b/libavcodec/dnxhdenc.h > @@ -50,6 +50,7 @@ typedef struct DNXHDEncContext { > int profile; > int bit_depth; > int is_444; > + int colorspace; > const CIDEntry *cid_table; > uint8_t *msip; ///< Macroblock Scan Indexes Payload > uint32_t *slice_size; >
On 1/24/20, James Almer <jamrial@gmail.com> wrote: > On 1/24/2020 11:14 AM, Paul B Mahol wrote: >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavcodec/dnxhdenc.c | 11 ++++++++++- >> libavcodec/dnxhdenc.h | 1 + >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c >> index 2feb8baf21..03835fa794 100644 >> --- a/libavcodec/dnxhdenc.c >> +++ b/libavcodec/dnxhdenc.c >> @@ -406,6 +406,15 @@ static av_cold int dnxhd_encode_init(AVCodecContext >> *avctx) >> } >> >> ctx->is_444 = ctx->profile == FF_PROFILE_DNXHR_444; >> + >> + switch (avctx->colorspace) { >> + case AVCOL_SPC_UNSPECIFIED: > > Isn't 3 meant to be unspecified? If not, what is it for? This is to keep old behavior same. Where encoder would store unspecified as bt709. Mainly to not need to modify fate tests. > >> + case AVCOL_SPC_BT709: ctx->colorspace = 0; break; >> + case AVCOL_SPC_BT2020_NCL: ctx->colorspace = 1; break; >> + case AVCOL_SPC_BT2020_CL: ctx->colorspace = 2; break; >> + default: ctx->colorspace = 3; break; >> + } >> + >> avctx->profile = ctx->profile; >> ctx->cid = ff_dnxhd_find_cid(avctx, ctx->bit_depth); >> if (!ctx->cid) { >> @@ -576,7 +585,7 @@ static int dnxhd_write_header(AVCodecContext *avctx, >> uint8_t *buf) >> buf[0x21] = ctx->bit_depth == 10 ? 0x58 : 0x38; >> buf[0x22] = 0x88 + (ctx->interlaced << 2); >> AV_WB32(buf + 0x28, ctx->cid); // CID >> - buf[0x2c] = (!ctx->interlaced << 7) | (ctx->is_444 << 6) | >> (avctx->pix_fmt == AV_PIX_FMT_YUV444P10); >> + buf[0x2c] = (!ctx->interlaced << 7) | (ctx->is_444 << 6) | >> (ctx->colorspace << 1) | (avctx->pix_fmt == AV_PIX_FMT_YUV444P10); >> >> buf[0x5f] = 0x01; // UDL >> >> diff --git a/libavcodec/dnxhdenc.h b/libavcodec/dnxhdenc.h >> index 7b0d862e28..4d5f457e33 100644 >> --- a/libavcodec/dnxhdenc.h >> +++ b/libavcodec/dnxhdenc.h >> @@ -50,6 +50,7 @@ typedef struct DNXHDEncContext { >> int profile; >> int bit_depth; >> int is_444; >> + int colorspace; >> const CIDEntry *cid_table; >> uint8_t *msip; ///< Macroblock Scan Indexes Payload >> uint32_t *slice_size; >> > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 1/24/2020 12:59 PM, Paul B Mahol wrote: > On 1/24/20, James Almer <jamrial@gmail.com> wrote: >> On 1/24/2020 11:14 AM, Paul B Mahol wrote: >>> Signed-off-by: Paul B Mahol <onemda@gmail.com> >>> --- >>> libavcodec/dnxhdenc.c | 11 ++++++++++- >>> libavcodec/dnxhdenc.h | 1 + >>> 2 files changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c >>> index 2feb8baf21..03835fa794 100644 >>> --- a/libavcodec/dnxhdenc.c >>> +++ b/libavcodec/dnxhdenc.c >>> @@ -406,6 +406,15 @@ static av_cold int dnxhd_encode_init(AVCodecContext >>> *avctx) >>> } >>> >>> ctx->is_444 = ctx->profile == FF_PROFILE_DNXHR_444; >>> + >>> + switch (avctx->colorspace) { >>> + case AVCOL_SPC_UNSPECIFIED: >> >> Isn't 3 meant to be unspecified? If not, what is it for? > > This is to keep old behavior same. Where encoder would store > unspecified as bt709. > Mainly to not need to modify fate tests. The proper thing to do is adapt the tests to the new correct behavior, not keeping an encoder doing things technically wrong just to not update the tests. > >> >>> + case AVCOL_SPC_BT709: ctx->colorspace = 0; break; >>> + case AVCOL_SPC_BT2020_NCL: ctx->colorspace = 1; break; >>> + case AVCOL_SPC_BT2020_CL: ctx->colorspace = 2; break; >>> + default: ctx->colorspace = 3; break; >>> + } >>> + >>> avctx->profile = ctx->profile; >>> ctx->cid = ff_dnxhd_find_cid(avctx, ctx->bit_depth); >>> if (!ctx->cid) { >>> @@ -576,7 +585,7 @@ static int dnxhd_write_header(AVCodecContext *avctx, >>> uint8_t *buf) >>> buf[0x21] = ctx->bit_depth == 10 ? 0x58 : 0x38; >>> buf[0x22] = 0x88 + (ctx->interlaced << 2); >>> AV_WB32(buf + 0x28, ctx->cid); // CID >>> - buf[0x2c] = (!ctx->interlaced << 7) | (ctx->is_444 << 6) | >>> (avctx->pix_fmt == AV_PIX_FMT_YUV444P10); >>> + buf[0x2c] = (!ctx->interlaced << 7) | (ctx->is_444 << 6) | >>> (ctx->colorspace << 1) | (avctx->pix_fmt == AV_PIX_FMT_YUV444P10); >>> >>> buf[0x5f] = 0x01; // UDL >>> >>> diff --git a/libavcodec/dnxhdenc.h b/libavcodec/dnxhdenc.h >>> index 7b0d862e28..4d5f457e33 100644 >>> --- a/libavcodec/dnxhdenc.h >>> +++ b/libavcodec/dnxhdenc.h >>> @@ -50,6 +50,7 @@ typedef struct DNXHDEncContext { >>> int profile; >>> int bit_depth; >>> int is_444; >>> + int colorspace; >>> const CIDEntry *cid_table; >>> uint8_t *msip; ///< Macroblock Scan Indexes Payload >>> uint32_t *slice_size; >>> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Am Fr., 24. Jan. 2020 um 17:05 Uhr schrieb James Almer <jamrial@gmail.com>: > > On 1/24/2020 12:59 PM, Paul B Mahol wrote: > > On 1/24/20, James Almer <jamrial@gmail.com> wrote: > >> On 1/24/2020 11:14 AM, Paul B Mahol wrote: > >>> Signed-off-by: Paul B Mahol <onemda@gmail.com> > >>> --- > >>> libavcodec/dnxhdenc.c | 11 ++++++++++- > >>> libavcodec/dnxhdenc.h | 1 + > >>> 2 files changed, 11 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c > >>> index 2feb8baf21..03835fa794 100644 > >>> --- a/libavcodec/dnxhdenc.c > >>> +++ b/libavcodec/dnxhdenc.c > >>> @@ -406,6 +406,15 @@ static av_cold int dnxhd_encode_init(AVCodecContext > >>> *avctx) > >>> } > >>> > >>> ctx->is_444 = ctx->profile == FF_PROFILE_DNXHR_444; > >>> + > >>> + switch (avctx->colorspace) { > >>> + case AVCOL_SPC_UNSPECIFIED: > >> > >> Isn't 3 meant to be unspecified? If not, what is it for? > > > > This is to keep old behavior same. Where encoder would store > > unspecified as bt709. > > Mainly to not need to modify fate tests. > > The proper thing to do is adapt the tests to the new correct behavior, > not keeping an encoder doing things technically wrong just to not update > the tests. But this could be an independent patch which would be better. Carl Eugen
diff --git a/libavcodec/dnxhdenc.c b/libavcodec/dnxhdenc.c index 2feb8baf21..03835fa794 100644 --- a/libavcodec/dnxhdenc.c +++ b/libavcodec/dnxhdenc.c @@ -406,6 +406,15 @@ static av_cold int dnxhd_encode_init(AVCodecContext *avctx) } ctx->is_444 = ctx->profile == FF_PROFILE_DNXHR_444; + + switch (avctx->colorspace) { + case AVCOL_SPC_UNSPECIFIED: + case AVCOL_SPC_BT709: ctx->colorspace = 0; break; + case AVCOL_SPC_BT2020_NCL: ctx->colorspace = 1; break; + case AVCOL_SPC_BT2020_CL: ctx->colorspace = 2; break; + default: ctx->colorspace = 3; break; + } + avctx->profile = ctx->profile; ctx->cid = ff_dnxhd_find_cid(avctx, ctx->bit_depth); if (!ctx->cid) { @@ -576,7 +585,7 @@ static int dnxhd_write_header(AVCodecContext *avctx, uint8_t *buf) buf[0x21] = ctx->bit_depth == 10 ? 0x58 : 0x38; buf[0x22] = 0x88 + (ctx->interlaced << 2); AV_WB32(buf + 0x28, ctx->cid); // CID - buf[0x2c] = (!ctx->interlaced << 7) | (ctx->is_444 << 6) | (avctx->pix_fmt == AV_PIX_FMT_YUV444P10); + buf[0x2c] = (!ctx->interlaced << 7) | (ctx->is_444 << 6) | (ctx->colorspace << 1) | (avctx->pix_fmt == AV_PIX_FMT_YUV444P10); buf[0x5f] = 0x01; // UDL diff --git a/libavcodec/dnxhdenc.h b/libavcodec/dnxhdenc.h index 7b0d862e28..4d5f457e33 100644 --- a/libavcodec/dnxhdenc.h +++ b/libavcodec/dnxhdenc.h @@ -50,6 +50,7 @@ typedef struct DNXHDEncContext { int profile; int bit_depth; int is_444; + int colorspace; const CIDEntry *cid_table; uint8_t *msip; ///< Macroblock Scan Indexes Payload uint32_t *slice_size;
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavcodec/dnxhdenc.c | 11 ++++++++++- libavcodec/dnxhdenc.h | 1 + 2 files changed, 11 insertions(+), 1 deletion(-)