[FFmpeg-devel] [PATCH v2] avcodec/libkvazaar: Respect codec context color settings.

[FFmpeg-devel] [PATCH v2] avcodec/libkvazaar: Respect codec context color settings.

[John Mather via ffmpeg-devel]

This patch makes the libkvazaar encoder respect color settings that are
present on the codec context, including color range, primaries, transfer
function and colorspace.
---
 libavcodec/libkvazaar.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
index 2ef34dd82e..984f78ba65 100644
--- a/libavcodec/libkvazaar.c
+++ b/libavcodec/libkvazaar.c
@@ -101,6 +101,15 @@ FF_ENABLE_DEPRECATION_WARNINGS
         cfg->rc_algorithm = KVZ_LAMBDA;
     }
 
+    if (avctx->color_range != AVCOL_RANGE_UNSPECIFIED)
+        cfg->vui.fullrange = avctx->color_range == AVCOL_RANGE_JPEG;
+    if (avctx->color_primaries != AVCOL_PRI_UNSPECIFIED)
+        cfg->vui.colorprim = avctx->color_primaries;
+    if (avctx->color_trc != AVCOL_TRC_UNSPECIFIED)
+        cfg->vui.transfer = avctx->color_trc;
+    if (avctx->colorspace != AVCOL_SPC_UNSPECIFIED)
+        cfg->vui.colormatrix = avctx->colorspace;
+
     if (ctx->kvz_params) {
         AVDictionary *dict = NULL;
         if (!av_dict_parse_string(&dict, ctx->kvz_params, "=", ",", 0)) {
-- 
2.39.3

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] avcodec/libkvazaar: Respect codec context color settings.

[Vittorio Giovara]

On Fri, Sep 29, 2023 at 5:12 PM John Mather via ffmpeg-devel <
ffmpeg-devel@ffmpeg.org> wrote:

> This patch makes the libkvazaar encoder respect color settings that are
> present on the codec context, including color range, primaries, transfer
> function and colorspace.
> ---
>  libavcodec/libkvazaar.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
> index 2ef34dd82e..984f78ba65 100644
> --- a/libavcodec/libkvazaar.c
> +++ b/libavcodec/libkvazaar.c
> @@ -101,6 +101,15 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          cfg->rc_algorithm = KVZ_LAMBDA;
>      }
>
> +    if (avctx->color_range != AVCOL_RANGE_UNSPECIFIED)
> +        cfg->vui.fullrange = avctx->color_range == AVCOL_RANGE_JPEG;
> +    if (avctx->color_primaries != AVCOL_PRI_UNSPECIFIED)
> +        cfg->vui.colorprim = avctx->color_primaries;
> +    if (avctx->color_trc != AVCOL_TRC_UNSPECIFIED)
> +        cfg->vui.transfer = avctx->color_trc;
> +    if (avctx->colorspace != AVCOL_SPC_UNSPECIFIED)
> +        cfg->vui.colormatrix = avctx->colorspace;
>

since both avcodec and the library follow the same standard, you could
avoid checking for UNSPECIFIED entirely and just assign the value there
-- 
Vittorio
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] avcodec/libkvazaar: Respect codec context color settings.

[Jan Ekström]

On Sat, Sep 30, 2023 at 12:39 AM Vittorio Giovara
<vittorio.giovara@gmail.com> wrote:
>
> On Fri, Sep 29, 2023 at 5:12 PM John Mather via ffmpeg-devel <
> ffmpeg-devel@ffmpeg.org> wrote:
>
> > This patch makes the libkvazaar encoder respect color settings that are
> > present on the codec context, including color range, primaries, transfer
> > function and colorspace.
> > ---
> >  libavcodec/libkvazaar.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
> > index 2ef34dd82e..984f78ba65 100644
> > --- a/libavcodec/libkvazaar.c
> > +++ b/libavcodec/libkvazaar.c
> > @@ -101,6 +101,15 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >          cfg->rc_algorithm = KVZ_LAMBDA;
> >      }
> >
> > +    if (avctx->color_range != AVCOL_RANGE_UNSPECIFIED)
> > +        cfg->vui.fullrange = avctx->color_range == AVCOL_RANGE_JPEG;
> > +    if (avctx->color_primaries != AVCOL_PRI_UNSPECIFIED)
> > +        cfg->vui.colorprim = avctx->color_primaries;
> > +    if (avctx->color_trc != AVCOL_TRC_UNSPECIFIED)
> > +        cfg->vui.transfer = avctx->color_trc;
> > +    if (avctx->colorspace != AVCOL_SPC_UNSPECIFIED)
> > +        cfg->vui.colormatrix = avctx->colorspace;
> >
>
> since both avcodec and the library follow the same standard, you could
> avoid checking for UNSPECIFIED entirely and just assign the value there

For the record, both libx264 and libx265 wrappers essentially have the
UNSPECIFIED check. For x265 I guess it was due to
bEnableColorDescriptionPresentFlag having to be set by the API caller
as well, while for x264 this logic was added in
48d39c8786d2a1a36258d8e442602729eef0474c , I wonder if due to x264
having an initial default value in the struct, and checking against
that for whether a user set that value or not. Or maybe it was just
cargo culted from somewhere else?

So if kvazaar has no initial value there in those values of the struct
that it uses for checking if a value was set, then the ifs are indeed
unnecessary.

Jan
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] avcodec/libkvazaar: Respect codec context color settings.

[Jan Ekström]

On Sat, Sep 30, 2023 at 12:12 AM John Mather via ffmpeg-devel
<ffmpeg-devel@ffmpeg.org> wrote:
>
> This patch makes the libkvazaar encoder respect color settings that are
> present on the codec context, including color range, primaries, transfer
> function and colorspace.
> ---
>  libavcodec/libkvazaar.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
> index 2ef34dd82e..984f78ba65 100644
> --- a/libavcodec/libkvazaar.c
> +++ b/libavcodec/libkvazaar.c
> @@ -101,6 +101,15 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          cfg->rc_algorithm = KVZ_LAMBDA;
>      }
>
> +    if (avctx->color_range != AVCOL_RANGE_UNSPECIFIED)
> +        cfg->vui.fullrange = avctx->color_range == AVCOL_RANGE_JPEG;
> +    if (avctx->color_primaries != AVCOL_PRI_UNSPECIFIED)
> +        cfg->vui.colorprim = avctx->color_primaries;
> +    if (avctx->color_trc != AVCOL_TRC_UNSPECIFIED)
> +        cfg->vui.transfer = avctx->color_trc;
> +    if (avctx->colorspace != AVCOL_SPC_UNSPECIFIED)
> +        cfg->vui.colormatrix = avctx->colorspace;
> +

Just one thing that I was going to type on IRC but then didn't have the moment:

If kvazaar does support chroma location, please could you add that as
well? :) You can see in libx264 wrapper how that is done when just a
single field needs to be filled, while libx265 wrapper's
9b2281a4a383677ded522e603514789cc22498fa shows how to handle the case
where both the bool and both the internal values have to be written
out.

Of course is kvazaar doesn't yet support writing this out, then
discard this part.

Finally, do these VUI entries require a new minimum kvazaar version
compared to the current configure check? As in, if you try to build
the wrapper with your commit together with... 0.8.1 - does it build
successfully?

Jan
_______________________________________________
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".