Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

2019-11-10 Thread Vittorio Giovara
On Mon, Oct 28, 2019 at 10:48 PM Paul B Mahol  wrote:

> The new API is more extensible and allows for custom layouts.
> More accurate information is exported, eg for decoders that do not
> set a channel layout, lavc will not make one up for them.
>
> Deprecate the old API working with just uint64_t bitmasks.
>
> Original commit by Anton Khirnov .
> Expanded and completed by Vittorio Giovara .
> Adapted for FFmpeg by Paul B Mahol .
>
> Signed-off-by: Anton Khirnov 
> Signed-off-by: Vittorio Giovara 
> Signed-off-by: Paul B Mahol 
> ---
>

Hello,
this seems to coming from an outdated branch.

Please use https://git.khirnov.net/libav.git/log/?h=ambisonic2 as
reference: it contains several improvements from the last iteration, such
as a fully functional backward compatibility layer, and a reworked
implementation of ambisonics, and contains a fully rebased tree with all
ffmpeg formats and codecs.

Really looking forward to getting this merged, thanks for your help in the
process.
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/ivfenc: Change the length fields to 32 bits

2019-10-07 Thread Vittorio Giovara
this seems good to me, unless objections i'll merge it in 24h

On Mon, Oct 7, 2019 at 11:10 AM Raphaël Zumer  wrote:

> Just sending a reminder for my set of patches (the set of v2 patches up
> in the thread and this one). Are there any further comments?
>
> Thanks
> Raphaël Zumer
>
> On Wed, 2019-10-02 at 09:04 -0400, Raphaël Zumer wrote:
> > There is no change in the encoded bitstream, but this
> > ensures that the written field length is consistent
> > with the reference implementation.
> >
> > Unused bytes are zeroed out for backwards compatibility.
> >
> > Signed-off-by: Raphaël Zumer 
> > ---
> >  libavformat/ivfenc.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c
> > index ae461a872b..eb70421c44 100644
> > --- a/libavformat/ivfenc.c
> > +++ b/libavformat/ivfenc.c
> > @@ -84,7 +84,8 @@ static int ivf_write_trailer(AVFormatContext *s)
> >
> >  avio_seek(pb, 24, SEEK_SET);
> >  // overwrite the "length" field (duration)
> > -avio_wl64(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx-
> > >frame_cnt - 1));
> > +avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx-
> > >frame_cnt - 1));
> > +avio_wl32(pb, 0); // zero out unused bytes
> >  avio_seek(pb, end, SEEK_SET);
> >  }
> >
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".



-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [REQUEST] ffmpeg-security subscription

2019-08-15 Thread Vittorio Giovara
On Wed, Aug 14, 2019 at 10:11 PM Reimar Döffinger 
wrote:

> On 14.08.2019, at 11:45, Paul B Mahol  wrote:
> > I strongly disagree with you. Why some people have subscription to
> security
> > mailing list and I'm not allowed also?
>
> Long version, explaining to the best of my knowledge and memory:
> The people on it are on it because at some point it was considered
> necessary to have them on it.
> You have not brought an argument why the project would need you to be on
> it in order to deal with issues in a satisfactory way.
> Generally only basic technical skills are needed, enough to discuss the
> issue and potentially hand over to the maintainer. And whoever is involved
> in the releases is generally needed.
> Beyond that I would describe it as a PR function (giving a polite and
> level headed response to a security researcher claiming something that is
> obvious nonsense to an FFmpeg developer tends to make things go much
> smoother), which I would have assumed to not be among your aspirations.
> It's definitely not about a "right" or a "priviledge" or having "earned"
> it, it's about need.
> And when possible a bit of trust (the personal kind, not just the "not
> malicious" kind which is of course an absolute requirement), though that
> might be more relevant for projects like Linux where really bad stuff
> causing stress and possibly conflicts tends to hit. Flame wars is the last
> thing one needs in the middle of dealing with a bad issue.
>
> TL;DR is probably: one doesn't end up on security lists by asking to be on
> it but by persons Y and Z saying "we should/need to have person X on the
> list".
> I am not aware of any such wishes (though admittedly I wouldn't be the one
> contacted about it I guess).
>

I think being on the security list may have some professional implications
too: if you use ffmpeg in your $dayjob, being notified of security problem
in ffmpeg, and acting upon it before the fix lands in the tree, may be
crucial. I think Paul is lamenting the fact that being selected for the
security list is extremely arbitrary and there is no process described on
how to joining it.
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 6/8] avcodec/decode: Do not overwrite AVFrame.pkt_pos if its already set

2019-08-12 Thread Vittorio Giovara
On Mon, Aug 12, 2019 at 9:19 PM Michael Niedermayer 
wrote:

> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/decode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 6c31166ec2..09a509d659 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -435,7 +435,7 @@ static int decode_simple_internal(AVCodecContext
> *avctx, AVFrame *frame)
>  if (!(avctx->codec->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS))
>  frame->pkt_dts = pkt->dts;
>  if (avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
> -if(!avctx->has_b_frames)
> +if(!avctx->has_b_frames && frame->pkt_pos < 0)
>  frame->pkt_pos = pkt->pos;
>  //FIXME these should be under if(!avctx->has_b_frames)
>  /* get_buffer is supposed to set frame parameters */
> --
>

sure but why?
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] mov: Support fake moov boxes disguised as hoov

2019-08-12 Thread Vittorio Giovara
On Thu, Aug 8, 2019 at 10:28 PM Vittorio Giovara 
wrote:

> Some broken apps generate files that have a fake box named 'hoov'
> instead of a proper 'moov' one. This is speculation but it seems like
> this box contains data to be modified later (eg as file grows in size,
> data gets re-written) and its name is supposed to be changed to 'moov'
> once it can be used as a 'moov', but for some reason this step is skipped.
>
> Since this is not the first time this happens ('moov' boxes can be found
> in 'free' ones) extend the existing hacks to search for the moov in such
> boxes and skip the moov_retry since it needs to be found right away.
> ---
>  libavformat/mov.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> This code is ugly, tips for improving it are welcome, or a full
> rejection is ok too. Unfortunately I cannot share the sample, but VLC
> is able to play it.
> Vittorio
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 24de5429d1..0c2986b72e 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -6800,10 +6800,10 @@ static int mov_read_default(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
>  if (atom.size >= 8) {
>  a.size = avio_rb32(pb);
>  a.type = avio_rl32(pb);
> -if (a.type == MKTAG('f','r','e','e') &&
> +if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) ||
> +  a.type == MKTAG('h','o','o','v')) &&
>  a.size >= 8 &&
> -c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT &&
> -c->moov_retry) {
> +c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
>  uint8_t buf[8];
>  uint32_t *type = (uint32_t *)buf + 1;
>  if (avio_read(pb, buf, 8) != 8)
> @@ -6811,7 +6811,7 @@ static int mov_read_default(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
>  avio_seek(pb, -8, SEEK_CUR);
>  if (*type == MKTAG('m','v','h','d') ||
>  *type == MKTAG('c','m','o','v')) {
> -av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free
> atom.\n");
> +av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free
> or hoov atom.\n");
>  a.type = MKTAG('m','o','o','v');
>  }
>  }
> --
>

ping?
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avutil/pixfmt: Add EBU Tech. 3213-E AVColorPrimaries value

2019-08-09 Thread Vittorio Giovara
On Fri, Aug 9, 2019 at 3:29 AM  wrote:

> From: Raphaël Zumer 
>
> This is an alias for JEDEC P22.
>
> The name associated with the value is also changed
> from "jedec-p22" to "ebu3213" to match ITU-T H.273.
>
> Signed-off-by: Raphaël Zumer 
> ---
>  doc/APIchanges  | 3 +++
>  libavutil/pixdesc.c | 2 +-
>  libavutil/pixfmt.h  | 1 +
>  libavutil/version.h | 2 +-
>  4 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 6603a8229e..f71b0c4a75 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>
>  API changes, most recent first:
>
> +2019-08-08 - xx - lavu 56.34.100 - pixfmt.h
> +  Add EBU Tech. 3213-E AVColorPrimaries value
>

this value was already present, this is just a rename


> +
>  2019-07-27 - xx - lavu 56.33.100 - tx.h
>Add AV_TX_DOUBLE_FFT and AV_TX_DOUBLE_MDCT
>
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index b97b0665b0..3b2d3b3123 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -2369,7 +2369,7 @@ static const char * const
> color_primaries_names[AVCOL_PRI_NB] = {
>  [AVCOL_PRI_SMPTE428] = "smpte428",
>  [AVCOL_PRI_SMPTE431] = "smpte431",
>  [AVCOL_PRI_SMPTE432] = "smpte432",
> -[AVCOL_PRI_JEDEC_P22] = "jedec-p22",
> +[AVCOL_PRI_JEDEC_P22] = "ebu3213",
>  };
>
>  static const char * const color_transfer_names[] = {
> diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h
> index 8b54c9415b..7f7b537721 100644
> --- a/libavutil/pixfmt.h
> +++ b/libavutil/pixfmt.h
> @@ -457,6 +457,7 @@ enum AVColorPrimaries {
>  AVCOL_PRI_SMPTE431= 11, ///< SMPTE ST 431-2 (2011) / DCI P3
>  AVCOL_PRI_SMPTE432= 12, ///< SMPTE ST 432-1 (2010) / P3 D65 /
> Display P3
>  AVCOL_PRI_JEDEC_P22   = 22, ///< JEDEC P22 phosphors
> +AVCOL_PRI_EBU3213 = AVCOL_PRI_JEDEC_P22,
>

I'd do the opposite, similarly to what is done for AVCOL_PRI_SMPTEST428_1

   AVCOL_PRI_EBU3213 = 22, ///< JEDEC P22 phosphors, EBU Tech 3213 E
   AVCOL_PRI_JEDEC_P22   = AVCOL_PRI_EBU3213,

 AVCOL_PRI_NB///< Not part of ABI
>  };
>
> --
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-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] mov: Support fake moov boxes disguised as hoov

2019-08-08 Thread Vittorio Giovara
Some broken apps generate files that have a fake box named 'hoov'
instead of a proper 'moov' one. This is speculation but it seems like
this box contains data to be modified later (eg as file grows in size,
data gets re-written) and its name is supposed to be changed to 'moov'
once it can be used as a 'moov', but for some reason this step is skipped.

Since this is not the first time this happens ('moov' boxes can be found
in 'free' ones) extend the existing hacks to search for the moov in such
boxes and skip the moov_retry since it needs to be found right away.
---
 libavformat/mov.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

This code is ugly, tips for improving it are welcome, or a full
rejection is ok too. Unfortunately I cannot share the sample, but VLC
is able to play it.
Vittorio

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 24de5429d1..0c2986b72e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6800,10 +6800,10 @@ static int mov_read_default(MOVContext *c, AVIOContext 
*pb, MOVAtom atom)
 if (atom.size >= 8) {
 a.size = avio_rb32(pb);
 a.type = avio_rl32(pb);
-if (a.type == MKTAG('f','r','e','e') &&
+if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) ||
+  a.type == MKTAG('h','o','o','v')) &&
 a.size >= 8 &&
-c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT &&
-c->moov_retry) {
+c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
 uint8_t buf[8];
 uint32_t *type = (uint32_t *)buf + 1;
 if (avio_read(pb, buf, 8) != 8)
@@ -6811,7 +6811,7 @@ static int mov_read_default(MOVContext *c, AVIOContext 
*pb, MOVAtom atom)
 avio_seek(pb, -8, SEEK_CUR);
 if (*type == MKTAG('m','v','h','d') ||
 *type == MKTAG('c','m','o','v')) {
-av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free 
atom.\n");
+av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free or 
hoov atom.\n");
 a.type = MKTAG('m','o','o','v');
 }
 }
-- 
2.22.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavc/videotoolboxenc: add hdr10, linear, hlg color transfer function for videotoolboxenc

2019-07-17 Thread Vittorio Giovara
On Tue, Jul 16, 2019 at 10:29 PM Limin Wang  wrote:

> On Tue, Jul 16, 2019 at 09:36:32PM -0400, Rick Kern wrote:
> > Testing for the new transfer functions when compiling for OSX 10.12
> reports
> > the color settings as "yuv420p(tv, bt2020nc/bt2020/reserved)" in ffprobe.
> > Is "reserved" (0) the expected default when the transfer function isn't
> > supported?
> mac 10.13 support:
> kCVImageBufferTransferFunction_SMPTE_ST_2084_PQ
> kCVImageBufferTransferFunction_ITU_R_2100_HLG
>
> OSX 10.14 support:
> kCVImageBufferTransferFunction_Linear
>
> For 10.12, the HAVE_* macros should be detected as 0 if correct.
>

0 is a reserved value in the color specification and should be avoided if
possible.
Would it be possible to fallback on 2 (unknown) when these macros are not
found?
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavfilter/vf_colorspace.c: fix demarcation point of gamma linearize function

2019-07-03 Thread Vittorio Giovara
On Wed, Jul 3, 2019 at 12:51 AM vincenluo(罗永林) 
wrote:

> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Great find, your patch is absolutely correct.
Pushing soon, thank you.
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/cfhd: remove unused function

2019-06-27 Thread Vittorio Giovara
On Thu, Jun 27, 2019 at 9:44 AM Nicolas George  wrote:

> Kieran Kunhya (12019-06-27):
> > I'm happy to do it now that I am aware of the issue. I will do it when I
> am
> > at home in a few days.
>
> Thanks. I am sure Steven will not mind waiting a few days.
>
> > This absolutism is absurd.
>
> Do you have an example of situation where dead code is good?
>

If i could add my 2 cents, for a reverse engineered codec it's important to
leave unused functions purely for documentation purposes, so that future
maintainers could implement and read about it right away, rather than
digging in a large and messy git history.
Additionally most compilers run passes that drop dead code already in a way
that does not affect runtime one bit. So I really don't see any gains in
removing these 14 lines of code.
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avcodec/h264_sei: Add acces to truncated SEI data

2019-06-09 Thread Vittorio Giovara
On Sat, Jun 8, 2019 at 9:28 AM Antonin Gouzer 
wrote:

> ---
> Some codecs editors had miss interpreted the H264 standart and
> have coded a wrong size in the SEI data.
> size = SEI size + 1.
> The SEI data is detected as "truncated"
> Ex:
> https://drive.google.com/file/d/1cNtLwnfPnyJnYqE7OYhU3SCoLRtuXIUM/view?usp=sharing
> Command:
> ffprobe -print_format xml -show_frames -read_intervals %+0.04
> truncated.h264
> This (simple) patch add the possibility to read this false truncated SEI
> data with the default stric_std_compliance or less.
> The error remain logged in both cases.
>
> V2: Modifiy the patch for only the off by one values
>
> Thanks in advance !
> ---
>  libavcodec/h264_sei.c | 24 +++-
>  libavcodec/h264_sei.h |  2 +-
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index d4eb9c0dab..7871cf87ed 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -402,7 +402,7 @@ static int
> decode_alternative_transfer(H264SEIAlternativeTransfer *h,
>  }
>
>  int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb,
> -   const H264ParamSets *ps, void *logctx)
> +   const H264ParamSets *ps, AVCodecContext *avctx)
>  {
>  int master_ret = 0;
>
>
This may be a minor note, but i don't think it's worth to check for std
compliance within a *parsing* function: in my opinion it would make more
sense to check in the calling function, and always allow for truncated sei,
and error out only if (avctx->error | AV_ERROR_EXPLODE) is set (sorry going
by memory here)
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avcodec: Add librav1e encoder

2019-05-29 Thread Vittorio Giovara
On Wed, May 29, 2019 at 2:28 PM Derek Buitenhuis 
wrote:

> Uses the crav1e C bindings for rav1e.
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -1378,6 +1378,35 @@ makes it possible to store non-rgb pix_fmts.
>
>  @end table
>
> +@section librav1e
> +
> +rav1e AV1 encoder wrapper.
> +
> +Requires the presence of the rav1e headers and library from crav1e
> +during configuration. You need to explicitly configue the build with
>

nit: typo configure

+
> +static inline RaPixelRange range_map(enum AVColorRange range)
> +
> +{
>

nit: extra line

+switch (range) {
> +case AVCOL_RANGE_MPEG:
> +return RA_PIXEL_RANGE_LIMITED;
> +case AVCOL_RANGE_JPEG:
> +return RA_PIXEL_RANGE_FULL;
> +default:
> +return RA_PIXEL_RANGE_UNSPECIFIED;
> +}
> +}
> +
> +static inline RaChromaSampling pix_fmt_map(enum AVPixelFormat pix_fmt)
> +{
> +switch (pix_fmt) {
> +case AV_PIX_FMT_YUV420P:
> +case AV_PIX_FMT_YUV420P10:
> +case AV_PIX_FMT_YUV420P12:
> +return RA_CHROMA_SAMPLING_CS420;
> +case AV_PIX_FMT_YUV422P:
> +case AV_PIX_FMT_YUV422P10:
> +case AV_PIX_FMT_YUV422P12:
> +return RA_CHROMA_SAMPLING_CS422;
> +case AV_PIX_FMT_YUV444P:
> +case AV_PIX_FMT_YUV444P10:
> +case AV_PIX_FMT_YUV444P12:
> +return RA_CHROMA_SAMPLING_CS444;
>

no love for the J formats? ;_;
could they be added to librav1e_pix_fmts[]?

+
> +static av_cold int librav1e_encode_init(AVCodecContext *avctx)
> +{
> +librav1eContext *ctx = avctx->priv_data;
> +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
>

nit: check desc != NULL

+RaConfig *cfg = NULL;
> +int rret;
> +int ret = 0;
> +
>
[...]

> +
> +rret = rav1e_config_parse_int(cfg, "threads", avctx->thread_count);
> +if (rret < 0)
> +av_log(avctx, AV_LOG_WARNING, "Invalid number of threads,
> defaulting to auto.\n");
> +
> +if (avctx->bit_rate && ctx->quantizer < 0) {
>

would be nice to have an info/debug line about the rc in use


> +static int librav1e_send_frame(AVCodecContext *avctx, const AVFrame
> *frame)
> +{
> +librav1eContext *ctx = avctx->priv_data;
> +int ret;
> +
> +if (!ctx->rframe && frame) {
> +const AVPixFmtDescriptor *desc =
> av_pix_fmt_desc_get(frame->format);
>
> nit: check desc != NULL


> +ctx->rframe = rav1e_frame_new(ctx->ctx);
> +if (!ctx->rframe) {
> +av_log(avctx, AV_LOG_ERROR, "Could not allocate new rav1e
> frame.\n");
> +return AVERROR(ENOMEM);
> +}
> +
>

overall lgtm
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] Export display matrix mirroring info as part of the rotate API

2019-05-16 Thread Vittorio Giovara
On Thu, May 16, 2019 at 9:32 PM Jun Li  wrote:

> On Thu, May 16, 2019 at 4:34 PM Ted Meyer <
> tmathmeyer-at-google@ffmpeg.org> wrote:
>
> > Right now ffmpeg doesn't export a mirroring status when checking the
> > display matrix for rotation.
> > Here is an example video: https://files.tedm.io/flip.mp4
> > -Ted
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
>
> There is a patch but not merged into master, quality not guaranteed :)
> Hope this helps.
> https://patchwork.ffmpeg.org/patch/13130/
>

hey i just noticed this patch

+if (CONV_FP(m[0]) * CONV_FP(m[4]) < CONV_FP(m[1]) *
CONV_FP(m[3])) {+*hflip = 1;+av_display_matrix_flip(m,
1, 0);+}

the long if is basically computing the determinant of the matrix, but you
only need the fact whether it's positive or negative, you can discard the
result so you can avoid converting to CONV_FP, and just cast to int64_t

+return av_display_rotation_get(m);

don't you need to set vertical flip only if det < 0 and rot = 180?

beside that small point, this patch introduces an api that basically
supersedes the normal av_display_rotation_get(), and does many more things,
I'd be tempted to deprecate any other use, and what do you think? in that
case you could just call it av_display_rotation_get2() like is tradition

I can't find the other patches from the set to review, would you be able to
send an updated version?
thanks
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_tonemap: add slice threading

2019-04-30 Thread Vittorio Giovara
On Tue, Apr 30, 2019 at 6:07 AM Paul B Mahol  wrote:

> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/vf_tonemap.c | 39 ++-
>  1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/libavfilter/vf_tonemap.c b/libavfilter/vf_tonemap.c
> index efd4af5466..d1728c8513 100644
> --- a/libavfilter/vf_tonemap.c
> +++ b/libavfilter/vf_tonemap.c
> @@ -191,10 +191,36 @@ static void tonemap(TonemapContext *s, AVFrame *out,
> const AVFrame *in,
>  *b_out *= sig / sig_orig;
>  }
>
> +typedef struct ThreadData {
> +AVFrame *in, *out;
> +const AVPixFmtDescriptor *desc;
> +double peak;
> +} ThreadData;
> +
> +static int tonemap_slice(AVFilterContext *ctx, void *arg, int jobnr, int
> nb_jobs)
> +{
> +TonemapContext *s = ctx->priv;
> +ThreadData *td = arg;
> +AVFrame *in = td->in;
> +AVFrame *out = td->out;
> +const AVPixFmtDescriptor *desc = td->desc;
> +const int slice_start = (in->height * jobnr) / nb_jobs;
> +const int slice_end = (in->height * (jobnr+1)) / nb_jobs;
> +double peak = td->peak;
> +
> +for (int y = slice_start; y < slice_end; y++)
> +for (int x = 0; x < out->width; x++)
> +tonemap(s, out, in, desc, x, y, peak);
> +
> +return 0;
> +}
> +
>  static int filter_frame(AVFilterLink *link, AVFrame *in)
>  {
> -TonemapContext *s = link->dst->priv;
> -AVFilterLink *outlink = link->dst->outputs[0];
> +AVFilterContext *ctx = link->dst;
> +TonemapContext *s = ctx->priv;
> +AVFilterLink *outlink = ctx->outputs[0];
> +ThreadData td;
>  AVFrame *out;
>  const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
>  const AVPixFmtDescriptor *odesc =
> av_pix_fmt_desc_get(outlink->format);
> @@ -245,9 +271,11 @@ static int filter_frame(AVFilterLink *link, AVFrame
> *in)
>  }
>
>  /* do the tone map */
> -for (y = 0; y < out->height; y++)
> -for (x = 0; x < out->width; x++)
> -tonemap(s, out, in, desc, x, y, peak);
> +td.out = out;
> +td.in = in;
> +td.desc = desc;
> +td.peak = peak;
> +ctx->internal->execute(ctx, tonemap_slice, , NULL,
> FFMIN(in->height, ff_filter_get_nb_threads(ctx)));
>

why this FFMIN()?

 /* copy/generate alpha if needed */
>  if (desc->flags & AV_PIX_FMT_FLAG_ALPHA && odesc->flags &
> AV_PIX_FMT_FLAG_ALPHA) {
> @@ -315,4 +343,5 @@ AVFilter ff_vf_tonemap = {
>  .priv_class  = _class,
>  .inputs  = tonemap_inputs,
>  .outputs = tonemap_outputs,
> +.flags   = AVFILTER_FLAG_SLICE_THREADS,
>  };
> --
>

lgtm
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avcodec: add AV_CODEC_FLAG_DROPCHANGED to flags

2019-04-17 Thread Vittorio Giovara
On Tue, Apr 16, 2019 at 3:42 AM Gyan  wrote:

> Patch revised as per
> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-April/242591.html
>
> Gyan
>

sorry if i'm late to the party but why are these new fields in
avcodeccontext?
they seem to be perfect candidates for side data, or am i missing something?
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avutil/colorspace: add macros for RGB->YUV BT.709

2019-04-17 Thread Vittorio Giovara
On Wed, Apr 17, 2019 at 12:26 AM Gyan  wrote:

>
>
> On 13-04-2019 05:23 PM, Gyan wrote:
> > Will be helpful for correct result in filters that paint like
> > fillborders/drawbox or those using drawutils.
>
> Ping.
>

these seem to only work for 8 bit content, is that their only intended
usecase?
if so, could there be at least a comment mentioning that please?
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v10 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

2019-03-29 Thread Vittorio Giovara
On Fri, Mar 29, 2019 at 5:28 AM Jing Sun  wrote:

> +static int config_enc_params(EB_H265_ENC_CONFIGURATION *param,
> + AVCodecContext *avctx)
> +{
> +SvtContext *svt_enc = avctx->priv_data;
> +int ret;
> +
> +param->sourceWidth = avctx->width;
> +param->sourceHeight = avctx->height;
> +param->encoderBitDepth = 8;
> +
> +if (avctx->pix_fmt == AV_PIX_FMT_YUV420P10LE) {
> +av_log(avctx, AV_LOG_DEBUG, "Encoder 10 bits depth input\n");
> +
> +param->encoderBitDepth = 10;
> +}
> +param->encoderColorFormat = EB_YUV420;
>
>
this patch blatantly ignores my review(s) and therefore it is rejected

to reiterate:
- if the encoder does not support 10 bit (or if scales down from 10 to 8
internally), this feature should not be present in the wrapper either (or
it should at least warn the user)
- if the encoder does not support setting the color properties in the VUI,
the wrapper should definitely warn the user of the loss of information (or
we should wait until this features is present upstream)

reagrds
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v9 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper

2019-03-27 Thread Vittorio Giovara
On Tue, Mar 26, 2019 at 10:47 PM Jing Sun  wrote:

> Signed-off-by: Zhengxu Huang 
> Signed-off-by: Hassene Tmar 
> Signed-off-by: Jun Zhao 
> Signed-off-by: Jing Sun 
> ---
>  configure|   4 +
>  libavcodec/Makefile  |   1 +
>  libavcodec/allcodecs.c   |   1 +
>  libavcodec/libsvt_hevc.c | 502
> +++
>  4 files changed, 508 insertions(+)
>  create mode 100644 libavcodec/libsvt_hevc.c
>
> +
> +if (avctx->pix_fmt == AV_PIX_FMT_YUV420P10LE) {
> +av_log(avctx, AV_LOG_DEBUG , "Encoder 10 bits depth input\n");
> +
> +// SVT-HEVC's compressed 10-bit format is to supported,
> +// without which it works well functionally but with a
> +// slight performance penalty caused by the extra conv
> +// step from yuv420p10le to that format.
> +
> +param->compressedTenBitFormat = 0;
> +param->encoderBitDepth= 10;
> +}
>

So what is happening in this case? The encoder is slower because it
converts from 10 to 8 bit internally? And then the output encode is 8 bit
right now? If that is the case, I'd rather have this functionality removed
since the conversion can happen directly within ffmpeg. Having the
conversion performed by the encoder is guaranteed to be slower and less
precise, and if the output is not 10 bit it is very surprising too.

At the same time the comment in the code is useless because users will
never read something buried deep in the code, I'd suggest printing
something at the warning level so that it will be shown during the
conversion (and please have it proofread by a native English-speaking
person).


> +param->encoderColorFormat = EB_YUV420;
> +
> +// Update param from options
> +param->hierarchicalLevels = svt_enc->hierarchical_level - 1;
> +param->encMode= svt_enc->enc_mode;
> +param->profile= svt_enc->profile;
> +param->tier   = svt_enc->tier;
> +param->level  = svt_enc->level;
> +param->rateControlMode= svt_enc->rc_mode;
> +param->sceneChangeDetection   = svt_enc->scd;
> +param->tune   = svt_enc->tune;
> +param->baseLayerSwitchMode= svt_enc->base_layer_switch_mode;
> +param->qp = svt_enc->qp;
> +param->accessUnitDelimiter= svt_enc->aud;
> +
> +param->targetBitRate  = avctx->bit_rate;
> +if (avctx->gop_size > 0)
> +param->intraPeriodLength  = avctx->gop_size - 1;
> +
> +if (avctx->framerate.num > 0 && avctx->framerate.den > 0) {
> +param->frameRateNumerator = avctx->framerate.num;
> +param->frameRateDenominator   = avctx->framerate.den *
> avctx->ticks_per_frame;
> +} else {
> +param->frameRateNumerator = avctx->time_base.den;
> +param->frameRateDenominator   = avctx->time_base.num *
> avctx->ticks_per_frame;
> +}
> +
> +if (param->rateControlMode) {
> +param->maxQpAllowed   = avctx->qmax;
> +param->minQpAllowed   = avctx->qmin;
> +}
> +
> +param->intraRefreshType   =
> +!!(avctx->flags & AV_CODEC_FLAG_CLOSED_GOP) + 1;
> +
> +// is it repeat headers for MP4 or Annex-b
> +if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER)
> +param->codeVpsSpsPps  = 0;
> +else
> +param->codeVpsSpsPps  = 1;
> +
> +param->codeEosNal = 1;
>

nit: excessive whitespace alignment

+
> +if (svt_enc->hdr) {
> +svt_enc->vui_info = 1;
> +param->highDynamicRangeInput = svt_enc->hdr;
> +}
>

Where is the warning that notifies the lack of color properties support?


> +
> +avctx->extradata_size = header_ptr->nFilledLen;
> +avctx->extradata = av_mallocz(avctx->extradata_size +
> AV_INPUT_BUFFER_PADDING_SIZE);
> +if (!avctx->extradata) {
> +av_log(avctx, AV_LOG_ERROR,
> +   "Cannot allocate HEVC header of size %d.\n",
> avctx->extradata_size);
> +svt_ret = EB_ErrorInsufficientResources;
> +goto failed_init_enc;
> +}
>

initialize extradata_size only in case of success, some code may rely on it

+#define OFFSET(x) offsetof(SvtContext, x)
> +#define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +static const AVOption options[] = {
> +{ "vui", "Enable vui info", OFFSET(vui_info),
> +  AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, VE },
> +
> +{ "aud", "Include AUD", OFFSET(aud),
> +  AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
>

IMO these options help text could be improved.

-- 
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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

2019-03-19 Thread Vittorio Giovara
On Tue, Mar 19, 2019 at 2:17 AM Sun, Jing A  wrote:

> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Jing SUN
> Sent: Monday, March 11, 2019 6:38 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Sun, Jing A ; Huang, Zhengxu <
> zhengxu.hu...@intel.com>; Jun Zhao ; Tmar, Hassene <
> hassene.t...@intel.com>
> Subject: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc
> encoder wrapper.
>
> From: Jing Sun 
>
> base on patch by Huang, Zhengxu from https://github.com/intel/SVT-HEVC
>
> V4: - Fix the build error with new API in PR#52
> - Fix the encoding hang issue by API change in PR#52
> - Fix the last frame dropping issue
> - Fix the invalid parameter causing segmentation fault issue
> - Add the support to svt hevc and av1 plugins coexistance
> - Add the VMAF optimized mode to "-tune"
> - Add the "-hdr" parameter
>
> V3: - Fix the build error with new API
>
> V2: - Change the config options (didn't need to enable-gpl for BSD+Patent,
>   it's can compatible with LGPL2+, thanks Xavier correct this part),
>   now just need to "--enable-libsvthevc" option
> - Add force_idr option
> - Remove default GoP size setting in the wrapper, SVT-HEVC will calc
>   the the GoP size internal
> - Refine the code as the FFmpeg community's comments
>   (https://patchwork.ffmpeg.org/patch/11347/)
>
> V1: - base on patch by Huang, Zhengxu, then refine some code.
>

all this version history should be in the email thread but not in the
commit section in my opinion


>
> Change-Id: If0dcc5044ab9effd6847a8f48797b985d02b0816
>

this id means nothing in ffmpeg git, I'd suggest dropping it


> Signed-off-by: Huang, Zhengxu 
> Signed-off-by: hassene 
>

Should there be a real name here?


> Signed-off-by: Jun Zhao 
> Signed-off-by: Jing Sun 
> Signed-off-by: Jing SUN 
>

double sign-off?

---
>
[..]

> --
> 1.8.3.1
>
> Hi maintainers,
>
> A week has passed and no negative comment is received except a new feature
> request, which will be considered in the next version. Is the first version
> good to be picked?
>

Until that feature request is added, there should be a warning about that
missing missing feature IMO.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

2019-03-13 Thread Vittorio Giovara
On Wed, Mar 13, 2019 at 8:29 PM Sun, Jing A  wrote:

>
>
> > On Tue, Mar 12, 2019 at 11:40 PM Sun, Jing A 
> wrote:
>
> Hi Giovara,
>
>
>
> SVT HEVC has the interface to enable/disable sending a vui structure in
> the HEVC bitstream, but supports no interface for setting the color
> properties before encoding yet. I will be opening an issue in SVT HEVC
> github asking if they have plans to add such feature, and will keep you
> posted. In the meantime, I think it is not blocking the first version of
> this plugin’s merging , is it?
>
>
>
>
>
> > It kind-of is, what is the point of setting the HDR information (or
> encoding in hevc at all) if you can't set the related color properties?
>
> > At the very least there should be a big fat warning that notifies users
> of this missing feature, when such information is lost.
>
> > --
>
> > Vittorio
>
>
>
> SVT HEVC’s HDR is also an on-off switch, please refer to
> https://github.com/intel/SVT-HEVC/blob/master/Docs/svt-hevc_encoder_user_guide.md
> .
>
>
>
> -Jing
>

Sorry, I fail to see the logical connection between my comment and your
reply.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

2019-03-13 Thread Vittorio Giovara
On Tue, Mar 12, 2019 at 11:40 PM Sun, Jing A  wrote:

> Hi Giovara,
>
>
>
> SVT HEVC has the interface to enable/disable sending a vui structure in
> the HEVC bitstream, but supports no interface for setting the color
> properties before encoding yet. I will be opening an issue in SVT HEVC
> github asking if they have plans to add such feature, and will keep you
> posted. In the meantime, I think it is not blocking the first version of
> this plugin’s merging , is it?
>
>
>
It kind-of is, what is the point of setting the HDR information (or
encoding in hevc at all) if you can't set the related color properties?
At the very least there should be a big fat warning that notifies users of
this missing feature, when such information is lost.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

2019-03-11 Thread Vittorio Giovara
On Mon, Mar 11, 2019 at 12:50 AM Sun, Jing A  wrote:

> I just searched my inbox again but failed to find that email of question
> you mentioned.
>

Yeah I often see my mail bounced with this message:
Address not foundYour message wasn't delivered to *jun.z...@intel.com*
because the address couldn't be found, or is unable to receive mail.

For reference this was the message on the mailing list
https://ffmpeg.org/pipermail/ffmpeg-devel/2019-March/240663.html

Could you please elaborate your request? What is the preservation for and
> how is it expected to work?
>

Yes of course, when you encode an HEVC stream you should be able to signal
how the color properties of the video buffers should be rendered. This is
usually conveyed with three parameters, the matrix coefficients, the color
primaries and the transfer characteristics. Without such information, the
data stored in the video may be interpreted differently and often
incorrectly by modern video players, causing image degradation, wrong
rendering and off colors.

For HEVC they are usually expressed in the stream itself, under the VUI,
and it is kinda expected that modern encoder allow to set them to any of
the applicable values.
In ffmpeg-land, they are represented by the colorspace, color_primaries and
color_transfer options in AVCodecContext and carried over through the whole
video processing.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v7 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

2019-03-08 Thread Vittorio Giovara
On Fri, Mar 8, 2019 at 4:39 AM Jing SUN  wrote:

> From: Jing Sun 
>
> base on patch by Huang, Zhengxu from https://github.com/intel/SVT-HEVC
>
> V4: - Fix the build error with new API in PR#52
> - Fix the encoding hang issue by API change in PR#52
> - Fix the last frame dropping issue
> - Fix the invalid parameter causing segmentation fault issue
> - Add the support to svt hevc and av1 plugins coexistance
> - Add the VMAF optimized mode to "-tune"
> - Add the "-hdr" parameter
>

Apologies if i missed your reply but i think my question was not answered

Is there any way to preserve the color matrix/primary/transfer properties
during encoding?

Thank you
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] avcodec/libdav1d: use a custom picture allocator

2019-03-05 Thread Vittorio Giovara
On Mon, Mar 4, 2019 at 4:08 PM James Almer  wrote:

> Replaces the libdav1d internal allocator. It uses an AVBufferPool to
> reduce the
> amount of allocated buffers.
> About 5% speed up when decoding 720p or higher streams.
>
> Signed-off-by: James Almer 
> ---
> get_buffer2() can't be used for this decoder, as there's no guarantee the
> buffers
> it returns will respect the constrains specified by libdav1d.
>
>  libavcodec/libdav1d.c | 72 ++-
>  1 file changed, 65 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index 459bbae687..855c27db7f 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -22,6 +22,7 @@
>  #include 
>
>  #include "libavutil/avassert.h"
> +#include "libavutil/imgutils.h"
>  #include "libavutil/opt.h"
>
>  #include "avcodec.h"
> @@ -31,12 +32,21 @@
>  typedef struct Libdav1dContext {
>  AVClass *class;
>  Dav1dContext *c;
> +AVBufferPool *pool;
> +int pool_size;
>
>  Dav1dData data;
>  int tile_threads;
>  int apply_grain;
>  } Libdav1dContext;
>
> +static const enum AVPixelFormat pix_fmt[][3] = {
> +[DAV1D_PIXEL_LAYOUT_I400] = { AV_PIX_FMT_GRAY8,   AV_PIX_FMT_GRAY10,
>   AV_PIX_FMT_GRAY12 },
> +[DAV1D_PIXEL_LAYOUT_I420] = { AV_PIX_FMT_YUV420P,
> AV_PIX_FMT_YUV420P10, AV_PIX_FMT_YUV420P12 },
> +[DAV1D_PIXEL_LAYOUT_I422] = { AV_PIX_FMT_YUV422P,
> AV_PIX_FMT_YUV422P10, AV_PIX_FMT_YUV422P12 },
> +[DAV1D_PIXEL_LAYOUT_I444] = { AV_PIX_FMT_YUV444P,
> AV_PIX_FMT_YUV444P10, AV_PIX_FMT_YUV444P12 },
> +};
> +
>  static void libdav1d_log_callback(void *opaque, const char *fmt, va_list
> vl)
>  {
>  AVCodecContext *c = opaque;
> @@ -44,6 +54,57 @@ static void libdav1d_log_callback(void *opaque, const
> char *fmt, va_list vl)
>  av_vlog(c, AV_LOG_ERROR, fmt, vl);
>  }
>
> +static int libdav1d_picture_allocator(Dav1dPicture *p, void *cookie) {
> +Libdav1dContext *dav1d = cookie;
> +enum AVPixelFormat format = pix_fmt[p->p.layout][p->seq_hdr->hbd];
> +int ret, linesize[4], h = FFALIGN(p->p.h, 128);
> +uint8_t *aligned_ptr, *data[4];
> +AVBufferRef *buf;
> +
> +ret = av_image_fill_arrays(data, linesize, NULL, format,
> FFALIGN(p->p.w, 128),
> +   h, DAV1D_PICTURE_ALIGNMENT);
> +if (ret < 0)
> +return ret;
> +
> +if (ret != dav1d->pool_size) {
> +av_buffer_pool_uninit(>pool);
> +// Use twice the amount of required padding bytes for aligned_ptr
> below.
> +dav1d->pool = av_buffer_pool_init(ret + DAV1D_PICTURE_ALIGNMENT *
> 2, NULL);
> +if (!dav1d->pool)
> +return -ENOMEM;
>

AVERROR(ENOMEM) ?

+dav1d->pool_size = ret;
> +}
> +buf = av_buffer_pool_get(dav1d->pool);
> +if (!buf)
> +return -ENOMEM;
> +
> +// libdav1d requires DAV1D_PICTURE_ALIGNMENT aligned buffers, which
> av_malloc()
> +// doesn't guarantee for example when AVX is disabled at configure
> time.
> +// Use the extra DAV1D_PICTURE_ALIGNMENT padding bytes in the buffer
> to align it
> +// if required.
> +aligned_ptr = (uint8_t *)FFALIGN((uintptr_t)buf->data,
> DAV1D_PICTURE_ALIGNMENT);
> +ret = av_image_fill_pointers(data, format, h, aligned_ptr, linesize);
> +if (ret < 0) {
> +av_buffer_unref();
> +return ret;
> +}
> +
> +p->data[0] = data[0];
> +p->data[1] = data[1];
> +p->data[2] = data[2];
> +p->stride[0] = linesize[0];
> +p->stride[1] = linesize[1];
> +p->allocator_data = buf;
> +
> +return 0;
> +}
> +
> +static void libdav1d_picture_release(Dav1dPicture *p, void *cookie) {
> +AVBufferRef *buf = p->allocator_data;
> +
> +av_buffer_unref();
> +}
>

nit: keep { on a new line for functions

-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] avcodec/libdav1d: route dav1d internal logs through av_log()

2019-03-05 Thread Vittorio Giovara
On Mon, Mar 4, 2019 at 4:08 PM James Almer  wrote:

> Bump the minimum required version to the first one with the logger API
> callback.
>
> Signed-off-by: James Almer 
> ---
>  configure | 2 +-
>  libavcodec/libdav1d.c | 9 +
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index dcead3a300..a5cef4bc09 100755
> --- a/configure
> +++ b/configure
> @@ -6142,7 +6142,7 @@ enabled libcelt   && require libcelt
> celt/celt.h celt_decode -lcelt0 &&
> die "ERROR: libcelt must be installed and
> version must be >= 0.11.0."; }
>  enabled libcaca   && require_pkg_config libcaca caca caca.h
> caca_create_canvas
>  enabled libcodec2 && require libcodec2 codec2/codec2.h
> codec2_create -lcodec2
> -enabled libdav1d  && require_pkg_config libdav1d "dav1d >= 0.1.0"
> "dav1d/dav1d.h" dav1d_version
> +enabled libdav1d  && require_pkg_config libdav1d "dav1d >= 0.2.0"
> "dav1d/dav1d.h" dav1d_version
>  enabled libdavs2  && require_pkg_config libdavs2 "davs2 >= 1.6.0"
> davs2.h davs2_decoder_open
>  enabled libdc1394 && require_pkg_config libdc1394 libdc1394-2
> dc1394/dc1394.h dc1394_new
>  enabled libdrm&& require_pkg_config libdrm libdrm xf86drm.h
> drmGetVersion
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index ed02da4ebf..459bbae687 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -37,6 +37,13 @@ typedef struct Libdav1dContext {
>  int apply_grain;
>  } Libdav1dContext;
>
> +static void libdav1d_log_callback(void *opaque, const char *fmt, va_list
> vl)
> +{
> +AVCodecContext *c = opaque;
> +
> +av_vlog(c, AV_LOG_ERROR, fmt, vl);
> +}
> +
>  static av_cold int libdav1d_init(AVCodecContext *c)
>  {
>  Libdav1dContext *dav1d = c->priv_data;
> @@ -46,6 +53,8 @@ static av_cold int libdav1d_init(AVCodecContext *c)
>  av_log(c, AV_LOG_INFO, "libdav1d %s\n", dav1d_version());
>
>  dav1d_default_settings();
> +s.logger.cookie = c;
> +s.logger.callback = libdav1d_log_callback;
>  s.n_tile_threads = dav1d->tile_threads;
>  s.apply_grain = dav1d->apply_grain;
>  s.n_frame_threads = FFMIN(c->thread_count ? c->thread_count :
> av_cpu_count(), DAV1D_MAX_FRAME_THREADS);
> --
> 2.21.0
>

set seems good to me
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v6 1/2] lavc/svt_hevc: add libsvt hevc encoder wrapper.

2019-03-05 Thread Vittorio Giovara
On Tue, Mar 5, 2019 at 1:45 AM Jing SUN  wrote:

> From: Jing Sun 
>
> base on patch by Huang, Zhengxu from https://github.com/intel/SVT-HEVC
>
> V4: - Fix the build error with new API in PR#52
> - Fix the encoding hang issue by API change in PR#52
> - Fix the last frame dropping issue
> - Fix the invalid parameter causing segmentation fault issue
> - Add the support to svt hevc and av1 plugins coexistance
> - Add the VMAF optimized mode to "-tune"
> - Add the "-hdr" parameter
>

Is there any way to preserve the color matrix/primary/transfer properties
during encoding?
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] libdav1d: Add support for reading hdr10 metadata

2019-03-05 Thread Vittorio Giovara
---
 configure |  2 +-
 libavcodec/libdav1d.c | 30 +-
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index dcead3a300..a5cef4bc09 100755
--- a/configure
+++ b/configure
@@ -6142,7 +6142,7 @@ enabled libcelt   && require libcelt celt/celt.h 
celt_decode -lcelt0 &&
die "ERROR: libcelt must be installed and 
version must be >= 0.11.0."; }
 enabled libcaca   && require_pkg_config libcaca caca caca.h 
caca_create_canvas
 enabled libcodec2 && require libcodec2 codec2/codec2.h codec2_create 
-lcodec2
-enabled libdav1d  && require_pkg_config libdav1d "dav1d >= 0.1.0" 
"dav1d/dav1d.h" dav1d_version
+enabled libdav1d  && require_pkg_config libdav1d "dav1d >= 0.2.0" 
"dav1d/dav1d.h" dav1d_version
 enabled libdavs2  && require_pkg_config libdavs2 "davs2 >= 1.6.0" 
davs2.h davs2_decoder_open
 enabled libdc1394 && require_pkg_config libdc1394 libdc1394-2 
dc1394/dc1394.h dc1394_new
 enabled libdrm&& require_pkg_config libdrm libdrm xf86drm.h 
drmGetVersion
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index ed02da4ebf..355dd184f4 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -22,6 +22,7 @@
 #include 
 
 #include "libavutil/avassert.h"
+#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/opt.h"
 
 #include "avcodec.h"
@@ -90,7 +91,7 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame 
*frame)
 Libdav1dContext *dav1d = c->priv_data;
 Dav1dData *data = >data;
 Dav1dPicture *p;
-int res;
+int i, res;
 
 if (!data->sz) {
 AVPacket pkt = { 0 };
@@ -206,6 +207,33 @@ FF_ENABLE_DEPRECATION_WARNINGS
 return AVERROR_INVALIDDATA;
 }
 
+if (p->mastering_display) {
+AVMasteringDisplayMetadata *mastering = 
av_mastering_display_metadata_create_side_data(frame);
+if (!mastering)
+return AVERROR(ENOMEM);
+
+for (i = 0; i < 3; i++) {
+mastering->display_primaries[i][0] = 
av_make_q(p->mastering_display->primaries[i][0], 1 << 16);
+mastering->display_primaries[i][1] = 
av_make_q(p->mastering_display->primaries[i][1], 1 << 16);
+}
+mastering->white_point[0] = 
av_make_q(p->mastering_display->white_point[0], 1 << 16);
+mastering->white_point[1] = 
av_make_q(p->mastering_display->white_point[1], 1 << 16);
+
+mastering->max_luminance = 
av_make_q(p->mastering_display->max_luminance, 1 << 8);
+mastering->min_luminance = 
av_make_q(p->mastering_display->min_luminance, 1 << 14);
+
+mastering->has_primaries = 1;
+mastering->has_luminance = 1;
+}
+if (p->content_light) {
+AVContentLightMetadata *light = 
av_content_light_metadata_create_side_data(frame);
+if (!light)
+return AVERROR(ENOMEM);
+
+light->MaxCLL = p->content_light->max_content_light_level;
+light->MaxFALL = p->content_light->max_frame_average_light_level;
+}
+
 return 0;
 }
 
-- 
2.20.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avformat/mov: fix start_time for streams with empty edits in the middle

2019-02-25 Thread Vittorio Giovara
On Sat, Feb 23, 2019 at 1:09 PM Vittorio Giovara 
wrote:

> Empty edits can occur at any position within the edit list except for at
> the end. Empty edits in the middle should not impact the reported stream
> start_time or the video PTS adjustment, so only include empty edits at
> the start of the list in empty_edits_sum_duration.
>
> Please see attachment.
>

Any objections to pushing this later today?
Thanks
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avformat/mov: fix start_time for streams with empty edits in the middle

2019-02-23 Thread Vittorio Giovara
Empty edits can occur at any position within the edit list except for at
the end. Empty edits in the middle should not impact the reported stream
start_time or the video PTS adjustment, so only include empty edits at
the start of the list in empty_edits_sum_duration.

Please see attachment.
-- 
Vittorio


0001-avformat-mov-fix-start_time-for-streams-with-empty-e.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] http: Do not try to make a new request when seeking past the end of the file

2019-02-22 Thread Vittorio Giovara
On Wed, Feb 20, 2019 at 9:54 AM Vittorio Giovara 
wrote:

> From: Justin Ruggles 
>
> This avoids making invalid HTTP Range requests for a byte range past the
> known end of the file during a seek. Those requests generally return a HTTP
> response of 416 Range Not Satisfiable, which causes an error response.
>
> Reference: https://tools.ietf.org/html/rfc7233
>
> Signed-off-by: Vittorio Giovara 
> ---
>  libavformat/http.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index a0a0636cf2..1e40268599 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -1691,6 +1691,13 @@ static int64_t http_seek_internal(URLContext *h,
> int64_t off, int whence, int fo
>  if (s->off && h->is_streamed)
>  return AVERROR(ENOSYS);
>
> +/* do not try to make a new connection if seeking past the end of the
> file */
> +if (s->end_off || s->filesize != UINT64_MAX) {
> +uint64_t end_pos = s->end_off ? s->end_off : s->filesize;
> +if (s->off >= end_pos)
> +return s->off;
> +}
> +
>  /* we save the old context in case the seek fails */
>  old_buf_size = s->buf_end - s->buf_ptr;
>  memcpy(old_buf, s->buf_ptr, old_buf_size);
> --
> 2.20.1
>
>
If no objections, I'd like to push this to master later today.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] http: Do not try to make a new request when seeking past the end of the file

2019-02-20 Thread Vittorio Giovara
From: Justin Ruggles 

This avoids making invalid HTTP Range requests for a byte range past the
known end of the file during a seek. Those requests generally return a HTTP
response of 416 Range Not Satisfiable, which causes an error response.

Reference: https://tools.ietf.org/html/rfc7233

Signed-off-by: Vittorio Giovara 
---
 libavformat/http.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/libavformat/http.c b/libavformat/http.c
index a0a0636cf2..1e40268599 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1691,6 +1691,13 @@ static int64_t http_seek_internal(URLContext *h, int64_t 
off, int whence, int fo
 if (s->off && h->is_streamed)
 return AVERROR(ENOSYS);
 
+/* do not try to make a new connection if seeking past the end of the file 
*/
+if (s->end_off || s->filesize != UINT64_MAX) {
+uint64_t end_pos = s->end_off ? s->end_off : s->filesize;
+if (s->off >= end_pos)
+return s->off;
+}
+
 /* we save the old context in case the seek fails */
 old_buf_size = s->buf_end - s->buf_ptr;
 memcpy(old_buf, s->buf_ptr, old_buf_size);
-- 
2.20.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avformat/http: Do not try to make a new request when seeking past the end of the file

2019-02-15 Thread Vittorio Giovara
From: Justin Ruggles 

This avoids making invalid HTTP Range requests for a byte range past the
known end of the file during a seek. Those requests generally return a HTTP
response of 416 Range Not Satisfiable, which causes an error response.

Reference: https://tools.ietf.org/html/rfc7233

Signed-off-by: Vittorio Giovara 
---
 libavformat/http.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/libavformat/http.c b/libavformat/http.c
index a0a0636cf2..1e40268599 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -1691,6 +1691,13 @@ static int64_t http_seek_internal(URLContext *h,
int64_t off, int whence, int fo
 if (s->off && h->is_streamed)
 return AVERROR(ENOSYS);

+/* do not try to make a new connection if seeking past the end of the
file */
+if (s->end_off || s->filesize != UINT64_MAX) {
+uint64_t end_pos = s->end_off ? s->end_off : s->filesize;
+if (s->off >= end_pos)
+return s->off;
+}
+
 /* we save the old context in case the seek fails */
 old_buf_size = s->buf_end - s->buf_ptr;
 memcpy(old_buf, s->buf_ptr, old_buf_size);
-- 
2.14.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: change the assert in trailing_bits to accept zero bits when reading

2019-02-13 Thread Vittorio Giovara
On Sun, Feb 10, 2019 at 3:12 PM James Almer  wrote:

> If nb_bits is zero when reading an OBU, then it's not a bug in CBS but an
> invalid bitstream, and we should abort gracefully instead.
>
> Signed-off-by: James Almer 
> ---
> rav1e is currently encoding invalid Metadata OBUs without trailing bits,
> which
> are triggering the assert when parsed by CBS. This change makes sure to
> instead
> report the bitstream as invalid and gracefully return with an error code
> instead of crashing.
>

In case anyone is interested, this got fixed tonight, rav1e can produce
conformant bitstreams when encoding metadata now.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] SDR->HDR tone mapping algorithm?

2019-02-08 Thread Vittorio Giovara
On Fri, Feb 8, 2019 at 3:22 AM Harish Krupo 
wrote:

> Hello,
>
> We are in the process of implementing HDR rendering support in the
> Weston display compositor [1] (HDR discussion here [2]). When HDR
> and SDR surfaces like a video buffer and a subtitle buffer are presented
> together, the composition would take place as follows:
> - If the display does not support HDR metadata:
>   in-coming HDR surfaces would be tone mapped using opengl to SDR and
>   blended with the other SDR surfaces. We are currently using the Hable
>   operator for tone mapping.
> - If the display supports setting HDR metadata:
>   SDR surfaces would be tone mapped to HDR and blended with HDR surfaces.
>
> The literature available for SDR->HDR tone mapping varies from simple
> linear expansion of luminance to CNN based approaches. We wanted to know
> your recommendations for an acceptable algorithm for SDR->HDR tone mapping.
>
> Any help is greatly appreciated!
>
> [1] https://gitlab.freedesktop.org/wayland/weston
> [2]
> https://lists.freedesktop.org/archives/wayland-devel/2019-January/039809.html
>
> Thank you
> Regards
> Harish Krupo
>

In *theory* the tonemapping functions should be reversible, so if you use
vf_tonemap or vf_tonemap_opencl and properly expand the range via zimg
(vf_zscale) before compression it should work fine. However I have never
tried it myself, so I cannot guarantee that those filters will work as is.
Of course haasn from the libplacebo project might have better suggestions,
so you should really reach out to him.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-22 Thread Vittorio Giovara
On Mon, Jan 21, 2019 at 2:15 PM FeRD  wrote:

> On Mon, Jan 21, 2019 at 1:55 PM Moritz Barsnick  wrote:
>
> > On Mon, Jan 21, 2019 at 12:38:58 -0500, FeRD (Frank Dana) wrote:
> >
> > > After applying both patches, 'make fate' succeeds and ffmpeg is still
> > > functional.
> >
> > You're not allowed to break fate (or compilation). So the two pathes
> > need to be merged.
>
>
> Aha, thanks. I'll resubmit squashed into a single patch.
>

maybe it would be a good opportunity to expose the symbol publicly, and
prefix it with AV_ instead of FF_, like it was done for AV_CEIL_RSHIFT
(21f946840260da150affd9a20cc35b3d56194ba6)
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/rscc: Avoid returning frames that have nearly no undamaged pixels in them

2019-01-18 Thread Vittorio Giovara
On Thu, Jan 17, 2019 at 6:34 PM Michael Niedermayer 
wrote:

> On Wed, Jan 16, 2019 at 09:05:18PM -0500, Vittorio Giovara wrote:
> > On Wed, Jan 16, 2019 at 7:44 PM Michael Niedermayer
> 
> > wrote:
> >
> > > Fixes: Timeout
> > > Fixes:
> > >
> 12192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> > >
> > > Before:
> > >
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> > > in 15423 ms
> > > After:
> > >
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> > > in 190 ms
> > >
> > > Found-by: continuous fuzzing process
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by
> > > <
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by
> >:
> > > Michael Niedermayer 
> > > ---
> > >  libavcodec/rscc.c | 7 ++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/rscc.c b/libavcodec/rscc.c
> > > index 7921f149ed..fa066afd7f 100644
> > > --- a/libavcodec/rscc.c
> > > +++ b/libavcodec/rscc.c
> > > @@ -64,6 +64,7 @@ typedef struct RsccContext {
> > >  /* zlib interaction */
> > >  uint8_t *inflated_buf;
> > >  uLongf inflated_size;
> > > +int valid_pixels
> > >  } RsccContext;
> > >
> > >  static av_cold int rscc_init(AVCodecContext *avctx)
> > > @@ -348,7 +349,11 @@ static int rscc_decode_frame(AVCodecContext
> *avctx,
> > > void *data,
> > >  memcpy (frame->data[1], ctx->palette, AVPALETTE_SIZE);
> > >  }
> > >
> > > -*got_frame = 1;
> > > +// We only return a picture when more than 5% is undameged, this
> > > avoids copying nearly broken frames around
> > > +if (ctx->valid_pixels < ctx->inflated_size)
> > > +ctx->valid_pixels += pixel_size;
> > > +if (ctx->valid_pixels >= ctx->inflated_size/20)
> > >
> >
> > this feels arbitrary and hackish, for very little gain, IMO
>
> Would you be ok with rejecting RSCC files without a keyframe ?
> or more precissely all frames before a keyframe and thus if there is
> no keyframe the whole file
> (that would be a superset of what this patch rejects)
>

If that could be achieved in a clean way without codec-specific additions,
sure why not. It could maybe exploit the AV_FRAME_FLAG_CORRUPT flag and
some other combination of codec flag to expose this feature. However I'm
still adamant at "fixing" issues in the code found by external code tools
in this way: at most it is the fuzzer that should be better instrumented to
accept longer or no timeouts for this codec.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified

2019-01-18 Thread Vittorio Giovara
On Fri, Jan 18, 2019 at 6:43 AM Carl Eugen Hoyos  wrote:

> 2019-01-18 4:48 GMT+01:00, Neil Birkbeck :
> > On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck 
> > wrote:
> >
> >> This allows preservation of color values set from the container,
> >> while still letting the bitstream take precedent when its values
> >> are specified to some actual value (e.g., not *UNSPECIFIED).
> >>
> >> Signed-off-by: Neil Birkbeck 
> >> ---
> >>  libavcodec/proresdec2.c | 9 ++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> >> index 6209c229c9..662ca7d48b 100644
> >> --- a/libavcodec/proresdec2.c
> >> +++ b/libavcodec/proresdec2.c
> >> @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx,
> >> const uint8_t *buf,
> >>  }
> >>  }
> >>
> >> -avctx->color_primaries = buf[14];
> >> -avctx->color_trc   = buf[15];
> >> -avctx->colorspace  = buf[16];
> >> +if (buf[14] != AVCOL_PRI_UNSPECIFIED)
> >> +avctx->color_primaries = buf[14];
> >> +if (buf[15] != AVCOL_TRC_UNSPECIFIED)
> >> +avctx->color_trc   = buf[15];
> >> +if (buf[16] != AVCOL_SPC_UNSPECIFIED)
> >> +avctx->colorspace  = buf[16];
> >>  avctx->color_range = AVCOL_RANGE_MPEG;
> >>
> >>  ptr   = buf + 20;
> >> --
> >> 2.20.1.321.g9e740568ce-goog
> >>
> >>
> > To add a bit more context. The patch is a fix for the case when prores
> > bitstream code points are explicitly set to unspecified and are
> overriding
> > what may have been in the container (unlike h264/h265 where such values
> can
> > behind the color_description flag, these fields always must be present in
> > the prores header).
>
> Isn't this even more of a reason to prefer the container value over
> the bitstream value?
>

Absolutely not
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/proresdec2: only set avctx->color* when values are specified

2019-01-18 Thread Vittorio Giovara
On Thu, Jan 17, 2019 at 10:57 PM Neil Birkbeck 
wrote:

> On Thu, Jan 17, 2019 at 7:43 PM Neil Birkbeck 
> wrote:
>
> > This allows preservation of color values set from the container,
> > while still letting the bitstream take precedent when its values
> > are specified to some actual value (e.g., not *UNSPECIFIED).
> >
> > Signed-off-by: Neil Birkbeck 
> > ---
> >  libavcodec/proresdec2.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
> > index 6209c229c9..662ca7d48b 100644
> > --- a/libavcodec/proresdec2.c
> > +++ b/libavcodec/proresdec2.c
> > @@ -262,9 +262,12 @@ static int decode_frame_header(ProresContext *ctx,
> > const uint8_t *buf,
> >  }
> >  }
> >
> > -avctx->color_primaries = buf[14];
> > -avctx->color_trc   = buf[15];
> > -avctx->colorspace  = buf[16];
> > +if (buf[14] != AVCOL_PRI_UNSPECIFIED)
> > +avctx->color_primaries = buf[14];
> > +if (buf[15] != AVCOL_TRC_UNSPECIFIED)
> > +avctx->color_trc   = buf[15];
> > +if (buf[16] != AVCOL_SPC_UNSPECIFIED)
> > +avctx->colorspace  = buf[16];
> >  avctx->color_range = AVCOL_RANGE_MPEG;
> >
> >  ptr   = buf + 20;
> > --
> > 2.20.1.321.g9e740568ce-goog
> >
> >
> To add a bit more context. The patch is a fix for the case when prores
> bitstream code points are explicitly set to unspecified and are overriding
> what may have been in the container (unlike h264/h265 where such values can
> behind the color_description flag, these fields always must be present in
> the prores header). Of course, ideally these should be the same.
>
> We noticed this inconsistency on some HDR content, as prior to
>
> https://github.com/FFmpeg/FFmpeg/commit/81d78fe13bc1fd94845c002f3439fe44d9e9e0d9
> the color information in the prores bitstream (which may have been
> unspecified) was simply ignored.
>
> In practice, I guess some workflows may not have known about the new code
> points and put unspecified in the bitstream (or worse where some headers
> will contain non-HDR code points).
>
> The patch seemed like a situation where we could resolve the inconsistency
> in favor of the container (given my understanding, and from what I see in
> the code, I'm assuming the codec is typically given preference). But I'm
> interested in hearing ffmpeg-devel's opinions on whether this is most
> consistent (actually, for the HDR files that I've seen, the container is
> correct--although I'm sure there are cases where the opposite is true).
>
> I guess the most closely related discussion about codecs overriding these
> types of values is here
> https://patchwork.ffmpeg.org/patch/11279/
> but this case seemed a bit different.
>

This makes a lot of sense, but it should be part of the commit message
instead of the attached thread.
So ok with me.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/rscc: Avoid returning frames that have nearly no undamaged pixels in them

2019-01-16 Thread Vittorio Giovara
On Wed, Jan 16, 2019 at 7:44 PM Michael Niedermayer 
wrote:

> Fixes: Timeout
> Fixes:
> 12192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
>
> Before:
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> in 15423 ms
> After:
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RSCC_fuzzer-6279038004363264
> in 190 ms
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by
> :
> Michael Niedermayer 
> ---
>  libavcodec/rscc.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/rscc.c b/libavcodec/rscc.c
> index 7921f149ed..fa066afd7f 100644
> --- a/libavcodec/rscc.c
> +++ b/libavcodec/rscc.c
> @@ -64,6 +64,7 @@ typedef struct RsccContext {
>  /* zlib interaction */
>  uint8_t *inflated_buf;
>  uLongf inflated_size;
> +int valid_pixels
>  } RsccContext;
>
>  static av_cold int rscc_init(AVCodecContext *avctx)
> @@ -348,7 +349,11 @@ static int rscc_decode_frame(AVCodecContext *avctx,
> void *data,
>  memcpy (frame->data[1], ctx->palette, AVPALETTE_SIZE);
>  }
>
> -*got_frame = 1;
> +// We only return a picture when more than 5% is undameged, this
> avoids copying nearly broken frames around
> +if (ctx->valid_pixels < ctx->inflated_size)
> +ctx->valid_pixels += pixel_size;
> +if (ctx->valid_pixels >= ctx->inflated_size/20)
>

this feels arbitrary and hackish, for very little gain, IMO
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] matroskadec: Adjust content light side data check

2019-01-13 Thread Vittorio Giovara
While in practice both fields are always initialized, this mimics
what other tools like ffms2, and x265 do more closely.

This work has been sponsored by Tyrell Corporation, for a compensation
of dozen of cents of US dollars.
---
 libavformat/matroskadec.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 4ad99db7db..3ff3516c24 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -1877,7 +1877,7 @@ static int mkv_parse_video_color(AVStream *st, const 
MatroskaTrack *track) {
 avcodec_chroma_pos_to_enum((color->chroma_siting_horz - 1) << 7,
(color->chroma_siting_vert - 1) << 7);
 }
-if (color->max_cll && color->max_fall) {
+if (color->max_cll || color->max_fall) {
 size_t size = 0;
 int ret;
 AVContentLightMetadata *metadata = 
av_content_light_metadata_alloc();
@@ -1891,6 +1891,9 @@ static int mkv_parse_video_color(AVStream *st, const 
MatroskaTrack *track) {
 }
 metadata->MaxCLL  = color->max_cll;
 metadata->MaxFALL = color->max_fall;
+av_log(NULL, AV_LOG_INFO, "Content Light Level Metadata, "
+   "MaxCLL=%d, MaxFALL=%d",
+   metadata->MaxCLL, metadata->MaxFALL);
 }
 
 if (has_mastering_primaries || has_mastering_luminance) {
@@ -3552,6 +3555,8 @@ static int matroska_read_seek(AVFormatContext *s, int 
stream_index,
 AVStream *st = s->streams[stream_index];
 int i, index, index_min;
 
+flags ^= AVSEEK_FLAG_ANY;
+
 /* Parse the CUES now since we need the index data to seek. */
 if (matroska->cues_parsing_deferred > 0) {
 matroska->cues_parsing_deferred = 0;
-- 
2.20.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V5 1/2] avutil: add ROI (Region Of Interest) data struct and bump version

2019-01-07 Thread Vittorio Giovara
On Sat, Jan 5, 2019 at 2:15 AM James Almer  wrote:

> On 1/4/2019 10:08 AM, Vittorio Giovara wrote:
> > On Fri, Jan 4, 2019 at 12:22 PM Nicolas George  wrote:
> >
> >> Rostislav Pehlivanov (12019-01-04):
> >>>> +typedef struct AVRegionOfInterest {
> >>>> +size_t self_size;
> >>>> +size_t top;
> >>>> +size_t bottom;
> >>>> +size_t left;
> >>>> +size_t right;
> >>> I'd still much rather have uints with fixed sizes than these platform
> >>> dependent types.
> >>
> >> Guo, Yejun said:
> >>
> >>>> I usually choose 'size_t' for the meanings with length/size.
> >>
> >> But that is a mistake. size_t is for length/size of objects in memory,
> >> not any length/size.
> >>
> >> These numbers, unless I am mistaken, are coordinates within an AVFrame.
> >> In that case, the only correct type is the same as AVFrame.width and
> >> AVFrame.height.
> >>
> >
> > I personally disagree, what are coordinates within an AVFrame if not the
> > length/size of an object in memory?
> > A buffer containing video data is still an object in memory after all, so
> > IMHO using size_t makes a lot of sense here.
>
> We already did size_t for AVSphericalMapping and had to change them to
> uint32_t.
> size_t is not the correct type at all, as video dimensions are not
> system dependent, and it will generate the same issues we already
> experienced with AVSphericalMapping in fate tests.
>
>
Just for the sake of information, the issue with the spherical fate tests
is that some mov tests included the side data size which was thankfully
removed because it did not made any sense. Even then, the
AVSphericalMapping structure was changed only to make it more similar to
the specification -- this is not the case here: size_t represents a length
so using it for video dimensions makes perfect sense. That being said, use
whatever type you want, as long as it's correctly documented and argumented.
Regards
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V5 1/2] avutil: add ROI (Region Of Interest) data struct and bump version

2019-01-04 Thread Vittorio Giovara
On Fri, Jan 4, 2019 at 7:57 PM Rostislav Pehlivanov 
wrote:

> On Fri, 4 Jan 2019 at 16:28, Vittorio Giovara 
> wrote:
>
> > On Fri, Jan 4, 2019 at 2:37 PM Nicolas George  wrote:
> >
> > > Vittorio Giovara (12019-01-04):
> > > > I personally disagree, what are coordinates within an AVFrame if not
> > the
> > > > length/size of an object in memory?
> > >
> > > That would be an argument for making AVFrame.width and AVFrame.height
> > > size_t. But they are not, and therefore these ROI values have no reason
> > > to be either. There is no point in being able to express ROI
> coordinates
> > > in the quadrillion when the size of the frame is bounded by much less.
> > >
> >
> > That seems a poor argument since the code base is so old that there are a
> > plethora of bad design decisions that should not dictate what choices are
> > made now.
> >
>
> Hence we should avoid making a new one now.


Right, we should do things that make sense and argument them with something
more than saying "it's confusing".

Pixels shouldn't have the size_t type, it just makes things confusing.
>
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V5 1/2] avutil: add ROI (Region Of Interest) data struct and bump version

2019-01-04 Thread Vittorio Giovara
On Fri, Jan 4, 2019 at 2:37 PM Nicolas George  wrote:

> Vittorio Giovara (12019-01-04):
> > I personally disagree, what are coordinates within an AVFrame if not the
> > length/size of an object in memory?
>
> That would be an argument for making AVFrame.width and AVFrame.height
> size_t. But they are not, and therefore these ROI values have no reason
> to be either. There is no point in being able to express ROI coordinates
> in the quadrillion when the size of the frame is bounded by much less.
>

That seems a poor argument since the code base is so old that there are a
plethora of bad design decisions that should not dictate what choices are
made now.
size_t really seems the right choice here, and since it's the one chosen by
the author of the patch (revision 5) I think we should respect that.
Using AVRational instead of float for qoffset has the added benefit of
making the field encoder independent, so it's a sensible change, while
using uint32 instead of sizet does not bring anything to the table, as far
as I can tell, so let's not ask Yejun to change it.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V5 1/2] avutil: add ROI (Region Of Interest) data struct and bump version

2019-01-04 Thread Vittorio Giovara
On Fri, Jan 4, 2019 at 12:22 PM Nicolas George  wrote:

> Rostislav Pehlivanov (12019-01-04):
> > > +typedef struct AVRegionOfInterest {
> > > +size_t self_size;
> > > +size_t top;
> > > +size_t bottom;
> > > +size_t left;
> > > +size_t right;
> > I'd still much rather have uints with fixed sizes than these platform
> > dependent types.
>
> Guo, Yejun said:
>
> >> I usually choose 'size_t' for the meanings with length/size.
>
> But that is a mistake. size_t is for length/size of objects in memory,
> not any length/size.
>
> These numbers, unless I am mistaken, are coordinates within an AVFrame.
> In that case, the only correct type is the same as AVFrame.width and
> AVFrame.height.
>

I personally disagree, what are coordinates within an AVFrame if not the
length/size of an object in memory?
A buffer containing video data is still an object in memory after all, so
IMHO using size_t makes a lot of sense here.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V4 1/2] avutil: add ROI data struct and bump version

2019-01-02 Thread Vittorio Giovara
On Wed, Jan 2, 2019 at 6:48 PM James Almer  wrote:

> On 12/28/2018 7:09 AM, Guo, Yejun wrote:
> > The encoders such as libx264 support different QPs offset for different
> MBs,
> > it makes possible for ROI-based encoding. It makes sense to add support
> > within ffmpeg to generate/accept ROI infos and pass into encoders.
> >
> > Typical usage: After AVFrame is decoded, a ffmpeg filter or user's code
> > generates ROI info for that frame, and the encoder finally does the
> > ROI-based encoding.
> >
> > The ROI info is maintained as side data of AVFrame.
> >
> > Signed-off-by: Guo, Yejun 
> > ---
> >  libavutil/frame.c   |  1 +
> >  libavutil/frame.h   | 23 +++
> >  libavutil/version.h |  2 +-
> >  3 files changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index 34a6210..bebc50e 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -841,6 +841,7 @@ const char *av_frame_side_data_name(enum
> AVFrameSideDataType type)
> >  case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table
> data";
> >  #endif
> >  case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata
> SMPTE2094-40 (HDR10+)";
> > +case AV_FRAME_DATA_ROIS: return "Regions Of Interest";
> >  }
> >  return NULL;
> >  }
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index 582ac47..3460b01 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -173,6 +173,12 @@ enum AVFrameSideDataType {
> >   * volume transform - application 4 of SMPTE 2094-40:2016 standard.
> >   */
> >  AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
> > +
> > +/**
> > + * Regions Of Interest, the data is an array of AVROI type, the
> array size
> > + * is implied by AVFrameSideData::size / sizeof(AVROI).
> > + */
> > +AV_FRAME_DATA_ROIS,
> >  };
> >
> >  enum AVActiveFormatDescription {
> > @@ -201,6 +207,23 @@ typedef struct AVFrameSideData {
> >  } AVFrameSideData;
> >
> >  /**
> > + * Structure to hold Region Of Interest.
> > + *
> > + * top/bottom/left/right are coordinates at frame pixel level.
> > + * They will be extended internally if the codec requires an alignment.
> > + * If the regions overlap, the last value in the list will be used.
> > + *
> > + * qoffset is quant offset, it is encoder dependent.
> > + */
> > +typedef struct AVROI {
> > +size_t top;
> > +size_t bottom;
> > +size_t left;
> > +size_t right;
>
> uint32_t, please. Make the struct have a fixed size so we don't repeat
> the same issues we had with fate tests and AVSphericalMapping.
>

I thought we dropped the side data size from fate tests in
21a8e751ad6abb2d423afa3041da92f8f7741997.
If so, size_t seems the better choice here.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V4 1/2] avutil: add ROI data struct and bump version

2019-01-02 Thread Vittorio Giovara
On Wed, Jan 2, 2019 at 6:45 PM James Almer  wrote:

> On 1/2/2019 2:18 PM, Vittorio Giovara wrote:
> > On Wed, Jan 2, 2019 at 4:13 PM Vittorio Giovara <
> vittorio.giov...@gmail.com>
> > wrote:
> >
> >>
> >>
> >> On Fri, Dec 28, 2018 at 3:17 AM Guo, Yejun  wrote:
> >>
> >
> > AVRegionOfInterest {
> >  size_t top/left/right/bottom
> > }
> >
> > AVRegionOfInterestSet {
> >  int rois_nb;
> >  AVRegionOfInterest *rois;
>
> This will go south as soon as you start copying, referencing and freeing
> frames and/or frame side data.
>
> All side data types need to be a contiguous array of bytes in memory
> with no pointers.
>

Hmm you're right, but embedding an entire array is pretty bad too,
especially because it locks the struct size...
Do you have any alternative ideas for this?
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V4 1/2] avutil: add ROI data struct and bump version

2019-01-02 Thread Vittorio Giovara
On Wed, Jan 2, 2019 at 4:13 PM Vittorio Giovara 
wrote:

>
>
> On Fri, Dec 28, 2018 at 3:17 AM Guo, Yejun  wrote:
>
>> The encoders such as libx264 support different QPs offset for different
>> MBs,
>> it makes possible for ROI-based encoding. It makes sense to add support
>> within ffmpeg to generate/accept ROI infos and pass into encoders.
>>
>> Typical usage: After AVFrame is decoded, a ffmpeg filter or user's code
>> generates ROI info for that frame, and the encoder finally does the
>> ROI-based encoding.
>>
>> The ROI info is maintained as side data of AVFrame.
>>
>> Signed-off-by: Guo, Yejun 
>> ---
>>  libavutil/frame.c   |  1 +
>>  libavutil/frame.h   | 23 +++
>>  libavutil/version.h |  2 +-
>>  3 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>> index 34a6210..bebc50e 100644
>> --- a/libavutil/frame.c
>> +++ b/libavutil/frame.c
>> @@ -841,6 +841,7 @@ const char *av_frame_side_data_name(enum
>> AVFrameSideDataType type)
>>  case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table
>> data";
>>  #endif
>>  case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata
>> SMPTE2094-40 (HDR10+)";
>> +case AV_FRAME_DATA_ROIS: return "Regions Of Interest";
>>  }
>>  return NULL;
>>  }
>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>> index 582ac47..3460b01 100644
>> --- a/libavutil/frame.h
>> +++ b/libavutil/frame.h
>> @@ -173,6 +173,12 @@ enum AVFrameSideDataType {
>>   * volume transform - application 4 of SMPTE 2094-40:2016 standard.
>>   */
>>  AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
>> +
>> +/**
>> + * Regions Of Interest, the data is an array of AVROI type, the
>> array size
>> + * is implied by AVFrameSideData::size / sizeof(AVROI).
>> + */
>> +AV_FRAME_DATA_ROIS,
>>
>
> Any chance i could convince you of unfolding this acronym into
> AV_FRAME_REGIONS_OF_INTEREST (and AVRegionOfInterest)?
> When I first read it I thought you were talking about Return of
> Investments or Request of Invention, which were mildly confusing.
>
> The "AVFrameSideData::size" is a C++-ism, could you please use
> "AVFrameSideData.size" like elsewhere in the header?
>
> You should probably document that sizeof() of this struct is part of the
> public ABI.
>

After thinking some more about this, it's probably a bad idea to embed an
array in a side data. Not only it constrains the structure to only change
at major bumps, but it seems very reminiscent of binary side data which
is/should be not used for newer side data. As a matter of fact side data
normally hosts either structs or simple types like ints or enums only.
Instead of this, why not creating a structure hosting the number of
elements and a pointer? Something like

AVRegionOfInterest {
 size_t top/left/right/bottom
}

AVRegionOfInterestSet {
 int rois_nb;
 AVRegionOfInterest *rois;
}
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V4 1/2] avutil: add ROI data struct and bump version

2019-01-02 Thread Vittorio Giovara
On Fri, Dec 28, 2018 at 3:17 AM Guo, Yejun  wrote:

> The encoders such as libx264 support different QPs offset for different
> MBs,
> it makes possible for ROI-based encoding. It makes sense to add support
> within ffmpeg to generate/accept ROI infos and pass into encoders.
>
> Typical usage: After AVFrame is decoded, a ffmpeg filter or user's code
> generates ROI info for that frame, and the encoder finally does the
> ROI-based encoding.
>
> The ROI info is maintained as side data of AVFrame.
>
> Signed-off-by: Guo, Yejun 
> ---
>  libavutil/frame.c   |  1 +
>  libavutil/frame.h   | 23 +++
>  libavutil/version.h |  2 +-
>  3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 34a6210..bebc50e 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -841,6 +841,7 @@ const char *av_frame_side_data_name(enum
> AVFrameSideDataType type)
>  case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table
> data";
>  #endif
>  case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata
> SMPTE2094-40 (HDR10+)";
> +case AV_FRAME_DATA_ROIS: return "Regions Of Interest";
>  }
>  return NULL;
>  }
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 582ac47..3460b01 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -173,6 +173,12 @@ enum AVFrameSideDataType {
>   * volume transform - application 4 of SMPTE 2094-40:2016 standard.
>   */
>  AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
> +
> +/**
> + * Regions Of Interest, the data is an array of AVROI type, the array
> size
> + * is implied by AVFrameSideData::size / sizeof(AVROI).
> + */
> +AV_FRAME_DATA_ROIS,
>

Any chance i could convince you of unfolding this acronym into
AV_FRAME_REGIONS_OF_INTEREST (and AVRegionOfInterest)?
When I first read it I thought you were talking about Return of Investments
or Request of Invention, which were mildly confusing.

The "AVFrameSideData::size" is a C++-ism, could you please use
"AVFrameSideData.size" like elsewhere in the header?

You should probably document that sizeof() of this struct is part of the
public ABI.


>  };
>
>  enum AVActiveFormatDescription {
> @@ -201,6 +207,23 @@ typedef struct AVFrameSideData {
>  } AVFrameSideData;
>
>  /**
> + * Structure to hold Region Of Interest.
> + *
> + * top/bottom/left/right are coordinates at frame pixel level.
>

what does  "pixel level" mean? May I suggest better wording?

Number of pixels to discard from the the top/bottom/left/right border
of the frame to obtain the region of interest of the frame.

+ * They will be extended internally if the codec requires an alignment.
> + * If the regions overlap, the last value in the list will be used.
>

Isn't this encoder dependent too?


> + *
> + * qoffset is quant offset, it is encoder dependent.

+ */
> +typedef struct AVROI {
> +size_t top;
> +size_t bottom;
> +size_t left;
> +size_t right;
> +int qoffset;
>

so int instead of float?

+} AVROI;
> +
> +/**
>   * This structure describes decoded (raw) audio or video data.
>   *
>   * AVFrame must be allocated using av_frame_alloc(). Note that this only
> diff --git a/libavutil/version.h b/libavutil/version.h
> index f997615..1fcdea9 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>   */
>
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.

2018-12-21 Thread Vittorio Giovara
On Thu, Dec 20, 2018 at 8:14 PM Mohammad Izadi  wrote:

> From: Mohammad Izadi 
>
> The dynamic metadata contains data for color volume transform -
> application 4 of SMPTE 2094-40:2016 standard. The data comes from HEVC in
> the SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35.
>
>
pushed, thanks
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.

2018-12-20 Thread Vittorio Giovara
On Thu, Dec 20, 2018 at 7:18 PM Mohammad Izadi  wrote:

> Hi Vittorio,
>
> Thank you for your feedback ! Here is my answers to your questions:
>
> I thought we were going to rename the header as dynamic_hdr.h since it may
> contain multiple variants of metadata.
> Also I believe "metadata" in the name is redundant, but won't insist too
> much if you have strong feelings for it.
> *dynamic_hdr does not really point to dynamic metadata. When we use
> dynamic_hdr, it may be interpreted as a new type of HDR. I think
> hdr_dynamic_metadata is more meaningful. *
>

ok i won't insist about this too much if you prefer


> Since these two types only apply to HDR, do you think adding HDR in their
> names, like AVHDRPlusOverlapProcessOption and AVHDRPlusPercentile, would
> make sense?
> Would make them similar to the other types below
> *Done.*
>
> maybe add "or NULL on failure." here too
> also why return type and function name on two different lines?
> *Done.*
>

cool thanks
by the way do you have commit access or would like me to push the updated
patch when you send it?
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.

2018-12-19 Thread Vittorio Giovara
On Wed, Dec 19, 2018 at 7:08 PM Mohammad Izadi  wrote:

> From: Mohammad Izadi 
>
> The dynamic metadata contains data for color volume transform -
> application 4 of SMPTE 2094-40:2016 standard. The data comes from HEVC in
> the SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35.
>
> I'll add support to HEVC in a follow-up.
> ---
>

Thanks for the updated patch, here are some more comments below.


>  libavutil/Makefile   |   2 +
>  libavutil/frame.c|   1 +
>  libavutil/frame.h|   7 +
>  libavutil/hdr_dynamic_metadata.c |  47 +
>  libavutil/hdr_dynamic_metadata.h | 344 +++
>  libavutil/version.h  |   4 +-
>  6 files changed, 403 insertions(+), 2 deletions(-)
>  create mode 100644 libavutil/hdr_dynamic_metadata.c
>  create mode 100644 libavutil/hdr_dynamic_metadata.h
>

I thought we were going to rename the header as dynamic_hdr.h since it may
contain multiple variants of metadata.
Also I believe "metadata" in the name is redundant, but won't insist too
much if you have strong feelings for it.

diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 66f27f44bd..582ac470b2 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -166,6 +166,13 @@ enum AVFrameSideDataType {
>   * function in libavutil/timecode.c.
>   */
>  AV_FRAME_DATA_S12M_TIMECODE,
> +
> +/**
> + * HDR dynamic metadata associated with a video frame. The payload is
> + * an AVDynamicHDRPlus type and contains information for color
> + * volume transform - application 4 of SMPTE 2094-40:2016 standard.
> + */
> +AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
>  };
>
>  enum AVActiveFormatDescription {
> diff --git a/libavutil/hdr_dynamic_metadata.c
> b/libavutil/hdr_dynamic_metadata.c
> new file mode 100644
> index 00..7e4d7dec10
> --- /dev/null
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -0,0 +1,47 @@
> +/**
> + * Copyright (c) 2018 Mohammad Izadi 
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +#include "hdr_dynamic_metadata.h"
> +#include "mem.h"
> +
> +AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
> +{
> +AVDynamicHDRPlus *hdr_plus = av_mallocz(sizeof(AVDynamicHDRPlus));
> +if (!hdr_plus)
> +return NULL;
> +
> +if (size)
> +*size = sizeof(*hdr_plus);
> +
> +return hdr_plus;
> +}
>

this looks good

+ * Option for overlapping elliptical pixel selectors in an image.
> + */
> +enum AVOverlapProcessOption {
> +AV_OVERLAP_PROCESS_WEIGHTED_AVERAGING = 0,
> +AV_OVERLAP_PROCESS_LAYERING = 1,
> +};
> +
> +/**
> + * Percentile represents the percentile at a specific percentage in
> + * a distribution.
> + */
> +typedef struct AVPercentile {
> +/**
> + * The percentage value corresponding to a specific percentile
> linearized
> + * RGB value in the processing window in the scene. The value shall
> be in
> + * the range of 0 to100, inclusive.
> + */
> +uint8_t percentage;
> +/**
> + * The linearized maxRGB value at a specific percentile in the
> processing
> + * window in the scene. The value shall be in the range of 0 to 1,
> inclusive
> + * and in multiples of 0.1.
> + */
> +AVRational percentile;
> +} AVPercentile;
>

Since these two types only apply to HDR, do you think adding HDR in their
names, like AVHDRPlusOverlapProcessOption and AVHDRPlusPercentile, would
make sense?
Would make them similar to the other types below

+
> +/**
> + * Color transform parameters at a processing window in a dynamic
> metadata for
> + * SMPTE 2094-40.
> + */
> +typedef struct AVHDRPlusColorTransformParams {
> +/**
> + * The relative x coordinate of the top left pixel of the processing
> + * window. The value shall be in the range of 0 and 1, inclusive and
> + * in multiples of 1/(width of Picture – 1). The value 1 corresponds
> + * to the absolute coordinate of width of Picture – 1. The value for
> + * first processing window shall be 0.
> + */
> +AVRational window_upper_left_corner_x;
> +
> +/**
> + * The relative y coordinate of the top left pixel of the processing
> + * window. The value shall be in the range of 0 and 1, inclusive and
> +  

Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SPMTE 2094-40) to libavutil.

2018-12-17 Thread Vittorio Giovara
On Tue, Dec 18, 2018 at 12:04 AM Mohammad Izadi  wrote:

> As Jan mentioned, we need to specify the kind of the HDR metadata. Here are
> some alternatives:
>
> AVDynamicHDRPlus
> AVSMPTE2094App4 (Application 4 is HDR10+ while Dolby is application 1)
> AVHDRPlusMetadata
>
> Which one do you prefer?


IMO AVDynamicHDRPlus or AVHDRPlusMetadata.
Thanks,
Vittorio


> --
> Best,
> Mohammad
>
>
> On Mon, Dec 17, 2018 at 3:07 PM Jan Ekström  wrote:
>
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SPMTE 2094-40) to libavutil.

2018-12-17 Thread Vittorio Giovara
On Mon, Dec 17, 2018 at 5:53 PM Jan Ekström  wrote:

> On Tue, Dec 18, 2018 at 12:41 AM Vittorio Giovara
>  wrote:
> >
> > On Mon, Dec 10, 2018 at 2:50 PM Mohammad Izadi 
> wrote:
> >
> > > From: Mohammad Izadi 
> > >
> > > The dynamic metadata contains data for color volume transform -
> > > application 4 of SPMTE 2094-40:2016 standard. The data comes from HEVC
> in
> > > the SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35.
> > >
> > > I'll add support to HEVC in a follow-up.
> > >
> > > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > > index 9b3fb13e68..c5f30b6847 100644
> > > --- a/libavutil/frame.c
> > > +++ b/libavutil/frame.c
> > > @@ -840,6 +840,7 @@ const char *av_frame_side_data_name(enum
> > > AVFrameSideDataType type)
> > >  case AV_FRAME_DATA_QP_TABLE_PROPERTIES: return "QP table
> > > properties";
> > >  case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table
> > > data";
> > >  #endif
> > > +case AV_FRAME_DATA_HDR_DYNAMIC_METADATA_SMPTE2094_40: return "HDR
> > > Dynamic Metadata SMPTE2094-40";
> > >
> >
> > I like
> VeryLongJavaLikeNamingForFunctionsAndDataTypesAsWellAsEnumsOfCourse
> > as much as the next guy, but this is overly too long and the related
> > structure name (AVDynamicMetadataSMPTE2094_40) is inconsistent with the
> > public type naming present in this project.
> >
> > If I may suggest, please use the following names:
> > - AV_FRAME_DATA_DYNAMIC_HDR for frame data type: it is obviously
> metadata,
> > and the fact that is based on SMPTE2094-40 should not be hardcoded in the
> > name
> > - dynamic_hdr.h as header name: again the fact that it's metadata does
> not
> > be carried everywhere
> > - AVDynamicHDR as structure name: short and to the point, and without
> spec
> > numbers or underscores
> >
>
> I don't think SMPTE ST.2094-XX utilized the same fields?
>
> I think SMPTE ST.2094-40 is Samsung's dynamic metadata which is effect
> usually called "HDR10+" ? Not just calling it AVDynamicHDR could come
> up useful if we plan on adding support for SMPTE ST.2094-10 which is
> one part of what's called "Dolby Vision" (even though it is just
> dynamic metadata - because of course you have to stick everything
> under a single marketing name).
>

It's still technically dynamic metadata but I get your point.
It could maybe be stored in dynamic_hdr.h header, and call it
AVDynamicHDRPlus to differenciate it from the future AVDolbyVisionHDR. What
do you think?


> Just my 20 cents.
>
> Jan
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SPMTE 2094-40) to libavutil.

2018-12-17 Thread Vittorio Giovara
On Mon, Dec 10, 2018 at 2:50 PM Mohammad Izadi  wrote:

> From: Mohammad Izadi 
>
> The dynamic metadata contains data for color volume transform -
> application 4 of SPMTE 2094-40:2016 standard. The data comes from HEVC in
> the SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35.
>
> I'll add support to HEVC in a follow-up.
>
---
>  libavutil/Makefile   |   2 +
>  libavutil/frame.c|   1 +
>  libavutil/frame.h|   7 +
>  libavutil/hdr_dynamic_metadata.c |  40 
>  libavutil/hdr_dynamic_metadata.h | 344 +++
>  libavutil/version.h  |   2 +-
>  6 files changed, 395 insertions(+), 1 deletion(-)
>  create mode 100644 libavutil/hdr_dynamic_metadata.c
>  create mode 100644 libavutil/hdr_dynamic_metadata.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index caddc7e155..7dcb92b06b 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -31,6 +31,7 @@ HEADERS = adler32.h
>\
>file.h\
>frame.h   \
>hash.h\
> +  hdr_dynamic_metadata.h\
>

There

>hmac.h\
>hwcontext.h   \
>hwcontext_cuda.h  \
> @@ -119,6 +120,7 @@ OBJS = adler32.o
>   \
> fixed_dsp.o  \
> frame.o  \
> hash.o   \
> +   hdr_dynamic_metadata.o   \
> hmac.o   \
> hwcontext.o  \
> imgutils.o   \
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 9b3fb13e68..c5f30b6847 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -840,6 +840,7 @@ const char *av_frame_side_data_name(enum
> AVFrameSideDataType type)
>  case AV_FRAME_DATA_QP_TABLE_PROPERTIES: return "QP table
> properties";
>  case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table
> data";
>  #endif
> +case AV_FRAME_DATA_HDR_DYNAMIC_METADATA_SMPTE2094_40: return "HDR
> Dynamic Metadata SMPTE2094-40";
>

I like VeryLongJavaLikeNamingForFunctionsAndDataTypesAsWellAsEnumsOfCourse
as much as the next guy, but this is overly too long and the related
structure name (AVDynamicMetadataSMPTE2094_40) is inconsistent with the
public type naming present in this project.

If I may suggest, please use the following names:
- AV_FRAME_DATA_DYNAMIC_HDR for frame data type: it is obviously metadata,
and the fact that is based on SMPTE2094-40 should not be hardcoded in the
name
- dynamic_hdr.h as header name: again the fact that it's metadata does not
be carried everywhere
- AVDynamicHDR as structure name: short and to the point, and without spec
numbers or underscores


>  }
>  return NULL;
>  }
> diff --git a/libavutil/hdr_dynamic_metadata.c
> b/libavutil/hdr_dynamic_metadata.c
> new file mode 100644
> index 00..59dfcc84e4
> --- /dev/null
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -0,0 +1,40 @@
> +
> +#include "hdr_dynamic_metadata.h"
> +#include "mem.h"
> +
> +AVDynamicMetadataSMPTE2094_40
> *av_dynamic_metadata_smpte2094_40_alloc(void)
>

you need to return the size of allocation back to the caller, see
spherical.h as example (especially since you document that "its size is not
a part of the public ABI.")

+/**
> + * Option for overlapping elliptical pixel selectors in an image.
> + */
> +enum AVOverlapProcessOption {
> +AV_OVERLAP_PROCESS_WEIGHTED_AVERAGING = 0,
> +AV_OVERLAP_PROCESS_LAYERING = 1,
> +};
>

I'm not a fan of these names, but i've bikeshed enough


> +
> +/**
> + * Percentile represents the percentile at a specific percentage in
> + * a distribution.
> + */
> +typedef struct AVPercentile {
> +/**
> + * The percentage value corresponding to a spesific percentile
> linearized
>

typo 'spesific'

+ * RGB value in the processing window in the scene. The value shall be
> in
> + * the range of 0 to100, inclusive.
> + */
> +uint8_t percentage;
> +/**
> + * The linearized maxRGB value at a specific percentile in the
> processing
> + * window in the scene. The value shall be in the range of 0 to 1,
> inclusive
> + * and in multiples of 0.1.
> + */
> +AVRational percentile;
> +} AVPercentile;
> +
>
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] [PATCH 1/4] proresenc_nanatoliy: Rename a profile name with the correct one

2018-11-05 Thread Vittorio Giovara
On Sat, Nov 3, 2018 at 10:23 AM Paul B Mahol  wrote:

> On 11/2/18, Vittorio Giovara  wrote:
> > In all Apple documentation, this profile is called Prores .
> > ---
> >  libavcodec/proresenc_anatoliy.c | 16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
>
>
> There is typo in log message, with that fixed patchset lgtm.
>

thanks, fixed and pushed
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/4] mjpeg: Use profile names in the encoder and decoder

2018-11-05 Thread Vittorio Giovara
On Fri, Nov 2, 2018 at 5:29 PM Carl Eugen Hoyos  wrote:

> 2018-11-02 20:35 GMT+01:00, Vittorio Giovara :
> > ---
> >  libavcodec/codec_desc.c | 1 +
> >  libavcodec/mjpegdec.c   | 2 ++
> >  libavcodec/profiles.c   | 9 +
> >  libavcodec/profiles.h   | 1 +
> >  libavcodec/version.h| 2 +-
> >  5 files changed, 14 insertions(+), 1 deletion(-)
>
> How does this patch affect the encoder?
>

sorry forgot to fold in these changes

diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
index d2fcb8e191..0ea7bd3d10 100644
--- a/libavcodec/mjpegenc.c
+++ b/libavcodec/mjpegenc.c
@@ -38,6 +38,7 @@
 #include "mpegvideo.h"
 #include "mjpeg.h"
 #include "mjpegenc.h"
+#include "profiles.h"

 static int alloc_huffman(MpegEncContext *s)
 {
@@ -418,6 +419,7 @@ AVCodec ff_mjpeg_encoder = {
 AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
AV_PIX_FMT_NONE
 },
 .priv_class = _class,
+.profiles   = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
 };
 #endif

-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/4] prores: Use profile names in the various encoders and decoders

2018-11-02 Thread Vittorio Giovara
Export FF_PROFILE_PRORES_* symbols to the public header.
---
 libavcodec/avcodec.h|  7 +++
 libavcodec/codec_desc.c |  1 +
 libavcodec/profiles.c   | 10 ++
 libavcodec/profiles.h   |  1 +
 libavcodec/proresdec2.c |  2 ++
 libavcodec/proresenc_anatoliy.c | 11 +++
 libavcodec/proresenc_kostya.c   |  2 ++
 libavcodec/version.h|  4 ++--
 8 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 705a3ce4f3..cff821c3d4 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2960,6 +2960,13 @@ typedef struct AVCodecContext {
 
 #define FF_PROFILE_SBC_MSBC 1
 
+#define FF_PROFILE_PRORES_PROXY 0
+#define FF_PROFILE_PRORES_LT1
+#define FF_PROFILE_PRORES_STANDARD  2
+#define FF_PROFILE_PRORES_HQ3
+#define FF_PROFILE_PRORES_  4
+#define FF_PROFILE_PRORES_XQ5
+
 /**
  * level
  * - encoding: Set by user.
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 67a30542d1..ce6d265d13 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1077,6 +1077,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
 .name  = "prores",
 .long_name = NULL_IF_CONFIG_SMALL("Apple ProRes (iCodec Pro)"),
 .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
+.profiles  = NULL_IF_CONFIG_SMALL(ff_prores_profiles),
 },
 {
 .id= AV_CODEC_ID_JV,
diff --git a/libavcodec/profiles.c b/libavcodec/profiles.c
index c31399f83e..8a4447e438 100644
--- a/libavcodec/profiles.c
+++ b/libavcodec/profiles.c
@@ -151,4 +151,14 @@ const AVProfile ff_sbc_profiles[] = {
 { FF_PROFILE_UNKNOWN },
 };
 
+const AVProfile ff_prores_profiles[] = {
+{ FF_PROFILE_PRORES_PROXY,"Proxy"},
+{ FF_PROFILE_PRORES_LT,   "LT"   },
+{ FF_PROFILE_PRORES_STANDARD, "Standard" },
+{ FF_PROFILE_PRORES_HQ,   "HQ"   },
+{ FF_PROFILE_PRORES_, "" },
+{ FF_PROFILE_PRORES_XQ,   "XQ"   },
+{ FF_PROFILE_UNKNOWN }
+};
+
 #endif /* !CONFIG_SMALL */
diff --git a/libavcodec/profiles.h b/libavcodec/profiles.h
index 9d7e211e15..4669dad8d5 100644
--- a/libavcodec/profiles.h
+++ b/libavcodec/profiles.h
@@ -33,5 +33,6 @@ extern const AVProfile ff_vc1_profiles[];
 extern const AVProfile ff_vp9_profiles[];
 extern const AVProfile ff_av1_profiles[];
 extern const AVProfile ff_sbc_profiles[];
+extern const AVProfile ff_prores_profiles[];
 
 #endif /* AVCODEC_PROFILES_H */
diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
index d818e5d8da..6b3021bdfa 100644
--- a/libavcodec/proresdec2.c
+++ b/libavcodec/proresdec2.c
@@ -33,6 +33,7 @@
 #include "get_bits.h"
 #include "idctdsp.h"
 #include "internal.h"
+#include "profiles.h"
 #include "simple_idct.h"
 #include "proresdec.h"
 #include "proresdata.h"
@@ -730,4 +731,5 @@ AVCodec ff_prores_decoder = {
 .close  = decode_close,
 .decode = decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_SLICE_THREADS | 
AV_CODEC_CAP_FRAME_THREADS,
+.profiles   = NULL_IF_CONFIG_SMALL(ff_prores_profiles),
 };
diff --git a/libavcodec/proresenc_anatoliy.c b/libavcodec/proresenc_anatoliy.c
index dbbf13f4c0..f35f049a78 100644
--- a/libavcodec/proresenc_anatoliy.c
+++ b/libavcodec/proresenc_anatoliy.c
@@ -30,6 +30,7 @@
 #include "avcodec.h"
 #include "dct.h"
 #include "internal.h"
+#include "profiles.h"
 #include "proresdata.h"
 #include "put_bits.h"
 #include "bytestream.h"
@@ -37,12 +38,6 @@
 
 #define DEFAULT_SLICE_MB_WIDTH 8
 
-#define FF_PROFILE_PRORES_PROXY 0
-#define FF_PROFILE_PRORES_LT1
-#define FF_PROFILE_PRORES_STANDARD  2
-#define FF_PROFILE_PRORES_HQ3
-#define FF_PROFILE_PRORES_  4
-
 static const AVProfile profiles[] = {
 { FF_PROFILE_PRORES_PROXY,"apco"},
 { FF_PROFILE_PRORES_LT,   "apcs"},
@@ -679,7 +674,7 @@ AVCodec ff_prores_aw_encoder = {
 .encode2= prores_encode_frame,
 .pix_fmts   = (const enum AVPixelFormat[]){AV_PIX_FMT_YUV422P10, 
AV_PIX_FMT_YUV444P10, AV_PIX_FMT_NONE},
 .capabilities   = AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_INTRA_ONLY,
-.profiles   = profiles
+.profiles   = NULL_IF_CONFIG_SMALL(ff_prores_profiles),
 };
 
 AVCodec ff_prores_encoder = {
@@ -693,5 +688,5 @@ AVCodec ff_prores_encoder = {
 .encode2= prores_encode_frame,
 .pix_fmts   = (const enum AVPixelFormat[]){AV_PIX_FMT_YUV422P10, 
AV_PIX_FMT_YUV444P10, AV_PIX_FMT_NONE},
 .capabilities   = AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_INTRA_ONLY,
-.profiles   = profiles
+.profiles   = NULL_IF_CONFIG_SMALL(ff_prores_profiles),
 };
diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c
index 81f3865ea6..9a77d24fb6 100644
--- a/libavcodec/proresenc_kostya.c
+++ b/libavcodec/proresenc_kostya.c
@@ -28,6 

[FFmpeg-devel] [PATCH 4/4] mjpeg: Use profile names in the encoder and decoder

2018-11-02 Thread Vittorio Giovara
---
 libavcodec/codec_desc.c | 1 +
 libavcodec/mjpegdec.c   | 2 ++
 libavcodec/profiles.c   | 9 +
 libavcodec/profiles.h   | 1 +
 libavcodec/version.h| 2 +-
 5 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index ce6d265d13..7bf39862d2 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -81,6 +81,7 @@ static const AVCodecDescriptor codec_descriptors[] = {
 .long_name = NULL_IF_CONFIG_SMALL("Motion JPEG"),
 .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSY,
 .mime_types= MT("image/jpeg"),
+.profiles  = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
 },
 {
 .id= AV_CODEC_ID_MJPEGB,
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index b0cb3ffc83..96c425515a 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -43,6 +43,7 @@
 #include "mjpeg.h"
 #include "mjpegdec.h"
 #include "jpeglsdec.h"
+#include "profiles.h"
 #include "put_bits.h"
 #include "tiff.h"
 #include "exif.h"
@@ -2796,6 +2797,7 @@ AVCodec ff_mjpeg_decoder = {
 .capabilities   = AV_CODEC_CAP_DR1,
 .max_lowres = 3,
 .priv_class = _class,
+.profiles   = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
 .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
   FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
 .hw_configs = (const AVCodecHWConfigInternal*[]) {
diff --git a/libavcodec/profiles.c b/libavcodec/profiles.c
index 8a4447e438..e6f937fdb4 100644
--- a/libavcodec/profiles.c
+++ b/libavcodec/profiles.c
@@ -161,4 +161,13 @@ const AVProfile ff_prores_profiles[] = {
 { FF_PROFILE_UNKNOWN }
 };
 
+const AVProfile ff_mjpeg_profiles[] = {
+{ FF_PROFILE_MJPEG_HUFFMAN_BASELINE_DCT,"Baseline"},
+{ FF_PROFILE_MJPEG_HUFFMAN_EXTENDED_SEQUENTIAL_DCT, "Sequential"  },
+{ FF_PROFILE_MJPEG_HUFFMAN_PROGRESSIVE_DCT, "Progressive" },
+{ FF_PROFILE_MJPEG_HUFFMAN_LOSSLESS,"Lossless"},
+{ FF_PROFILE_MJPEG_JPEG_LS, "JPEG LS" },
+{ FF_PROFILE_UNKNOWN }
+};
+
 #endif /* !CONFIG_SMALL */
diff --git a/libavcodec/profiles.h b/libavcodec/profiles.h
index 4669dad8d5..ab61e03e15 100644
--- a/libavcodec/profiles.h
+++ b/libavcodec/profiles.h
@@ -34,5 +34,6 @@ extern const AVProfile ff_vp9_profiles[];
 extern const AVProfile ff_av1_profiles[];
 extern const AVProfile ff_sbc_profiles[];
 extern const AVProfile ff_prores_profiles[];
+extern const AVProfile ff_mjpeg_profiles[];
 
 #endif /* AVCODEC_PROFILES_H */
diff --git a/libavcodec/version.h b/libavcodec/version.h
index d2ac30d4a8..068004ac9d 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@
 
 #define LIBAVCODEC_VERSION_MAJOR  58
 #define LIBAVCODEC_VERSION_MINOR  34
-#define LIBAVCODEC_VERSION_MICRO 101
+#define LIBAVCODEC_VERSION_MICRO 102
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
LIBAVCODEC_VERSION_MINOR, \
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 3/4] proresdec2: Parse codec_tag and export profile information

2018-11-02 Thread Vittorio Giovara
---
 libavcodec/proresdec2.c | 24 
 libavcodec/version.h|  2 +-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c
index 6b3021bdfa..130a4e3fe8 100644
--- a/libavcodec/proresdec2.c
+++ b/libavcodec/proresdec2.c
@@ -62,6 +62,30 @@ static av_cold int decode_init(AVCodecContext *avctx)
 permute(ctx->progressive_scan, ff_prores_progressive_scan, 
idct_permutation);
 permute(ctx->interlaced_scan, ff_prores_interlaced_scan, idct_permutation);
 
+switch (avctx->codec_tag) {
+case MKTAG('a','p','c','o'):
+avctx->profile = FF_PROFILE_PRORES_PROXY;
+break;
+case MKTAG('a','p','c','s'):
+avctx->profile = FF_PROFILE_PRORES_LT;
+break;
+case MKTAG('a','p','c','n'):
+avctx->profile = FF_PROFILE_PRORES_STANDARD;
+break;
+case MKTAG('a','p','c','h'):
+avctx->profile = FF_PROFILE_PRORES_HQ;
+break;
+case MKTAG('a','p','4','h'):
+avctx->profile = FF_PROFILE_PRORES_;
+break;
+case MKTAG('a','p','4','x'):
+avctx->profile = FF_PROFILE_PRORES_XQ;
+break;
+default:
+avctx->profile = FF_PROFILE_UNKNOWN;
+av_log(avctx, AV_LOG_WARNING, "Unknown prores profile %d\n", 
avctx->codec_tag);
+}
+
 return 0;
 }
 
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 91809641b4..d2ac30d4a8 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@
 
 #define LIBAVCODEC_VERSION_MAJOR  58
 #define LIBAVCODEC_VERSION_MINOR  34
-#define LIBAVCODEC_VERSION_MICRO 100
+#define LIBAVCODEC_VERSION_MICRO 101
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
LIBAVCODEC_VERSION_MINOR, \
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/4] proresenc_nanatoliy: Rename a profile name with the correct one

2018-11-02 Thread Vittorio Giovara
In all Apple documentation, this profile is called Prores .
---
 libavcodec/proresenc_anatoliy.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavcodec/proresenc_anatoliy.c b/libavcodec/proresenc_anatoliy.c
index 6b9ce4a59a..dbbf13f4c0 100644
--- a/libavcodec/proresenc_anatoliy.c
+++ b/libavcodec/proresenc_anatoliy.c
@@ -41,14 +41,14 @@
 #define FF_PROFILE_PRORES_LT1
 #define FF_PROFILE_PRORES_STANDARD  2
 #define FF_PROFILE_PRORES_HQ3
-#define FF_PROFILE_PRORES_444   4
+#define FF_PROFILE_PRORES_  4
 
 static const AVProfile profiles[] = {
 { FF_PROFILE_PRORES_PROXY,"apco"},
 { FF_PROFILE_PRORES_LT,   "apcs"},
 { FF_PROFILE_PRORES_STANDARD, "apcn"},
 { FF_PROFILE_PRORES_HQ,   "apch"},
-{ FF_PROFILE_PRORES_444,  "ap4h"},
+{ FF_PROFILE_PRORES_, "ap4h"},
 { FF_PROFILE_UNKNOWN }
 };
 
@@ -550,7 +550,7 @@ static int prores_encode_frame(AVCodecContext *avctx, 
AVPacket *pkt,
 bytestream_put_buffer(, "fmpg", 4);
 bytestream_put_be16(, avctx->width);
 bytestream_put_be16(, avctx->height);
-if (avctx->profile == FF_PROFILE_PRORES_444) {
+if (avctx->profile == FF_PROFILE_PRORES_) {
 *buf++ = 0xC2; // 444, not interlaced
 } else {
 *buf++ = 0x82; // 422, not interlaced
@@ -605,13 +605,13 @@ static av_cold int prores_encode_init(AVCodecContext 
*avctx)
 av_log(avctx, AV_LOG_INFO,
 "encoding with ProRes standard (apcn) profile\n");
 } else if (avctx->pix_fmt == AV_PIX_FMT_YUV444P10) {
-avctx->profile = FF_PROFILE_PRORES_444;
+avctx->profile = FF_PROFILE_PRORES_;
 av_log(avctx, AV_LOG_INFO,
-   "encoding with ProRes 444 (ap4h) profile\n");
+   "encoding with ProRes  (ap4h) profile\n");
 }
 
 } else if (avctx->profile < FF_PROFILE_PRORES_PROXY
-|| avctx->profile > FF_PROFILE_PRORES_444) {
+|| avctx->profile > FF_PROFILE_PRORES_) {
 av_log(
 avctx,
 AV_LOG_ERROR,
@@ -622,13 +622,13 @@ static av_cold int prores_encode_init(AVCodecContext 
*avctx)
 av_log(avctx, AV_LOG_ERROR,
"encoding with ProRes 444 (ap4h) profile, need YUV444P10 
input\n");
 return AVERROR(EINVAL);
-}  else if ((avctx->pix_fmt == AV_PIX_FMT_YUV444P10) && (avctx->profile < 
FF_PROFILE_PRORES_444)){
+}  else if ((avctx->pix_fmt == AV_PIX_FMT_YUV444P10) && (avctx->profile < 
FF_PROFILE_PRORES_)){
 av_log(avctx, AV_LOG_ERROR,
"encoding with ProRes Proxy/LT/422/422 HQ (apco, apcs, apcn, 
ap4h) profile, need YUV422P10 input\n");
 return AVERROR(EINVAL);
 }
 
-if (avctx->profile < FF_PROFILE_PRORES_444) { /* 422 versions */
+if (avctx->profile < FF_PROFILE_PRORES_) { /* 422 versions */
 ctx->is_422 = 1;
 if ((avctx->height & 0xf) || (avctx->width & 0xf)) {
 ctx->fill_y = av_malloc(4 * (DEFAULT_SLICE_MB_WIDTH << 8));
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V3] Add a filter implementing HDR image reconstruction from a single exposure using deep CNNs

2018-10-21 Thread Vittorio Giovara
On Mon, Oct 22, 2018 at 4:19 AM Guo, Yejun  wrote:

> +.description   = NULL_IF_CONFIG_SMALL("HDR image reconstruction from
> a single exposure using deep CNNs."),
>
>
>
> > why "reconstruction"? there is nothing to construct back if the source
> wasn't hdr to begin with
>
> > "tonemap" is probably a better term here, in my opinion
>
> > same for previous uses
>
>
>
> there is more detail data generated with the dnn model, the model accepts
> sdr frame and generates hdr data.
>
> see more detail in paper @https://arxiv.org/pdf/1710.07480.pdf, and the
> description comes from the title of this paper.
>

Thanks for the link, however i'm still not sold on the term. You "generate"
hdr data, not "reconstruct": it's generated/estimated/made up data, not
data that is lost and needs to be reconstrcuted. I suggested "tonemap"
because you're mapping SDR tones (aka colors) to HDR ones, and that seems
the right term to use. If you really dislike it, at least consider "HDR
image generation from a single exposure using deep CNNs" which would work
much better.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V3] Add a filter implementing HDR image reconstruction from a single exposure using deep CNNs

2018-10-19 Thread Vittorio Giovara
On Fri, Oct 19, 2018 at 10:11 AM Guo, Yejun  wrote:

> see the algorithm's paper and code below.
>
> the filter's parameter looks like:
> sdr2hdr=model_filename=/path_to_tensorflow_graph.pb:out_fmt=gbrp10le
>

can you add some usage documentation to doc/filters.texi?

The input of the deep CNN model is RGB24 while the output is float
> for each color channel. This is the filter's default behavior to
> output format with gbrpf32le. And gbrp10le is also supported as the
> output, so we can see the rendering result in a player, as a reference.
>
> To generate the model file, we need modify the original script a little.
> - set name='y' for y_final within script at
> https://github.com/gabrieleilertsen/hdrcnn/blob/master/network.py
> - add the following code to the script at
> https://github.com/gabrieleilertsen/hdrcnn/blob/master/hdrcnn_predict.py
>
> graph = tf.graph_util.convert_variables_to_constants(sess, sess.graph_def,
> ["y"])
> tf.train.write_graph(graph, '.', 'graph.pb', as_text=False)
>
> The filter only works when tensorflow C api is supported in the system,
> native backend is not supported since there are some different types of
> layers in the deep CNN model, besides CONV and DEPTH_TO_SPACE.
>
> https://arxiv.org/pdf/1710.07480.pdf:
>   author   = "Eilertsen, Gabriel and Kronander, Joel, and Denes,
> Gyorgy and Mantiuk, Rafał and Unger, Jonas",
>   title= "HDR image reconstruction from a single exposure using
> deep CNNs",
>   journal  = "ACM Transactions on Graphics (TOG)",
>   number   = "6",
>   volume   = "36",
>   articleno= "178",
>   year = "2017"
>
> https://github.com/gabrieleilertsen/hdrcnn
>
> btw, as a whole solution, metadata should also be generated from
> the sdr video, so to be encoded as a HDR video. Not supported yet.
> This patch just focuses on this paper.
>

Is this something you are working on and will it be added later?


> v3: use int16_t instead of short
> v2: use AV_OPT_TYPE_PIXEL_FMT for filter option
> remove some unnecessary code
> Use in->linesize[0] and FFMAX/FFMIN
> remove flag AVFILTER_FLAG_SLICE_THREADS
> add av_log message when error
>

there is no need for this block to be left in the commit log


> Signed-off-by: Guo, Yejun 
> ---
>  libavfilter/Makefile |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/vf_sdr2hdr.c | 266
> +++
>  3 files changed, 268 insertions(+)
>  create mode 100644 libavfilter/vf_sdr2hdr.c
>
> +static av_cold int init(AVFilterContext* context)
> +{
> +SDR2HDRContext* ctx = context->priv;
> +
> +if (ctx->out_fmt != AV_PIX_FMT_GBRPF32LE && ctx->out_fmt !=
> AV_PIX_FMT_GBRP10LE) {
> +av_log(context, AV_LOG_ERROR, "could not support the output
> format\n");
> +return AVERROR(ENOSYS);
> +}
> +
> +#if (CONFIG_LIBTENSORFLOW == 1)
> +ctx->dnn_module = ff_get_dnn_module(DNN_TF);
> +if (!ctx->dnn_module){
> +av_log(context, AV_LOG_ERROR, "could not create DNN module for
> tensorflow backend\n");
> +return AVERROR(ENOMEM);
> +}
> +if (!ctx->model_filename){
> +av_log(context, AV_LOG_ERROR, "model file for network was not
> specified\n");
> +return AVERROR(EIO);
> +}
> +if (!ctx->dnn_module->load_model) {
> +av_log(context, AV_LOG_ERROR, "load_model for network was not
> specified\n");
> +return AVERROR(EIO);
> +}
> +ctx->model = (ctx->dnn_module->load_model)(ctx->model_filename);
> +if (!ctx->model){
> +av_log(context, AV_LOG_ERROR, "could not load DNN model\n");
> +return AVERROR(EIO);
> +}
> +return 0;
> +#else
> +return AVERROR(EIO);
> +#endif
> +}
>

this is incorrect, what you should do is make libtensorflow a dependency of
this filter in the configure file and disable this filter when it is not
enabled


> +
> +static int query_formats(AVFilterContext* context)
> +{
> +const enum AVPixelFormat in_formats[] = {AV_PIX_FMT_RGB24,
> + AV_PIX_FMT_NONE};
> +enum AVPixelFormat out_formats[2];
> +SDR2HDRContext* ctx = context->priv;
> +AVFilterFormats* formats_list;
> +int ret = 0;
> +
> +formats_list = ff_make_format_list(in_formats);
> +if ((ret = ff_formats_ref(formats_list,
> >inputs[0]->out_formats)) < 0)
> +return ret;
> +
> +out_formats[0] = ctx->out_fmt;
> +out_formats[1] = AV_PIX_FMT_NONE;
> +formats_list = ff_make_format_list(out_formats);
> +if ((ret = ff_formats_ref(formats_list,
> >outputs[0]->in_formats)) < 0)
> +return ret;
> +
> +return 0;
> +}
> +
> +static int config_props(AVFilterLink* inlink)
> +{
> +AVFilterContext* context = inlink->dst;
> +SDR2HDRContext* ctx = context->priv;
> +AVFilterLink* outlink = context->outputs[0];
> +DNNReturnType result;
> +
> +// the dnn model is tied with resolution due to deconv layer of
> tensorflow
> +// now just support 

Re: [FFmpeg-devel] [PATCH] avformat: add H264 and HEVC support in IVF muxer

2018-10-15 Thread Vittorio Giovara
On Thu, Oct 11, 2018 at 5:28 PM Jan Ekström  wrote:

> On Thu, Oct 11, 2018 at 10:58 PM Alex Sukhanov 
> wrote:
> >
> > Hi Mark,
> >
> > at Google we have some old service which is still running and it works
> only
> > with the IVF container. It would be great if ffmpeg could generate such
> > videos so we could give them to the service then. Given that ffmpeg IVF
> > demuxer already supports reading such files, I think it's reasonable to
> > make IVF muxer be able to generate them.
> > Hope it answers the question.
> >
> > Thank you
>
> Given the amount of code is not large I'm not against having it in,
> but if it's not something that ever was meant to go into the public
> I'd probably disable creation of such files unless you enable a less
> standards-compliant strictness mode.
>

maybe creation of such files could be allowed only with a strict compliance
flag?
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] decklink: Add support for output of Active Format Description (AFD)

2018-08-27 Thread Vittorio Giovara
On Wed, Aug 22, 2018 at 9:53 PM, Devin Heitmueller <
dheitmuel...@ltnglobal.com> wrote:

> Implement support for including AFD in decklink output.  This
> includes making sure the AFD data is preserved when going from
> an AVFrame to a V210 packet (needed for 10-bit support).
>
> Updated to reflect feedback from Marton Balint ,
> Carl Eugen Hoyos  and Aaron Levinson
> .
>
> Signed-off-by: Devin Heitmueller 
> ---
>  libavcodec/avcodec.h |  6 +
>  libavcodec/v210enc.c |  8 +++
>  libavdevice/decklink_enc.cpp | 54 ++
> --
>  3 files changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 31e50d5a94..192e15746d 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1362,6 +1362,12 @@ enum AVPacketSideDataType {
>  AV_PKT_DATA_ENCRYPTION_INFO,
>
>  /**
> + * Active Format Description data consisting of a single byte as
> specified
> + * in ETSI TS 101 154 using AVActiveFormatDescription enum.
> + */
> +AV_PKT_DATA_AFD,
> +
> +/**
>   * The number of side data types.
>   * This is not part of the public API/ABI in the sense that it may
>   * change when new side data types are added.
>

I think you should add an entry in ff_decode_frame_props() so that pkt side
data can propagate to frame side data
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/4] vf_tonemap: Update the default peak values

2018-07-31 Thread Vittorio Giovara
On Wed, Jul 25, 2018 at 5:46 PM, Vittorio Giovara <
vittorio.giov...@gmail.com> wrote:

> When there is no metadata attached to a frame, take into account both
> the PQ and HLG transfers, and change the HLG default value to 10:
> the value of 12 is the maximum range in scene referred light, but
> the reference OOTF maps this from 0 to 1000 cd/m² on the ideal HLG
> monitor.
>
> This matches what vf_tonemap_opencl does.
> ---
>  libavfilter/vf_tonemap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>

haasn was suggesting to change the title to `vf_tonemap: Fix the logic for
detecting the maximum peak of untagged sources` in order to better describe
the contents of this patch (and avoid having to split this simple change).
If no objections I'll amend the commit title and push soon.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Support for Ambisonics and OpusProjection* API.

2018-07-26 Thread Vittorio Giovara
On Thu, Jul 26, 2018 at 4:15 PM, Rostislav Pehlivanov 
wrote:

> Hey,
>
> As of now, the ambisonics API is enabled by default in libopus. We still
> don't have a way to signal ambisonics yet.
> We still have plenty of bits left in libavutil/channel_layout.h to signal
> many orders of ambisonics but some people have had opinions against
> extending that API. We could instead extend AVMatrixEncoding but I don't
> think that's entirely appropriate.
> What opinions do people have on this?
>

I had been working on a new API that would encompass ambisonic ordering
(see
https://github.com/kodabb/libav/commit/98d9b0a7b28525b29e40ae4c564e51e7c94449eb).
The downside is that it requires updating the whole channel layout API (see
https://github.com/kodabb/libav/commit/c023b553e6ad6da5af6d0d4ff067ff844b2fcfac
)
I got it mostly working but ran into issues during backward compatibility,
and didn't have time to debug and fix it.

If anyone wants to finish the set, backport it, and add the missing lswr
part it would be easy work. I'm available to help in the process just to
get this completed.
The full branch is available at https://github.com/kodabb/libav/commits/chl
(I hope this will be a mature discussion even though the patches belong to
another tree).
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 4/4] vf_tonemap: Update hdr metadata with the new peak value

2018-07-25 Thread Vittorio Giovara
Less effective than the approach in vf_tonemap_opencl because there
is no peak detection, but it's still a good idea to implement this.
---
 libavfilter/vf_tonemap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavfilter/vf_tonemap.c b/libavfilter/vf_tonemap.c
index b62532946b..98a2c4bd23 100644
--- a/libavfilter/vf_tonemap.c
+++ b/libavfilter/vf_tonemap.c
@@ -265,6 +265,8 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 
 av_frame_free();
 
+ff_update_hdr_metadata(out, peak);
+
 return ff_filter_frame(outlink, out);
 }
 
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/4] vf_tonemap[_opencl]: Move determine_signal_peak() to a shared file

2018-07-25 Thread Vittorio Giovara
The two functions are identical. Use the shared LumaCoeffients type too.
---
 libavfilter/Makefile|  2 +-
 libavfilter/colorspace.c| 29 +
 libavfilter/colorspace.h|  6 ++
 libavfilter/vf_tonemap.c| 38 +++--
 libavfilter/vf_tonemap_opencl.c | 28 +---
 5 files changed, 40 insertions(+), 63 deletions(-)

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 86b04e3efb..245302bbe8 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -365,7 +365,7 @@ OBJS-$(CONFIG_TILE_FILTER)   += vf_tile.o
 OBJS-$(CONFIG_TINTERLACE_FILTER) += vf_tinterlace.o
 OBJS-$(CONFIG_TLUT2_FILTER)  += vf_lut2.o framesync.o
 OBJS-$(CONFIG_TMIX_FILTER)   += vf_mix.o framesync.o
-OBJS-$(CONFIG_TONEMAP_FILTER)+= vf_tonemap.o
+OBJS-$(CONFIG_TONEMAP_FILTER)+= vf_tonemap.o colorspace.o
 OBJS-$(CONFIG_TONEMAP_OPENCL_FILTER) += vf_tonemap_opencl.o 
colorspace.o opencl.o \
 opencl/tonemap.o 
opencl/colorspace_common.o
 OBJS-$(CONFIG_TRANSPOSE_FILTER)  += vf_transpose.o
diff --git a/libavfilter/colorspace.c b/libavfilter/colorspace.c
index 45da1dd124..d6f6055401 100644
--- a/libavfilter/colorspace.c
+++ b/libavfilter/colorspace.c
@@ -17,6 +17,10 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "libavutil/frame.h"
+#include "libavutil/mastering_display_metadata.h"
+#include "libavutil/pixdesc.h"
+
 #include "colorspace.h"
 
 
@@ -89,3 +93,28 @@ void ff_fill_rgb2xyz_table(const struct PrimaryCoefficients 
*coeffs,
 rgb2xyz[2][1] *= sg;
 rgb2xyz[2][2] *= sb;
 }
+
+double ff_determine_signal_peak(AVFrame *in)
+{
+AVFrameSideData *sd = av_frame_get_side_data(in, 
AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
+double peak = 0;
+
+if (sd) {
+AVContentLightMetadata *clm = (AVContentLightMetadata *)sd->data;
+peak = clm->MaxCLL / REFERENCE_WHITE;
+}
+
+sd = av_frame_get_side_data(in, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
+if (!peak && sd) {
+AVMasteringDisplayMetadata *metadata = (AVMasteringDisplayMetadata 
*)sd->data;
+if (metadata->has_luminance)
+peak = av_q2d(metadata->max_luminance) / REFERENCE_WHITE;
+}
+
+// For untagged source, use peak of 1 if SMPTE ST.2084
+// otherwise assume HLG with reference display peak 1000.
+if (!peak)
+peak = in->color_trc == AVCOL_TRC_SMPTE2084 ? 100.0f : 10.0f;
+
+return peak;
+}
diff --git a/libavfilter/colorspace.h b/libavfilter/colorspace.h
index 9d45ee2366..75705ecfc7 100644
--- a/libavfilter/colorspace.h
+++ b/libavfilter/colorspace.h
@@ -21,6 +21,9 @@
 #define AVFILTER_COLORSPACE_H
 
 #include "libavutil/common.h"
+#include "libavutil/frame.h"
+
+#define REFERENCE_WHITE 100.0f
 
 struct LumaCoefficients {
 double cr, cg, cb;
@@ -40,4 +43,7 @@ void ff_matrix_mul_3x3(double dst[3][3],
 void ff_fill_rgb2xyz_table(const struct PrimaryCoefficients *coeffs,
const struct WhitepointCoefficients *wp,
double rgb2xyz[3][3]);
+
+double ff_determine_signal_peak(AVFrame *in);
+
 #endif
diff --git a/libavfilter/vf_tonemap.c b/libavfilter/vf_tonemap.c
index 10fd7ea016..b62532946b 100644
--- a/libavfilter/vf_tonemap.c
+++ b/libavfilter/vf_tonemap.c
@@ -30,17 +30,15 @@
 #include "libavutil/imgutils.h"
 #include "libavutil/internal.h"
 #include "libavutil/intreadwrite.h"
-#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
 
 #include "avfilter.h"
+#include "colorspace.h"
 #include "formats.h"
 #include "internal.h"
 #include "video.h"
 
-#define REFERENCE_WHITE 100.0f
-
 enum TonemapAlgorithm {
 TONEMAP_NONE,
 TONEMAP_LINEAR,
@@ -52,11 +50,6 @@ enum TonemapAlgorithm {
 TONEMAP_MAX,
 };
 
-typedef struct LumaCoefficients {
-double cr, cg, cb;
-} LumaCoefficients;
-
-
 static const struct LumaCoefficients luma_coefficients[AVCOL_SPC_NB] = {
 [AVCOL_SPC_FCC]= { 0.30,   0.59,   0.11   },
 [AVCOL_SPC_BT470BG]= { 0.299,  0.587,  0.114  },
@@ -75,7 +68,7 @@ typedef struct TonemapContext {
 double desat;
 double peak;
 
-const LumaCoefficients *coeffs;
+const struct LumaCoefficients *coeffs;
 } TonemapContext;
 
 static const enum AVPixelFormat pix_fmts[] = {
@@ -114,31 +107,6 @@ static av_cold int init(AVFilterContext *ctx)
 return 0;
 }
 
-static double determine_signal_peak(AVFrame *in)
-{
-AVFrameSideData *sd = av_frame_get_side_data(in, 
AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
-double peak = 0;
-
-if (sd) {
-AVContentLightMetadata *clm = (AVContentLightMetadata *)sd->data;
-peak = clm->MaxCLL / REFERENCE_WHITE;
-}
-
-sd = av_frame_get_side_data(in, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
- 

[FFmpeg-devel] [PATCH 3/4] vf_tonemap_opencl: Move update_metadata() to a shared file

2018-07-25 Thread Vittorio Giovara
---
 libavfilter/colorspace.c| 17 +
 libavfilter/colorspace.h|  1 +
 libavfilter/vf_tonemap_opencl.c | 19 +--
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/libavfilter/colorspace.c b/libavfilter/colorspace.c
index d6f6055401..c6682216d6 100644
--- a/libavfilter/colorspace.c
+++ b/libavfilter/colorspace.c
@@ -118,3 +118,20 @@ double ff_determine_signal_peak(AVFrame *in)
 
 return peak;
 }
+
+void ff_update_hdr_metadata(AVFrame *in, double peak)
+{
+AVFrameSideData *sd = av_frame_get_side_data(in, 
AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
+
+if (sd) {
+AVContentLightMetadata *clm = (AVContentLightMetadata *)sd->data;
+clm->MaxCLL = (unsigned)(peak * REFERENCE_WHITE);
+}
+
+sd = av_frame_get_side_data(in, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
+if (sd) {
+AVMasteringDisplayMetadata *metadata = (AVMasteringDisplayMetadata 
*)sd->data;
+if (metadata->has_luminance)
+metadata->max_luminance = av_d2q(peak * REFERENCE_WHITE, 1);
+}
+}
diff --git a/libavfilter/colorspace.h b/libavfilter/colorspace.h
index 75705ecfc7..936681815a 100644
--- a/libavfilter/colorspace.h
+++ b/libavfilter/colorspace.h
@@ -45,5 +45,6 @@ void ff_fill_rgb2xyz_table(const struct PrimaryCoefficients 
*coeffs,
double rgb2xyz[3][3]);
 
 double ff_determine_signal_peak(AVFrame *in);
+void ff_update_hdr_metadata(AVFrame *in, double peak);
 
 #endif
diff --git a/libavfilter/vf_tonemap_opencl.c b/libavfilter/vf_tonemap_opencl.c
index 7455ebb9fb..5da2333169 100644
--- a/libavfilter/vf_tonemap_opencl.c
+++ b/libavfilter/vf_tonemap_opencl.c
@@ -21,7 +21,6 @@
 #include "libavutil/bprint.h"
 #include "libavutil/common.h"
 #include "libavutil/imgutils.h"
-#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/mem.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
@@ -342,22 +341,6 @@ fail:
 return err;
 }
 
-static void update_metadata(AVFrame *in, double peak) {
-AVFrameSideData *sd = av_frame_get_side_data(in, 
AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
-
-if (sd) {
-AVContentLightMetadata *clm = (AVContentLightMetadata *)sd->data;
-clm->MaxCLL = (unsigned)(peak * REFERENCE_WHITE);
-}
-
-sd = av_frame_get_side_data(in, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
-if (sd) {
-AVMasteringDisplayMetadata *metadata = (AVMasteringDisplayMetadata 
*)sd->data;
-if (metadata->has_luminance)
-metadata->max_luminance =av_d2q(peak * REFERENCE_WHITE, 1);
-}
-}
-
 static int tonemap_opencl_filter_frame(AVFilterLink *inlink, AVFrame *input)
 {
 AVFilterContext*avctx = inlink->dst;
@@ -444,7 +427,7 @@ static int tonemap_opencl_filter_frame(AVFilterLink 
*inlink, AVFrame *input)
 
 av_frame_free();
 
-update_metadata(output, ctx->target_peak);
+ff_update_hdr_metadata(output, ctx->target_peak);
 
 av_log(ctx, AV_LOG_DEBUG, "Tone-mapping output: %s, %ux%u (%"PRId64").\n",
av_get_pix_fmt_name(output->format),
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/4] vf_tonemap: Update the default peak values

2018-07-25 Thread Vittorio Giovara
When there is no metadata attached to a frame, take into account both
the PQ and HLG transfers, and change the HLG default value to 10:
the value of 12 is the maximum range in scene referred light, but
the reference OOTF maps this from 0 to 1000 cd/m² on the ideal HLG
monitor.

This matches what vf_tonemap_opencl does.
---
 libavfilter/vf_tonemap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_tonemap.c b/libavfilter/vf_tonemap.c
index 10308bdb16..10fd7ea016 100644
--- a/libavfilter/vf_tonemap.c
+++ b/libavfilter/vf_tonemap.c
@@ -131,10 +131,10 @@ static double determine_signal_peak(AVFrame *in)
 peak = av_q2d(metadata->max_luminance) / REFERENCE_WHITE;
 }
 
-/* smpte2084 needs the side data above to work correctly
- * if missing, assume that the original transfer was arib-std-b67 */
+// For untagged source, use peak of 1 if SMPTE ST.2084
+// otherwise assume HLG with reference display peak 1000.
 if (!peak)
-peak = 12;
+peak = in->color_trc == AVCOL_TRC_SMPTE2084 ? 100.0f : 10.0f;
 
 return peak;
 }
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffv1dec: Ensure that pixel format constraints are respected

2018-07-19 Thread Vittorio Giovara
On Wed, Jul 18, 2018 at 10:37 PM, Michael Niedermayer <
mich...@niedermayer.cc> wrote:

> On Wed, Jul 18, 2018 at 11:03:47AM -0400, Derek Buitenhuis wrote:
> > On Wed, Jul 18, 2018 at 10:01 AM, Vittorio Giovara
> >  wrote:
> > >> this does not follow from what you write below. But more so this is
> not
> > >> how pixel formats were implemented in FFmpeg.
> > >> Where does this idea come from ?
> > >
> > > I found the following description of this pixel format pretty accurate:
> > > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv410.html
> > >
> > > I am not sure how non-mod4 sizes work, but probably codecs within
> ffmpeg
> > > take into account more alignment than necessary and make things work
> >
> > To expand on this (and other replies): The behavior in FFmpeg is very
> unexpected
> > for an API user who may wish to actually use the returned yuv410p data
> with an
> > application or other library that is *not* from FFmpeg, such as a
> renderer, or
> > external encoder lib, resizer, etc. Everything else on the planet
> > assumes that if
> > you have a buffer of a specific chroma subsampling type, you actually
> > have enough
> > data for it to be valid, with width/height that make it so. It's very
> > surprising when
> > you get back a set of buffers/width/height that don't make sense for a
> > given pixel
> > format, and is little to no documentation about why/how.
>
> I think i see what you mean.
> But iam not sure i understand how the proposed changes would be the ideal
> solution.
>

The change would guarantee that the received buffer respects the pixel
format constraints, and informs the users that there are some lines and
columns that ought to be cropped away in order to display the desired (or
valid) output.


> For example:
> lets assume we have a 3x3 image of 420 or 410 (be that from a jpeg or
> whatever)
>
> If we want to use this with software that that does support odd resolutions
> then it should just work. Theres no 4th column or row in the logic image
> that
> could be used.
>
> OTOH if we want to use this with softwate that does not support odd
> resolutions
> then its not going to work with a 3x3 (as odd) or 2x2 (looses a column and
> row)
> or a 4x4 (which has a column and a row that is uninitialized or black)
>

The problem with this reasoning is that things "happen" to work only
because ffmpeg decoders always over-allocate the output buffers and make
sure that there is addressable data. So as long as you stay "within" the
ffmpeg APIs, everything is perfect, however when you use such data in third
party libraries you risk running in these issues.


> what i mean is that the API by which one exports the width and height
> doesnt
> really affect this. To get from a 3x3 to something that actually works with
> external code which only supports even resolutions, something somewhere has
> to make it even and either scale, crop or fill in.
>

Well, IMO the API is low level enough that it would make sense to always
return the coded sizes and let the user crop to the desired resolution.
Yes several filters/encoders and user apps would need some tweaks, but
since this is the n-th case in which cropping produces broken results maybe
it's worth it.


> More specifically, saying that this 3x3 is a 4x4 image with crop is not
> really
> true as theres not neccesarily any data in the last column and row. And a
> filter or encoder using that anyway could produce bad output
>

There is always data in the last column and row, or it would have been
impossible to encode the buffer.
Whether it contains visible data or garbage is another story.

> I don't think "logic guarantees the buffer is mod4/aligned" is a
> > reasonable thing
> > to tell an API user in lieu of documentation for unexpected behavior.
>
> just posted a small patch to document this
>

I added some padding/cropping code to my program, so that's enough for now,
but i think a more thorough solution should be found.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] ffv1dec: Ensure that pixel format constraints are respected

2018-07-18 Thread Vittorio Giovara
On Tue, Jul 17, 2018 at 11:58:06PM +0200,Michael Niedermayer  wrote:
> > Its a consequence of the subsampling factor.
>
> this does not follow from what you write below. But more so this is not
> how pixel formats were implemented in FFmpeg.
> Where does this idea come from ?

I found the following description of this pixel format pretty accurate:
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv410.html

I am not sure how non-mod4 sizes work, but probably codecs within ffmpeg
take into account more alignment than necessary and make things work
within the ffmpeg libraries. Problems like this usually affect API users.

Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] svq1dec: Ensure that pixel format constraints are respected

2018-07-18 Thread Vittorio Giovara

2018-07-17 21:39 GMT+02:00, Carl Eugen Hoyos :
> 2018-07-17 21:39 GMT+02:00, Vittorio Giovara :
> > YUV410P requires that sizes are divisible by 4. There seem to be
> > some encoders that ignore that and encode a different value in
> > the bitstream itself. Handle that case by exporting the relative
> > cropping information.
>
>Can you provide a sample?
>
>Thank you, Carl Eugen

It is possible to generate samples affected by this bug with the svq1 or
ffv1 encoders, like this:

./ffmpeg -f lavfi -i testsrc -s 190x240 -t 1 -pix_fmt yuv410p -c:v ffv1 
output-yuv410p.avi

A prime example where the generated sample will fail is with a strict
scaler, such as zimg:

./ffmpeg -i output-yuv410p.avi -vf zscale -f null -

which will fail with

code 1027: image dimensions must be divisible by subsampling factor
Error while filtering: Generic error in an external library
Failed to inject frame into filter network: Generic error in an external library
Error while processing the decoded data for stream #0:0


While the proposed patch won't directly fix the issue with zscale, it will
offer tools for API users to adjust sizes accrodingly, and avoid it
altoether.

Regards,
Vittorio


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 2/2] ffv1dec: Ensure that pixel format constraints are respected

2018-07-17 Thread Vittorio Giovara
YUV410P requires that sizes are divisible by 4. There are some encoders
(including ffmpeg's) that ignore this constraint and encode a different
value in the bitstream itself. Handle that case by exporting the relative
cropping information.
---
Alternatively it is possible to always enforce mod4 sizes and call
it a day. I haven't explored ways to fix the encoder (or if other
decoders are affected).
Vittorio

 libavcodec/ffv1dec.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 7658a51685..8ff0f0deb8 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -971,6 +971,13 @@ static int decode_frame(AVCodecContext *avctx, void *data, 
int *got_frame, AVPac
 if ((ret = av_frame_ref(data, f->picture.f)) < 0)
 return ret;
 
+if (p->format == AV_PIX_FMT_YUV410P) {
+p->crop_left   = 0;
+p->crop_top= 0;
+p->crop_right  = FFALIGN(p->width,  4) - p->width;
+p->crop_bottom = FFALIGN(p->height, 4) - p->height;
+}
+
 *got_frame = 1;
 
 return buf_size;
@@ -1091,5 +1098,5 @@ AVCodec ff_ffv1_decoder = {
 .update_thread_context = ONLY_IF_THREADS_ENABLED(update_thread_context),
 .capabilities   = AV_CODEC_CAP_DR1 /*| AV_CODEC_CAP_DRAW_HORIZ_BAND*/ |
   AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_SLICE_THREADS,
-.caps_internal  = FF_CODEC_CAP_INIT_CLEANUP
+.caps_internal  = FF_CODEC_CAP_INIT_CLEANUP | 
FF_CODEC_CAP_EXPORTS_CROPPING,
 };
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/2] svq1dec: Ensure that pixel format constraints are respected

2018-07-17 Thread Vittorio Giovara
YUV410P requires that sizes are divisible by 4. There seem to be
some encoders that ignore that and encode a different value in
the bitstream itself. Handle that case by exporting the relative
cropping information.
---
Alternatively it is possible to always enforce mod4 sizes and call
it a day.
Vittorio

 libavcodec/svq1dec.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
index d3e60c3a4a..55047b43ce 100644
--- a/libavcodec/svq1dec.c
+++ b/libavcodec/svq1dec.c
@@ -663,7 +663,8 @@ static int svq1_decode_frame(AVCodecContext *avctx, void 
*data,
 return result;
 }
 
-result = ff_set_dimensions(avctx, s->width, s->height);
+/* sizes must be always disivible by 4 due to pixel format constraints */
+result = ff_set_dimensions(avctx, FFALIGN(s->width, 4), FFALIGN(s->height, 
4));
 if (result < 0)
 return result;
 
@@ -755,6 +756,11 @@ static int svq1_decode_frame(AVCodecContext *avctx, void 
*data,
 *got_frame = 1;
 result = buf_size;
 
+cur->crop_left   = 0;
+cur->crop_top= 0;
+cur->crop_right  = FFALIGN(s->width,  4) - s->width;
+cur->crop_bottom = FFALIGN(s->height, 4) - s->height;
+
 err:
 av_free(pmv);
 return result;
@@ -843,6 +849,7 @@ AVCodec ff_svq1_decoder = {
 .close  = svq1_decode_end,
 .decode = svq1_decode_frame,
 .capabilities   = AV_CODEC_CAP_DR1,
+.caps_internal  = FF_CODEC_CAP_EXPORTS_CROPPING,
 .flush  = svq1_flush,
 .pix_fmts   = (const enum AVPixelFormat[]) { AV_PIX_FMT_YUV410P,
  AV_PIX_FMT_NONE },
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] Use the same name for stereo3d frame/packet side data

2018-06-04 Thread Vittorio Giovara
---
 libavutil/frame.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 00215ac29a..deb9b6f334 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -819,7 +819,7 @@ const char *av_frame_side_data_name(enum 
AVFrameSideDataType type)
 switch(type) {
 case AV_FRAME_DATA_PANSCAN: return "AVPanScan";
 case AV_FRAME_DATA_A53_CC:  return "ATSC A53 Part 4 Closed 
Captions";
-case AV_FRAME_DATA_STEREO3D:return "Stereoscopic 3d metadata";
+case AV_FRAME_DATA_STEREO3D:return "Stereo 3D";
 case AV_FRAME_DATA_MATRIXENCODING:  return "AVMatrixEncoding";
 case AV_FRAME_DATA_DOWNMIX_INFO:return "Metadata relevant to a downmix 
procedure";
 case AV_FRAME_DATA_REPLAYGAIN:  return "AVReplayGain";
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavfi: a minor fix to tonemap peak detection.

2018-05-21 Thread Vittorio Giovara
On Mon, May 21, 2018 at 2:58 AM, Ruiling Song 
wrote:

> If the transfer was SMPTE2084, use the peak of 1 even if not tagged.
> Otherwise, we would assume it is HLG with a peak of 1200.
> Based on suggestion by Niklas Haas.
>
> Signed-off-by: Ruiling Song 
> ---
>  libavfilter/vf_tonemap.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/libavfilter/vf_tonemap.c b/libavfilter/vf_tonemap.c
> index 10308bd..ab45f2e 100644
> --- a/libavfilter/vf_tonemap.c
> +++ b/libavfilter/vf_tonemap.c
> @@ -131,10 +131,9 @@ static double determine_signal_peak(AVFrame *in)
>  peak = av_q2d(metadata->max_luminance) / REFERENCE_WHITE;
>  }
>
> -/* smpte2084 needs the side data above to work correctly
> - * if missing, assume that the original transfer was arib-std-b67 */
> +/* if not SMPTE2084, we would assume HLG */
>  if (!peak)
> -peak = 12;
> +peak = in->color_trc == AVCOL_TRC_SMPTE2084 ? 100 : 12;
>
>  return peak;
>  }
> --
> 2.7.4
>
>
seems reasonable
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 01/24] avcodec: add color_range to AVCodec struct

2018-05-01 Thread Vittorio Giovara
-- 

On Wed May 2 01:45:47 EEST 2018 Rostislav Pehlivanov wrote:
> I agree with you, they need to be gone. The purpose of the patchset also
> isn't just to get rid of them, its to have a good api to handle color
> ranges and how it ought to be handled by filters and codecs. His only
> objection was that there's a stigma to adding more fields to avctx, which
> can't be avoided in this case _because_ the color range is an essential
> property of images.

Well no, let's step back a little.

First of all there is no stigma about adding fields to AVCodecContext,
in fact, the more, the merrier, right?

Secondly, there _is_ concern about adding a field to AVCodec (not avctx),
since is a delicate point of entry, and very sensitive to ABI (which in fact
was broken in the first iteration of this patch).

Finally, I immensely disagree that this is a good API at all, just throwing
fields to structure is rarely a good approach to anything. On a separate note
I also disagree that color_range (despite its importance) is an "essential
property of images", at least not any more than the other color propeties:
it is simply being noticed because there are special purpose enum values, but
that doesn't mean that the other properties will work "just fine". Because
they don't.
The transfer characteristics for example is a much more impactful and
delicate property that can affect the the color of the image in a much
more profund way than color range.
Similar arguments could be made for primaries and matrix.

As much as you may hate hear it, this proposed filtering system does *not*
belong to neither lavfi, lavf or lavc, but rather to the scaling library.
Unfortunately the one used by default, swscale, is a monster spaghetti code
and too difficult to work around. That however should not affect the design
of APIs used in the other libraries: if swscale is not suited anymore for
its job it should be replaced, you shouldn't be adding workarounds to AVCodec.


Paul, if warnings are what's bothering you, just drop the warnings, instead
of introducing a years long, user facing, difficult to remove API.
Since I fear this message will be ignored too, it will be my last one on this
topic as well.

Regards,
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 01/24] avcodec: add color_range to AVCodec struct

2018-05-01 Thread Vittorio Giovara
--

On 5/1/2018 4:39 PM, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/avcodec.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index fb0c6fae70..3a8f69243c 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3433,6 +3433,7 @@ typedef struct AVCodec {
>  uint8_t max_lowres; ///< maximum value for lowres 
> supported by the decoder
>  const AVClass *priv_class;  ///< AVClass for the private 
> context
>  const AVProfile *profiles;  ///< array of recognized 
> profiles, or NULL if unknown, array is terminated by {FF_PROFILE_UNKNOWN}
> +const enum AVColorRange *color_ranges;  ///< array of supported color 
> ranges by encoder, or NULL if unknown, array is terminated by 
> AVCOL_RANGE_UNSPECIFIED
>  
>  /**
>   * Group name of the codec implementation.
> 

While I appreciate this effort to remove the year-long deprecated J-pixel
format, I have a feeling that this is not the right way to do it.

I have no doubt that the code presented here works as expected, however
adding YetAnotherField to the AVCodec structure is a slippery road. Namely
only adding color range as an extra feature of the pixel format is incomplete.

We should add every other single color parameter in this structure, insert
the appropriate conversion filters (which swscale is not fully able to
produce) and handle correct format negotiation across the chain, which is
not exactly trivial (ie. which color space should be favoured? which one has
a larger gamut? and so on).

This creates a precedent that, I think, is bad API-wise and user-wise.
I would rather keep the J-format around than creating a years long user-facing
problem. Additionally having this field (or these fields if we are to include
the other color properties) around makes the intrisic problem even harder
to properly fix.

I am aware that the goal of this patchset is not to properly support all
color conversion properties, and it's just to being able to drop the J-formats,
but I hope that this feedback will give a larger picture of the problem
involved, that it will help you in making the right decision about this set.

Regards
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] aac: Rework extradata parsing code

2018-04-17 Thread Vittorio Giovara
- enable the parsing code
- use the new buffer instead of replacing the context one
- do not push/pop configuration, just discard the exiting one
- propagate errors correctly
---

ping


 libavcodec/aacdec_template.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index cf97181092..0c899285dd 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -3324,20 +3324,14 @@ static int aac_decode_frame(AVCodecContext
*avctx, void *data,
AV_PKT_DATA_JP_DUALMONO,
_dualmono_size);

-if (new_extradata && 0) {
-av_free(avctx->extradata);
-avctx->extradata = av_mallocz(new_extradata_size +
-  AV_INPUT_BUFFER_PADDING_SIZE);
-if (!avctx->extradata)
-return AVERROR(ENOMEM);
-avctx->extradata_size = new_extradata_size;
-memcpy(avctx->extradata, new_extradata, new_extradata_size);
-push_output_configuration(ac);
-if (decode_audio_specific_config(ac, ac->avctx, >oc[1].m4ac,
- avctx->extradata,
- avctx->extradata_size*8LL, 1) < 0) {
-pop_output_configuration(ac);
-return AVERROR_INVALIDDATA;
+if (new_extradata) {
+/* discard previous configuration */
+ac->oc[1].status = OC_NONE;
+err = decode_audio_specific_config(ac, ac->avctx, >oc[1].m4ac,
+   new_extradata,
+   new_extradata_size * 8LL, 1);
+if (err < 0) {
+return err;
 }
 }

-- 
2.17.0


-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] aac: Rework extradata parsing code

2018-04-12 Thread Vittorio Giovara
- enable the parsing code
- use the new buffer instead of replacing the context one
- do not push/pop configuration, just discard the exiting one
- propagate errors correctly
---
 libavcodec/aacdec_template.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/libavcodec/aacdec_template.c b/libavcodec/aacdec_template.c
index cf97181092..0c899285dd 100644
--- a/libavcodec/aacdec_template.c
+++ b/libavcodec/aacdec_template.c
@@ -3324,20 +3324,14 @@ static int aac_decode_frame(AVCodecContext *avctx, void 
*data,
AV_PKT_DATA_JP_DUALMONO,
_dualmono_size);
 
-if (new_extradata && 0) {
-av_free(avctx->extradata);
-avctx->extradata = av_mallocz(new_extradata_size +
-  AV_INPUT_BUFFER_PADDING_SIZE);
-if (!avctx->extradata)
-return AVERROR(ENOMEM);
-avctx->extradata_size = new_extradata_size;
-memcpy(avctx->extradata, new_extradata, new_extradata_size);
-push_output_configuration(ac);
-if (decode_audio_specific_config(ac, ac->avctx, >oc[1].m4ac,
- avctx->extradata,
- avctx->extradata_size*8LL, 1) < 0) {
-pop_output_configuration(ac);
-return AVERROR_INVALIDDATA;
+if (new_extradata) {
+/* discard previous configuration */
+ac->oc[1].status = OC_NONE;
+err = decode_audio_specific_config(ac, ac->avctx, >oc[1].m4ac,
+   new_extradata,
+   new_extradata_size * 8LL, 1);
+if (err < 0) {
+return err;
 }
 }
 
-- 
2.17.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avformat/rtpenc_chain: use the proper function to free AVFormatContext

2018-04-06 Thread Vittorio Giovara
Yes the order of operations is a problem in a generic matrix, but for a
display matrix the order is more or less consolidated in a defacto standard:
check for flip first, then rotation. We have the same pattern in h264 and
hevc decoder for the rotation side data.

You are right that the description of the API does not convey the order
and that it should be better documented, although I don't have a specific
standard to quote (besides what is already mentioned in display.h).

> > The "special cases" are also the most common operations that every player
> > implements so I think it makes sense to have them readily available, with
> > as few calls as possible.
> 
> With what i suggest, not sure i explained it well enough
> there would be a single call to provide a struct or array of 4 scalars
> a 90 degree to the right rotation would for example have a
> {90, 1, 1, 0} and could be checked for by memcmp() or a more specialized
> function that returns a scalar "difference"
> so testing for these common operations should be very simple and compact.
> Testing for another angle of rotation or other transform would be similarly
> simple.


Well the only downside of that is that we are replacing a well-known set of
instructions coded in a 3x3 matrix with a ffmpeg-only 4x1 array of integers.
You would need special code and documentation to parse either the matrix or
the array, so it kinda defies my attempt at simplifying the API: I feel like
having a negative scale factor to represent a flip is as complex as having
euclidean math to compute the rotation angle when the allowed orientations
are just 4.

On the other hand, I'm not strongly advocating for one way of another, if
you could maybe point me to the right direction and share some code on how
you mean the matrix parsing should be carried out I'll try to prepare a
second version of this patch.

Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] display: Add AVDisplayOrientation API

2018-04-05 Thread Vittorio Giovara
---
> On Wed, Apr 04, 2018 at 05:30:24PM +0200, Vittorio Giovara wrote:
> >  libavutil/display.c | 92 
> > +
> >  libavutil/display.h | 53 ++
> >  2 files changed, 145 insertions(+)

> It might be more usefull to fully factorize the matrix than these special 
> cases
> 
> for example the matrix can be described by these 4 scalars
> 1. rotation
> 2. horizontal scale
> 3. vertical scale
> 4. shear

With the proposed API the matrix is already factorized: a mask is returned
containing a set of  operation. Special cases such as custom rotation is
already factorized, as in you need to call an additional function on the
same matrix to know more about the rotation angle. On the other hand the
most common cases are already ready to be used right away.

Do you think it should just expose the fact that it is a rotation and always
require the user to inspect the matrix again? Note that hflip and vflip can't
be described with pure rotation angles. I feel like this would complicate
the API.

For any missing operation, such as transpose, shear, and scaling, this API
should be extensible enoughe, so that if anybody wants to add enum values
and functions to retrieve the desired operation, it can be done.

> (there are more parameters like translation and z specific changes but 
> IIUC
>   these have no meaning ?)

Correct given that video right now is a 2d surface we can ignore anything
related to the depth axis.

> Note fliping in above would be a negative scale value
> shear could be specified as the angle between the x/y basis vectors
> 
> The reason i suggest this is that these 4 values are easier to understand
> for a human and should allow reconstructing an equivalent transform 
> matrix.
> While at the same time not limiting things to a subset of special cases
> 

The "special cases" are also the most common operations that every player
implements so I think it makes sense to have them readily available, with
as few calls as possible.

> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

(I'm trying something different wrt my mail client, I hope it doesn't botch
 the threaded reply again).
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] display: Add AVDisplayOrientation API

2018-04-04 Thread Vittorio Giovara
The transformation operations that can be described by a display matrix
are not limited to pure rotation, but include horizontal and vertical
flip, as well as transpose and antitranspose. Unfortunately the current
API can only return a rotation angle in degrees, and is not designed to
detect flip operations or a combination of rotation and flip.

So implement an additional API to analyze the display matrix and return
the most common rotation operations (multiples of 90º) as well flips or
a combination thereof. This function returns a bitfield mask composed of
AVDisplayOrientation elements that describe which rendering operations
should be performed on the frame. The existing API is still available
and useful in case of custom rotations.

Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
---
Note: the new operations describe a clockwise rotation, while the
old API provided a counterclockwise rotation. I always felt this was
a mistake as it's counterintuitive and suprising to new users, so I
didn't want to cargo-cult it to a new API. What do people think about it?

See also https://github.com/FFMS/ffms2/issues/317 for flipped samples, code,
and additional discussion.

Missing changelog entry and version bump.
Vittorio

 libavutil/display.c | 92 +
 libavutil/display.h | 53 ++
 2 files changed, 145 insertions(+)

diff --git a/libavutil/display.c b/libavutil/display.c
index f7500948ff..839961ec20 100644
--- a/libavutil/display.c
+++ b/libavutil/display.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 
+#include "avstring.h"
 #include "display.h"
 #include "mathematics.h"
 
@@ -73,3 +74,94 @@ void av_display_matrix_flip(int32_t matrix[9], int hflip, 
int vflip)
 for (i = 0; i < 9; i++)
 matrix[i] *= flip[i % 3];
 }
+
+uint32_t av_display_orientation_get(int32_t matrix_src[9])
+{
+int32_t matrix[9];
+uint32_t orientation = 0;
+int64_t det = (int64_t)matrix_src[0] * matrix_src[4] - 
(int64_t)matrix_src[1] * matrix_src[3];
+
+/* Duplicate matrix so that the input one is not modified in case of flip. 
*/
+memcpy(matrix, matrix_src, sizeof(*matrix_src) * 9);
+
+if (det < 0) {
+/* Always assume an horizontal flip for simplicity, it can be
+ * changed later if rotation is 180º. */
+orientation = AV_FLIP_HORIZONTAL;
+av_display_matrix_flip(matrix, 1, 0);
+}
+
+if (matrix[1] == (1 << 16) && matrix[3] == -(1 << 16)) {
+orientation |= AV_ROTATION_90;
+} else if (matrix[0] == -(1 << 16) && matrix[4] == -(1 << 16)) {
+if (det < 0)
+orientation = AV_FLIP_VERTICAL;
+else
+orientation |= AV_ROTATION_180;
+} else if (matrix[1] == -(1 << 16) && matrix[3] == (1 << 16)) {
+orientation |= AV_ROTATION_270;
+} else if (matrix[0] == (1 << 16) && matrix[4] == (1 << 16)) {
+orientation |= AV_IDENTITY;
+} else {
+orientation |= AV_ROTATION_CUSTOM;
+}
+
+return orientation;
+}
+
+void av_display_orientation_set(int32_t matrix[9], uint32_t orientation, 
double angle)
+{
+int hflip = !!(orientation & AV_FLIP_HORIZONTAL);
+int vflip = !!(orientation & AV_FLIP_VERTICAL);
+
+memset(matrix, 0, sizeof(*matrix) * 9);
+matrix[8] = 1 << 30;
+
+if (orientation & AV_IDENTITY) {
+matrix[0] = 1 << 16;
+matrix[4] = 1 << 16;
+} else if (orientation & AV_ROTATION_90) {
+matrix[1] = 1 << 16;
+matrix[3] = -(1 << 16);
+} else if (orientation & AV_ROTATION_180) {
+matrix[0] = -(1 << 16);
+matrix[4] = -(1 << 16);
+} else if (orientation & AV_ROTATION_270) {
+matrix[1] = -(1 << 16);
+matrix[3] = 1 << 16;
+} else if (orientation & AV_ROTATION_CUSTOM) {
+av_display_rotation_set(matrix, angle);
+}
+
+av_display_matrix_flip(matrix, hflip, vflip);
+}
+
+void av_display_orientation_name(uint32_t orientation, char *buf, size_t 
buf_size)
+{
+if (orientation == 0) {
+av_strlcpy(buf, "identity", buf_size);
+return;
+}
+
+if (orientation & AV_ROTATION_90)
+av_strlcpy(buf, "rotation_90", buf_size);
+else if (orientation & AV_ROTATION_180)
+av_strlcpy(buf, "rotation_180", buf_size);
+else if (orientation & AV_ROTATION_270)
+av_strlcpy(buf, "rotation_270", buf_size);
+else if (orientation & AV_ROTATION_CUSTOM)
+av_strlcpy(buf, "rotation_custom", buf_size);
+else
+buf[0] = '\0';
+
+if (orientation & AV_FLIP_HORIZONTAL) {
+if (buf[0] != '\0')
+av_strlcat(buf, "+", buf_size);
+av_strlcat(buf, &q

Re: [FFmpeg-devel] [PATCH 2/2] pixdesc: deprecate AV_PIX_FMT_FLAG_PSEUDOPAL

2018-04-03 Thread Vittorio Giovara
On Thu, Mar 29, 2018 at 03:30:43PM +0200, wm4 wrote:
>* PSEUDOPAL pixel formats are not paletted, but carried a palette with the
*>* intention of allowing code to treat unpaletted formats as paletted. The
*>* palette simply mapped the byte values to the resulting RGB values,
*>* making it some sort of LUT for RGB conversion.
*> >* It was used for 1 byte formats only: RGB4_BYTE, BGR4_BYTE, RGB8, BGR8,
*>* GRAY8. The first 4 are awfully obscure, used only by some ancient bitmap
*>* formats. The last one, GRAY8, is more common, but its treatment is
*>* grossly incorrect. It considers full range GRAY8 only, so GRAY8 coming
*>* from typical Y video planes was not mapped to the correct RGB values.
*>* Also, nothing actually used the PSEUDOPAL palette data, except xwdenc.
*>* All other code had to treat it as a special case, just to ignore or
*>* propagate palette data.
*> >* In conclusion, this was just a very strange old hack that has no real
*>* justification to exist. It's negatively useful, because API users who
*>* allocate their own pixel data have to be aware that they need to
*>* allocate the palette, or FFmpeg will crash on them in _some_ situations.
*>* On top of this, there was no API to allocate the pseuo palette outside
*>* of av_frame_get_buffer(). (avpriv_set_systematic_pal2() was not public,
*>* which is good, because GRAY8 treatment is broken.)
*> >* This patch not only deprecates AV_PIX_FMT_FLAG_PSEUDOPAL, but also makes
*>* the pseudo palette optional. Nothing accesses it anymore, though if it's
*>* set, it's propagated. It's still allocated and initialized for
*>* compatibility with API users that rely on this feature. But new API
*>* users do not need to allocate it. This was an explicit goal of this
*>* patch.
*> >* Most changes replace AV_PIX_FMT_FLAG_PSEUDOPAL with FF_PSEUDOPAL. I
*>* first tried #ifdefing all code, but it was a mess. The FF_PSEUDOPAL
*>* macro reduces the mess, and still allows defining FF_API_PSEUDOPAL to 0.
*> >* Passes FATE with FF_API_PSEUDOPAL enabled and disabled. In addition,
*>* FATE passes with FF_API_PSEUDOPAL set to 1, but with allocation
*>* functions manually changed to not allocating a palette.
*>* ---*

set looks good to me
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter/vf_scale: Do not set YUV color range for RGB formats

2018-04-02 Thread Vittorio Giovara
On 3/20/18, Michael Niedermayer http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> wrote:
>* Signed-off-by: Michael Niedermayer >
*>* ---
*>*  libavfilter/vf_scale.c | 7 ++-
*>*  1 file changed, 6 insertions(+), 1 deletion(-)
*>>* diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
*>* index 9f45032e85..2f6fa4791d 100644
*>* --- a/libavfilter/vf_scale.c
*>* +++ b/libavfilter/vf_scale.c
*>* @@ -407,6 +407,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
*>*  AVFilterLink *outlink = link->dst->outputs[0];
*>*  AVFrame *out;
*>*  const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
*>* +const AVPixFmtDescriptor *out_desc =
*>* av_pix_fmt_desc_get(outlink->format);
*>*  char buf[32];
*>*  int in_range;
*>>* @@ -497,7 +498,11 @@ static int filter_frame(AVFilterLink *link, AVFrame
*>* *in)
*>*   table, out_full,
*>*   brightness, contrast, saturation);
*>>* -out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
*>* +// color_range describes YUV, it is undefined for RGB
*>* +if ((out_desc->flags & AV_PIX_FMT_FLAG_RGB) &&
*>* out_desc->nb_components != 1) {
*>* +out->color_range = AVCOL_RANGE_UNSPECIFIED;
*>* +} else
*>* +out->color_range = out_full ? AVCOL_RANGE_JPEG :
*>* AVCOL_RANGE_MPEG;
*>*  }
*>>*  av_reduce(>sample_aspect_ratio.num,
>sample_aspect_ratio.den,
*>* --
*>* 2.16.2
*

shouldn't color_range be always set to FULL when outputting RGB? This would
simplify conversions towards YUV (which assumes UNSPEC == LIMITED) and
something that other libraries already assume (ie zimg).
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] libavcodec/mjpeg: remove YUVJ mentions

2017-12-11 Thread Vittorio Giovara
>* On 12/8/17, Paul B Mahol http://gmail.com>>*
>> On 12/8/17, Vittorio Giovara > <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> wrote:
>*> If we were to break this feature, I'd suggest going the full route of
*>*> adding a PixelFormaton and work on a sws alternative (one is allowed to
*>*> dream no?).
*>
> This is step to right direction, why would adding yet another API be better
> solution?
>
> J formats are hack - misfeature - most obvious reason why nobody added
> 10bit J formats, or one of alpha. Calling it otherwise, points to severe
> lack of understanding of problem.

I am perfectly aware that the J formats are a hack, that's why it would be
nice to avoid introducing more hacks to get rid of them.
As it has been pointed out in the other thread, simply adding .color_range
does not properly solve this problem, it is a breaking change without
proper deprecation period, and in general it seems like not a good idea
API-wise to add an endless number of fields to AVCodec.
Like I said, having dealt with the problem in the past, the only way I
suggest to go forward is decoupling codec/filter negotiation from pixel
formats and use a different scaling/color conversion library.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 01/27] avcodec: add color_range to AVCodec struct and use it

2017-12-11 Thread Vittorio Giovara
>* On Sat, Dec 9, 2017 at 10:37 AM, Paul B Mahol > wrote:*
> Endless bikeshed...
Being dismissive to valid points is not good for project health.

> I hate how people can be so ignorant these days.

Can you tone it down a notch maybe? I can't envision a scenario in which
saying this would help
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/2] libavcodec/mjpeg: remove YUVJ mentions

2017-12-08 Thread Vittorio Giovara
> On 12/8/17, Paul B Mahole * >* wrote:
>*> This will basically break everyone encoding mjpeg right now, since it
*>*> suddenly only accepts different formats without any common-ground
*>*> before/after.
*>*> Furthermore, there is no replacement for the indication that this
*>*> encoder wants full-range data, which the old pixfmts indicated.
*>
> So I will add .color_range to AVCodec
> 0 means encoder supports both.
>
> Is that ok?

I tried in the past and it is indeed possible, however it doesn't really
work: going that route you'll end up adding all the color properties,
including chroma location.
As much as I despise the J formats, it's a hacky solution at best leading
to a very confusing API, making the negotiation and conversion unnecessary
complex.
If we were to break this feature, I'd suggest going the full route of
adding a PixelFormaton and work on a sws alternative (one is allowed to
dream no?).
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] vf_zscale: Fix alpha destination graph for floating point pixel formats

2017-12-07 Thread Vittorio Giovara
This was setting the input pixel type instead of the output one,
leading to incorrect data being fed to the library.

Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
---
 libavfilter/vf_zscale.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 865910bd87..6e1d36cb4c 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -615,7 +615,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 s->alpha_dst_format.width = out->width;
 s->alpha_dst_format.height = out->height;
 s->alpha_dst_format.depth = odesc->comp[0].depth;
-s->alpha_dst_format.pixel_type = (desc->flags & 
AV_PIX_FMT_FLAG_FLOAT) ? ZIMG_PIXEL_FLOAT : odesc->comp[0].depth > 8 ? 
ZIMG_PIXEL_WORD : ZIMG_PIXEL_BYTE;
+s->alpha_dst_format.pixel_type = (odesc->flags & 
AV_PIX_FMT_FLAG_FLOAT) ? ZIMG_PIXEL_FLOAT : odesc->comp[0].depth > 8 ? 
ZIMG_PIXEL_WORD : ZIMG_PIXEL_BYTE;
 s->alpha_dst_format.color_family = ZIMG_COLOR_GREY;
 
 zimg_filter_graph_free(s->alpha_graph);
-- 
2.14.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 1/2] vf_zscale: Relax color properties maximum bounds

2017-11-28 Thread Vittorio Giovara
This simplifies adding new values, which are already validated elsewhere.

Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
---
 libavfilter/vf_zscale.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 09fd842fe5..972f720ee6 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -735,8 +735,8 @@ static const AVOption zscale_options[] = {
 { "unknown",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = -1}, 0, 0, FLAGS, "range" },
 { "tv",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_RANGE_LIMITED}, 0, 0, FLAGS, "range" },
 { "pc",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_RANGE_FULL},0, 0, FLAGS, "range" },
-{ "primaries", "set color primaries", OFFSET(primaries), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, ZIMG_PRIMARIES_ST432_1, FLAGS, "primaries" },
-{ "p", "set color primaries", OFFSET(primaries), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, ZIMG_PRIMARIES_ST432_1, FLAGS, "primaries" },
+{ "primaries", "set color primaries", OFFSET(primaries), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, INT_MAX, FLAGS, "primaries" },
+{ "p", "set color primaries", OFFSET(primaries), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, INT_MAX, FLAGS, "primaries" },
 { "input",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = -1}, 0, 0, FLAGS, "primaries" },
 { "709",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_709}, 0, 0, FLAGS, "primaries" },
 { "unspecified",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_UNSPECIFIED}, 0, 0, FLAGS, "primaries" },
@@ -749,8 +749,8 @@ static const AVOption zscale_options[] = {
 { "smpte240m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_240M},0, 0, FLAGS, "primaries" },
 { "bt2020",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_2020},0, 0, FLAGS, "primaries" },
 { "smpte432", 0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_ST432_1}, 0, 0, FLAGS, "primaries" },
-{ "transfer", "set transfer characteristic", OFFSET(trc), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, ZIMG_TRANSFER_ARIB_B67, FLAGS, "transfer" },
-{ "t","set transfer characteristic", OFFSET(trc), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, ZIMG_TRANSFER_ARIB_B67, FLAGS, "transfer" },
+{ "transfer", "set transfer characteristic", OFFSET(trc), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, INT_MAX, FLAGS, "transfer" },
+{ "t","set transfer characteristic", OFFSET(trc), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, INT_MAX, FLAGS, "transfer" },
 { "input",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = -1}, 0, 0, FLAGS, "transfer" },
 { "709",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_709}, 0, 0, FLAGS, "transfer" },
 { "unspecified",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_UNSPECIFIED}, 0, 0, FLAGS, "transfer" },
@@ -767,8 +767,8 @@ static const AVOption zscale_options[] = {
 { "smpte2084",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_ST2084},  0, 0, FLAGS, "transfer" },
 { "iec61966-2-1", 0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_IEC_61966_2_1},0, 0, FLAGS, "transfer" },
 { "arib-std-b67", 0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_ARIB_B67},0, 0, FLAGS, "transfer" },
-{ "matrix", "set colorspace matrix", OFFSET(colorspace), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, ZIMG_MATRIX_2020_CL, FLAGS, "matrix" },
-{ "m",  "set colorspace matrix", OFFSET(colorspace), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, ZIMG_MATRIX_2020_CL, FLAGS, "matrix" },
+{ "matrix", "set colorspace matrix", OFFSET(colorspace), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, INT_MAX, FLAGS, "matrix" },
+{ "m",  "set colorspace matrix", OFFSET(colorspace), AV_OPT_TYPE_INT, 
{.i64 = -1}, -1, INT_MAX, FLAGS, "matrix" },
 {

[FFmpeg-devel] [PATCH 2/2] vf_zscale: Add more supported input properties

2017-11-28 Thread Vittorio Giovara
Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
---
This version should be more complete.
Regarding configure changes, this library is not packaged by any distribution
that I could find, so users will just need to use zimg HEAD or any stable
snapshot (2.6.2).
Vittorio

 libavfilter/vf_zscale.c | 51 +
 1 file changed, 51 insertions(+)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 972f720ee6..865910bd87 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -353,16 +353,26 @@ static int convert_matrix(enum AVColorSpace colorspace)
 return ZIMG_MATRIX_709;
 case AVCOL_SPC_UNSPECIFIED:
 return ZIMG_MATRIX_UNSPECIFIED;
+case AVCOL_SPC_FCC:
+return ZIMG_MATRIX_FCC;
 case AVCOL_SPC_BT470BG:
 return ZIMG_MATRIX_470BG;
 case AVCOL_SPC_SMPTE170M:
 return ZIMG_MATRIX_170M;
+case AVCOL_SPC_SMPTE240M:
+return ZIMG_MATRIX_240M;
 case AVCOL_SPC_YCGCO:
 return ZIMG_MATRIX_YCGCO;
 case AVCOL_SPC_BT2020_NCL:
 return ZIMG_MATRIX_2020_NCL;
 case AVCOL_SPC_BT2020_CL:
 return ZIMG_MATRIX_2020_CL;
+case AVCOL_SPC_CHROMA_DERIVED_NCL:
+return ZIMG_MATRIX_CHROMATICITY_DERIVED_NCL;
+case AVCOL_SPC_CHROMA_DERIVED_CL:
+return ZIMG_MATRIX_CHROMATICITY_DERIVED_CL;
+case AVCOL_SPC_ICTCP:
+return ZIMG_MATRIX_ICTCP;
 }
 return ZIMG_MATRIX_UNSPECIFIED;
 }
@@ -374,10 +384,22 @@ static int convert_trc(enum AVColorTransferCharacteristic 
color_trc)
 return ZIMG_TRANSFER_UNSPECIFIED;
 case AVCOL_TRC_BT709:
 return ZIMG_TRANSFER_709;
+case AVCOL_TRC_GAMMA22:
+return ZIMG_TRANSFER_470_M;
+case AVCOL_TRC_GAMMA28:
+return ZIMG_TRANSFER_470_BG;
 case AVCOL_TRC_SMPTE170M:
 return ZIMG_TRANSFER_601;
+case AVCOL_TRC_SMPTE240M:
+return ZIMG_TRANSFER_240M;
 case AVCOL_TRC_LINEAR:
 return ZIMG_TRANSFER_LINEAR;
+case AVCOL_TRC_LOG:
+return ZIMG_TRANSFER_LOG_100;
+case AVCOL_TRC_LOG_SQRT:
+return ZIMG_TRANSFER_LOG_316;
+case AVCOL_TRC_IEC61966_2_4:
+return ZIMG_TRANSFER_IEC_61966_2_4;
 case AVCOL_TRC_BT2020_10:
 return ZIMG_TRANSFER_2020_10;
 case AVCOL_TRC_BT2020_12:
@@ -399,14 +421,26 @@ static int convert_primaries(enum AVColorPrimaries 
color_primaries)
 return ZIMG_PRIMARIES_UNSPECIFIED;
 case AVCOL_PRI_BT709:
 return ZIMG_PRIMARIES_709;
+case AVCOL_PRI_BT470M:
+return ZIMG_PRIMARIES_470_M;
+case AVCOL_PRI_BT470BG:
+return ZIMG_PRIMARIES_470_BG;
 case AVCOL_PRI_SMPTE170M:
 return ZIMG_PRIMARIES_170M;
 case AVCOL_PRI_SMPTE240M:
 return ZIMG_PRIMARIES_240M;
+case AVCOL_PRI_FILM:
+return ZIMG_PRIMARIES_FILM;
 case AVCOL_PRI_BT2020:
 return ZIMG_PRIMARIES_2020;
+case AVCOL_PRI_SMPTE428:
+return ZIMG_PRIMARIES_ST428;
+case AVCOL_PRI_SMPTE431:
+return ZIMG_PRIMARIES_ST431_2;
 case AVCOL_PRI_SMPTE432:
 return ZIMG_PRIMARIES_ST432_1;
+case AVCOL_PRI_JEDEC_P22:
+return ZIMG_PRIMARIES_EBU3213_E;
 }
 return ZIMG_PRIMARIES_UNSPECIFIED;
 }
@@ -745,10 +779,16 @@ static const AVOption zscale_options[] = {
 { "2020", 0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_2020},0, 0, FLAGS, "primaries" },
 { "unknown",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_UNSPECIFIED}, 0, 0, FLAGS, "primaries" },
 { "bt709",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_709}, 0, 0, FLAGS, "primaries" },
+{ "bt470m",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_470_M},   0, 0, FLAGS, "primaries" },
+{ "bt470bg",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_470_BG},  0, 0, FLAGS, "primaries" },
 { "smpte170m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_170M},0, 0, FLAGS, "primaries" },
 { "smpte240m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_240M},0, 0, FLAGS, "primaries" },
+{ "film", 0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_FILM},0, 0, FLAGS, "primaries" },
 { "bt2020",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_2020},0, 0, FLAGS, "primaries" },
+{ "smpte428", 0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_ST428},   0, 0, FLAGS, "primaries" },
+{ "smpte

Re: [FFmpeg-devel] [PATCH] mov: Support mdcv and clli boxes for mastering display an color light level

2017-11-27 Thread Vittorio Giovara
>* On 11/27/2017 5:20 PM, James Almer wrote:*> Pointless duplicate atoms :/ At 
>least these don't use fixed point
> values, so they are nicer.

indeed

> I assume no file will have both smdm and mdcv, or coll and clli, so
> reusing the MOVStreamContext fields should be ok, but maybe free the
> pointers first?
that's very unlikely as one set is defined for iso files (.mp4) and
the other one for qtff files (.mov).
if you think that is still needed, I'll go ahead and add that to a
separate patch

> Can't comment about the implementation as i don't have access to the spec.

if it helps, mediainfo got these properties too
https://github.com/MediaArea/MediaInfoLib/commit/818f2fd4fdfadf2dbb8cc010b1c35e8cf3ada504
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] mov: Support mdcv and clli boxes for mastering display an color light level

2017-11-27 Thread Vittorio Giovara
> On 11/27/2017 5:20 PM, James Almer wrote:
> Where are these two defined? Because
> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md
> describes coll and smdm, and those are already supported.

these are unrelated to vp9 (or mp4), they are the mov-only atoms used by
some upcoming tools to signal hdr
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] mov: Support mdcv and clli boxes for mastering display an color light level

2017-11-27 Thread Vittorio Giovara
Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com>
---
 libavformat/mov.c | 71 +++
 1 file changed, 71 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 79023ef369..bb463017a3 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5072,6 +5072,51 @@ static int mov_read_smdm(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 return 0;
 }
 
+static int mov_read_mdcv(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+MOVStreamContext *sc;
+const int mapping[3] = {1, 2, 0};
+const int chroma_den = 5;
+const int luma_den = 1;
+int i;
+
+if (c->fc->nb_streams < 1)
+return AVERROR_INVALIDDATA;
+
+sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
+
+if (atom.size < 24) {
+av_log(c->fc, AV_LOG_ERROR, "Invalid Mastering Display Color Volume 
box\n");
+return AVERROR_INVALIDDATA;
+}
+
+sc->mastering = av_mastering_display_metadata_alloc();
+if (!sc->mastering)
+return AVERROR(ENOMEM);
+
+for (i = 0; i < 3; i++) {
+const int j = mapping[i];
+sc->mastering->display_primaries[j][0].num = avio_rb16(pb);
+sc->mastering->display_primaries[j][0].den = chroma_den;
+sc->mastering->display_primaries[j][1].num = avio_rb16(pb);
+sc->mastering->display_primaries[j][1].den = chroma_den;
+}
+sc->mastering->white_point[0].num = avio_rb16(pb);
+sc->mastering->white_point[0].den = chroma_den;
+sc->mastering->white_point[1].num = avio_rb16(pb);
+sc->mastering->white_point[1].den = chroma_den;
+
+sc->mastering->max_luminance.num = avio_rb32(pb);
+sc->mastering->max_luminance.den = luma_den;
+sc->mastering->min_luminance.num = avio_rb32(pb);
+sc->mastering->min_luminance.den = luma_den;
+
+sc->mastering->has_luminance = 1;
+sc->mastering->has_primaries = 1;
+
+return 0;
+}
+
 static int mov_read_coll(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
 MOVStreamContext *sc;
@@ -5104,6 +5149,30 @@ static int mov_read_coll(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 return 0;
 }
 
+static int mov_read_clli(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+MOVStreamContext *sc;
+
+if (c->fc->nb_streams < 1)
+return AVERROR_INVALIDDATA;
+
+sc = c->fc->streams[c->fc->nb_streams - 1]->priv_data;
+
+if (atom.size < 4) {
+av_log(c->fc, AV_LOG_ERROR, "Empty Content Light Level Info box\n");
+return AVERROR_INVALIDDATA;
+}
+
+sc->coll = av_content_light_metadata_alloc(>coll_size);
+if (!sc->coll)
+return AVERROR(ENOMEM);
+
+sc->coll->MaxCLL  = avio_rb16(pb);
+sc->coll->MaxFALL = avio_rb16(pb);
+
+return 0;
+}
+
 static int mov_read_st3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
 AVStream *st;
@@ -5912,6 +5981,8 @@ static const MOVParseTableEntry mov_default_parse_table[] 
= {
 { MKTAG('S','m','D','m'), mov_read_smdm },
 { MKTAG('C','o','L','L'), mov_read_coll },
 { MKTAG('v','p','c','C'), mov_read_vpcc },
+{ MKTAG('m','d','c','v'), mov_read_mdcv },
+{ MKTAG('c','l','l','i'), mov_read_clli },
 { 0, NULL }
 };
 
-- 
2.14.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 0/1][TOOL][HACK] Allocation NULL check fuzzer

2017-11-24 Thread Vittorio Giovara
On 11/24/2017 11:35 PM, Derek *Buitenhuis*  wrote:
> It would probably make an absolute ton of reports, since there are quite
> a few unchecked allocs in FFmpeg... might be kinda spammy.

That might not be the case any more, most of the checks have been added in
the Coverity effort of 2015 and continued later on, so the report might
actually have interesting findings.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] vf_zscale: Add more supported input properties

2017-11-15 Thread Vittorio Giovara
---
Now without mxf code >_>

There is a stable release with this code, and it's a minor update,
not sure if it warrants a configure check, but I'll add it if requested.
Vittorio

 libavfilter/vf_zscale.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 09fd842fe5..7d048da1ef 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -353,10 +353,10 @@ static int convert_matrix(enum AVColorSpace colorspace)
 return ZIMG_MATRIX_709;
 case AVCOL_SPC_UNSPECIFIED:
 return ZIMG_MATRIX_UNSPECIFIED;
-case AVCOL_SPC_BT470BG:
-return ZIMG_MATRIX_470BG;
 case AVCOL_SPC_SMPTE170M:
 return ZIMG_MATRIX_170M;
+case AVCOL_SPC_SMPTE240M:
+return ZIMG_MATRIX_240M;
 case AVCOL_SPC_YCGCO:
 return ZIMG_MATRIX_YCGCO;
 case AVCOL_SPC_BT2020_NCL:
@@ -374,10 +374,22 @@ static int convert_trc(enum AVColorTransferCharacteristic 
color_trc)
 return ZIMG_TRANSFER_UNSPECIFIED;
 case AVCOL_TRC_BT709:
 return ZIMG_TRANSFER_709;
+case AVCOL_TRC_GAMMA22:
+return ZIMG_TRANSFER_470_M;
+case AVCOL_TRC_GAMMA28:
+return ZIMG_TRANSFER_470_BG;
 case AVCOL_TRC_SMPTE170M:
 return ZIMG_TRANSFER_601;
+case AVCOL_TRC_SMPTE240M:
+return ZIMG_TRANSFER_240M;
 case AVCOL_TRC_LINEAR:
 return ZIMG_TRANSFER_LINEAR;
+case AVCOL_TRC_LOG:
+return ZIMG_TRANSFER_LOG_100;
+case AVCOL_TRC_LOG_SQRT:
+return ZIMG_TRANSFER_LOG_316;
+case AVCOL_TRC_IEC61966_2_4:
+return ZIMG_TRANSFER_IEC_61966_2_4;
 case AVCOL_TRC_BT2020_10:
 return ZIMG_TRANSFER_2020_10;
 case AVCOL_TRC_BT2020_12:
@@ -399,6 +411,10 @@ static int convert_primaries(enum AVColorPrimaries 
color_primaries)
 return ZIMG_PRIMARIES_UNSPECIFIED;
 case AVCOL_PRI_BT709:
 return ZIMG_PRIMARIES_709;
+case AVCOL_PRI_BT470M:
+return ZIMG_PRIMARIES_470_M;
+case AVCOL_PRI_BT470BG:
+return ZIMG_PRIMARIES_470_BG;
 case AVCOL_PRI_SMPTE170M:
 return ZIMG_PRIMARIES_170M;
 case AVCOL_PRI_SMPTE240M:
@@ -745,6 +761,8 @@ static const AVOption zscale_options[] = {
 { "2020", 0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_2020},0, 0, FLAGS, "primaries" },
 { "unknown",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_UNSPECIFIED}, 0, 0, FLAGS, "primaries" },
 { "bt709",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_709}, 0, 0, FLAGS, "primaries" },
+{ "bt470m",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_470_M},   0, 0, FLAGS, "primaries" },
+{ "bt470bg",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_470_BG},  0, 0, FLAGS, "primaries" },
 { "smpte170m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_170M},0, 0, FLAGS, "primaries" },
 { "smpte240m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_240M},0, 0, FLAGS, "primaries" },
 { "bt2020",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_2020},0, 0, FLAGS, "primaries" },
@@ -756,9 +774,13 @@ static const AVOption zscale_options[] = {
 { "unspecified",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_UNSPECIFIED}, 0, 0, FLAGS, "transfer" },
 { "601",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_601}, 0, 0, FLAGS, "transfer" },
 { "linear",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_LINEAR},  0, 0, FLAGS, "transfer" },
+{ "log100",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_LOG_100},  0, 0, FLAGS, "transfer" },
+{ "log316",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_LOG_316},  0, 0, FLAGS, "transfer" },
 { "2020_10",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_2020_10}, 0, 0, FLAGS, "transfer" },
 { "2020_12",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_2020_12}, 0, 0, FLAGS, "transfer" },
 { "unknown",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_UNSPECIFIED}, 0, 0, FLAGS, "transfer" },
+{ "bt470m",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_470_M}, 0, 0, FLAGS, "transfer" },
+{ "bt470bg",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_470_BG}, 0, 0, FLAGS, "transfer" },
 { "smpte170m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = 

[FFmpeg-devel] [PATCH] vf_zscale: Add more supported input properties

2017-11-14 Thread Vittorio Giovara
---
 libavfilter/vf_zscale.c | 26 --
 libavformat/mxfdec.c| 20 
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 09fd842fe5..7d048da1ef 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -353,10 +353,10 @@ static int convert_matrix(enum AVColorSpace colorspace)
 return ZIMG_MATRIX_709;
 case AVCOL_SPC_UNSPECIFIED:
 return ZIMG_MATRIX_UNSPECIFIED;
-case AVCOL_SPC_BT470BG:
-return ZIMG_MATRIX_470BG;
 case AVCOL_SPC_SMPTE170M:
 return ZIMG_MATRIX_170M;
+case AVCOL_SPC_SMPTE240M:
+return ZIMG_MATRIX_240M;
 case AVCOL_SPC_YCGCO:
 return ZIMG_MATRIX_YCGCO;
 case AVCOL_SPC_BT2020_NCL:
@@ -374,10 +374,22 @@ static int convert_trc(enum AVColorTransferCharacteristic 
color_trc)
 return ZIMG_TRANSFER_UNSPECIFIED;
 case AVCOL_TRC_BT709:
 return ZIMG_TRANSFER_709;
+case AVCOL_TRC_GAMMA22:
+return ZIMG_TRANSFER_470_M;
+case AVCOL_TRC_GAMMA28:
+return ZIMG_TRANSFER_470_BG;
 case AVCOL_TRC_SMPTE170M:
 return ZIMG_TRANSFER_601;
+case AVCOL_TRC_SMPTE240M:
+return ZIMG_TRANSFER_240M;
 case AVCOL_TRC_LINEAR:
 return ZIMG_TRANSFER_LINEAR;
+case AVCOL_TRC_LOG:
+return ZIMG_TRANSFER_LOG_100;
+case AVCOL_TRC_LOG_SQRT:
+return ZIMG_TRANSFER_LOG_316;
+case AVCOL_TRC_IEC61966_2_4:
+return ZIMG_TRANSFER_IEC_61966_2_4;
 case AVCOL_TRC_BT2020_10:
 return ZIMG_TRANSFER_2020_10;
 case AVCOL_TRC_BT2020_12:
@@ -399,6 +411,10 @@ static int convert_primaries(enum AVColorPrimaries 
color_primaries)
 return ZIMG_PRIMARIES_UNSPECIFIED;
 case AVCOL_PRI_BT709:
 return ZIMG_PRIMARIES_709;
+case AVCOL_PRI_BT470M:
+return ZIMG_PRIMARIES_470_M;
+case AVCOL_PRI_BT470BG:
+return ZIMG_PRIMARIES_470_BG;
 case AVCOL_PRI_SMPTE170M:
 return ZIMG_PRIMARIES_170M;
 case AVCOL_PRI_SMPTE240M:
@@ -745,6 +761,8 @@ static const AVOption zscale_options[] = {
 { "2020", 0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_2020},0, 0, FLAGS, "primaries" },
 { "unknown",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_UNSPECIFIED}, 0, 0, FLAGS, "primaries" },
 { "bt709",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_709}, 0, 0, FLAGS, "primaries" },
+{ "bt470m",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_470_M},   0, 0, FLAGS, "primaries" },
+{ "bt470bg",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_470_BG},  0, 0, FLAGS, "primaries" },
 { "smpte170m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_170M},0, 0, FLAGS, "primaries" },
 { "smpte240m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_240M},0, 0, FLAGS, "primaries" },
 { "bt2020",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_PRIMARIES_2020},0, 0, FLAGS, "primaries" },
@@ -756,9 +774,13 @@ static const AVOption zscale_options[] = {
 { "unspecified",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_UNSPECIFIED}, 0, 0, FLAGS, "transfer" },
 { "601",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_601}, 0, 0, FLAGS, "transfer" },
 { "linear",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_LINEAR},  0, 0, FLAGS, "transfer" },
+{ "log100",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_LOG_100},  0, 0, FLAGS, "transfer" },
+{ "log316",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_LOG_316},  0, 0, FLAGS, "transfer" },
 { "2020_10",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_2020_10}, 0, 0, FLAGS, "transfer" },
 { "2020_12",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_2020_12}, 0, 0, FLAGS, "transfer" },
 { "unknown",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_UNSPECIFIED}, 0, 0, FLAGS, "transfer" },
+{ "bt470m",   0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_470_M}, 0, 0, FLAGS, "transfer" },
+{ "bt470bg",  0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_470_BG}, 0, 0, FLAGS, "transfer" },
 { "smpte170m",0,   0, AV_OPT_TYPE_CONST, 
{.i64 = ZIMG_TRANSFER_601}, 0, 0, FLAGS, "transfer" },
 { "bt709",0,   0, AV_OPT_TYPE_CONST, 

[FFmpeg-devel] [PATCH] prores: Always assume limited range

2017-09-27 Thread Vittorio Giovara
As defined by the spcifications
---
 libavcodec/proresdec_lgpl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/proresdec_lgpl.c b/libavcodec/proresdec_lgpl.c
index bc5bdb5a4d..c86d433f50 100644
--- a/libavcodec/proresdec_lgpl.c
+++ b/libavcodec/proresdec_lgpl.c
@@ -177,6 +177,7 @@ static int decode_frame_header(ProresContext *ctx, const 
uint8_t *buf,
 avctx->color_primaries = buf[14];
 avctx->color_trc   = buf[15];
 avctx->colorspace  = buf[16];
+avctx->color_range = AVCOL_RANGE_MPEG;
 
 ctx->qmat_changed = 0;
 ptr   = buf + 20;
-- 
2.14.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] lavu: add a gamma field to AVMasteringDisplayMetadata

2017-09-20 Thread Vittorio Giovara
>* In my opinion we should not add new fields to structures that map 1:1 to
*>* something defined elsewhere (with the exception of booleans).
*>* I think this should be a separate side data and type, ie AVGammaResponse,
*>* that can of course reside in the same header.
*>* --
*>* Vittorio
*>* ___
*>* ffmpeg-devel mailing list
*>* ffmpeg-devel at ffmpeg.org 
*>* http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

*>
Why? I don't see anything special about the struct. And this value fits in
exactly to what the struct is all about.


"mastering display" is a specification defined in several standards,
including h264 and h265. It groups two kind of properties, the mastering
display primaries (with its whitepoint) and the screen min and max
luminance. AVMasteringDisplayMetadata is modelled after this specification
and designed to expose the same kind of information; setters and getter
will expect to use all the information contained in it.

Adding a field that is missing from the original specification to that
structure is, in my opinion, semantically wrong, not only because
AVMasteringDisplayMetadata won't be strictly mapped to a the "mastering
display" any more, but also because the setters and getters of the
"mastering display" users will not need the new field at all.

There is a precedent to this: AVColorVolume. Its field are, in practice,
tied to mastering display, but they are stored in a separate structure, and
not squashed in the main AVMasteringDisplayMetadata structure. I think we
should continue this trend so that we can easily map structures with known
specifications and not overload them with unnecessary fields.
-- 
Vittorio
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


  1   2   3   4   >