Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Jan Ekström" <jeebjp@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec/libkvazaar: Respect codec context color settings.
Date: Sat, 30 Sep 2023 17:16:40 +0300
Message-ID: <CAEu79Sag+STrnMs+rgOeZgp0QjtLdw6OBmixZp3jG5aaETX75A@mail.gmail.com> (raw)
In-Reply-To: <CABLWnS8rbQqgzYxL0tJu394DBYmSe3eTHh_gAepXk2_t83fMpQ@mail.gmail.com>

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

  reply	other threads:[~2023-09-30 14:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-29 21:11 John Mather via ffmpeg-devel
2023-09-29 21:38 ` Vittorio Giovara
2023-09-30 14:16   ` Jan Ekström [this message]
2023-09-30 14:20 ` Jan Ekström

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAEu79Sag+STrnMs+rgOeZgp0QjtLdw6OBmixZp3jG5aaETX75A@mail.gmail.com \
    --to=jeebjp@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git