Re: [FFmpeg-devel] [PATCH] lavc/x265: set preferred_transfer_characteristics for HLG

2019-12-13 Thread Zhong Li
James Almer  于2019年12月13日周五 上午12:14写道:
>
> On 12/4/2019 11:24 AM, Zhong Li wrote:
> > "HEVC HDR UHDTV Bitstreams using HLG10 shall also contain the
> > alternative_transfer_characteristics SEI message. The
> > alternative_transfer_characteristics SEI message shall be inserted on
> > the HEVC DVB_RAP, and preferred_transfer_characteristics shall be set
> > equal to "18", indicating Recommendation ITU-R BT. 2100 [45] HLG
> > system."
> >
> > Signed-off-by: Zhong Li 
> > ---
> >  libavcodec/libx265.c | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
> > index 4e75077..963c28f 100644
> > --- a/libavcodec/libx265.c
> > +++ b/libavcodec/libx265.c
> > @@ -159,6 +159,10 @@ static av_cold int libx265_encode_init(AVCodecContext 
> > *avctx)
> >  // x265 validates the parameters internally
> >  ctx->params->vui.colorPrimaries  = avctx->color_primaries;
> >  ctx->params->vui.transferCharacteristics = avctx->color_trc;
> > +#if X265_BUILD >= 159
> > +if (avctx->color_trc == AVCOL_TRC_ARIB_STD_B67)
> > +ctx->params->preferredTransferCharacteristics = 
> > ctx->params->vui.transferCharacteristics;
> > +#endif
>
> isn't the point of preferred_transfer_characteristics to have the Arib
> value at the same time the standard VUI trc field contains a value like
> BT2020 instead, for device backwards compat purposes?
>
> If both have the Arib value, then it seems redundant.

It is also for robustness,if preferred_transfer_characteristics is
existed but not set, decoder may use
preferred_transfer_characteristics instead of vui
transfer_characteristics.
___
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] [PATCHv2] fate: Add an option for disabling the 2k/4k tests

2019-12-13 Thread Martin Storsjö

On Thu, 12 Dec 2019, James Almer wrote:


On 12/12/2019 7:03 PM, Martin Storsjö wrote:

On Wed, 11 Dec 2019, Martin Storsjö wrote:


On Wed, 11 Dec 2019, Carl Eugen Hoyos wrote:

Am Mi., 11. Dez. 2019 um 09:39 Uhr schrieb Martin Storsjö 

:


When testing on a memory limited system, these tests consume a
significant amount of memory and can often fail if testing by running
multiple processes in parallel.
---
Adjusted to use ALLYES instead of a -yes-yes construct.

Also moved the 2k tests to the same option.
---
 configure | 3 +++
 tests/fate/seek.mak   | 3 ++-
 tests/fate/vcodec.mak | 5 +++--
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index ca7137f341..922cd8d0ee 100755
--- a/configure
+++ b/configure
@@ -482,6 +482,7 @@ Developer options (useful when working on FFmpeg 

itself):

   --ignore-tests=TESTS comma-separated list (without "fate-"
prefix
    in the name) of tests whose result is
ignored
   --enable-linux-perf  enable Linux Performance Monitor API
+  --disable-large-tests    disable tests that use a large amount of 

memory


I would have suggested to control this when running the tests, if the 

configure

setting makes sense, it should at least be possible to change the
setting 

when

calling make.
Or is that possible anyway?


It's possible to do e.g. "make fate CONFIG_LARGE_TESTS=no"; any
var=value assignment on the make command line overrides any
var=othervalue assignment within the makefiles themselves, but that
doesn't seem very convenient.

But I'd like to have it as a configure option, to easily be able to
set it e.g. in a fate setup.


Any further opinions on this one - is it ok to go ahead with it in this
form, or are changes requested?


Configure option is fine if it can also be overridden from the command
line at runtime (like --samples and SAMPLES).


Well, it can be overridden at runtime, but it's not with a very convenient 
name (the CONFIG_* variable). Is that ok?


// Martin
___
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] avcodec/cbs_av1: avoid reset some members of loop filter

2019-12-13 Thread Fei Wang
According to spec 6.8.10, loop_filter_ref_deltas & loop_filter_mode_deltas
should be keep same as its previous value if it is not presented in current
syntax.

Signed-off-by: Fei Wang 
---
 libavcodec/cbs_av1.h |  3 +++
 libavcodec/cbs_av1_syntax_template.c | 24 +++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index 643e76793f..7267baaceb 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -444,6 +444,9 @@ typedef struct CodedBitstreamAV1Context {
 AV1ReferenceFrameState *ref;
 AV1ReferenceFrameState read_ref[AV1_NUM_REF_FRAMES];
 AV1ReferenceFrameState write_ref[AV1_NUM_REF_FRAMES];
+
+int8_t pre_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
+int8_t pre_loop_filter_mode_deltas[2];
 } CodedBitstreamAV1Context;
 
 
diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index f53955c52e..c9ac1dc600 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -819,6 +819,13 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext 
*ctx, RWContext *rw,
 CodedBitstreamAV1Context *priv = ctx->priv_data;
 int i, err;
 
+memcpy(current->loop_filter_ref_deltas,
+priv->pre_loop_filter_ref_deltas,
+AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
+memcpy(current->loop_filter_mode_deltas,
+priv->pre_loop_filter_mode_deltas,
+2 * sizeof(int8_t));
+
 if (priv->coded_lossless || current->allow_intrabc) {
 infer(loop_filter_level[0], 0);
 infer(loop_filter_level[1], 0);
@@ -832,7 +839,15 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext 
*ctx, RWContext *rw,
 infer(loop_filter_ref_deltas[AV1_REF_FRAME_ALTREF2], -1);
 for (i = 0; i < 2; i++)
 infer(loop_filter_mode_deltas[i], 0);
-return 0;
+
+memcpy(priv->pre_loop_filter_ref_deltas,
+current->loop_filter_ref_deltas,
+AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
+memcpy(priv->pre_loop_filter_mode_deltas,
+current->loop_filter_mode_deltas,
+2 * sizeof(int8_t));
+
+   return 0;
 }
 
 fb(6, loop_filter_level[0]);
@@ -865,6 +880,13 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext 
*ctx, RWContext *rw,
 }
 }
 
+memcpy(priv->pre_loop_filter_ref_deltas,
+current->loop_filter_ref_deltas,
+AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
+memcpy(priv->pre_loop_filter_mode_deltas,
+current->loop_filter_mode_deltas,
+2 * sizeof(int8_t));
+
 return 0;
 }
 
-- 
2.17.1

___
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 3/4] avfilter/vf_dnn_processing: add format GRAY8 and GRAYF32 support

2019-12-13 Thread Guo, Yejun
> From: Pedro Arthur [mailto:bygran...@gmail.com]
> Sent: Friday, December 13, 2019 12:45 AM
> To: FFmpeg development discussions and patches 
> Cc: Guo, Yejun 
> Subject: Re: [FFmpeg-devel] [PATCH 3/4] avfilter/vf_dnn_processing: add
> format GRAY8 and GRAYF32 support
> Hi,
> 
> how should I test this patch?

the fourth patch of this patch set is the fate test for this feature, so I 
ignored comments here.
I'll add the test descriptions back in v2.

> 
> Em sex., 22 de nov. de 2019 às 04:57, Guo, Yejun 
> escreveu:
> 
> > Signed-off-by: Guo, Yejun 
> > ---
> >  doc/filters.texi|   8 ++-
> >  libavfilter/vf_dnn_processing.c | 147
> > ++--
> >  2 files changed, 118 insertions(+), 37 deletions(-)
> >
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index 1f86ae1..c3f7997 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -8992,7 +8992,13 @@ Set the input name of the dnn network.
> >  Set the output name of the dnn network.
> >
> >  @item fmt
> > -Set the pixel format for the Frame. Allowed values are
> > @code{AV_PIX_FMT_RGB24}, and @code{AV_PIX_FMT_BGR24}.
> > +Set the pixel format for the Frame, the value is determined by the input
> > of the dnn network model.
> >
> This sentence is a bit confusing, also I think this property should be
> removed. (I will explain bellow).

sure, no problem.

> 
> +
> > +If the model handles RGB (or BGR) image and the data type of model input
> > is uint8, fmt must be @code{AV_PIX_FMT_RGB24} (or
> @code{AV_PIX_FMT_BGR24}.
> > +If the model handles RGB (or BGR) image and the data type of model input
> > is float, fmt must be @code{AV_PIX_FMT_RGB24} (or
> @code{AV_PIX_FMT_BGR24},
> > and this filter will do data type conversion internally.
> > +If the model handles GRAY image and the data type of model input is
> > uint8, fmt must be @code{AV_PIX_FMT_GRAY8}.
> > +If the model handles GRAY image and the data type of model input is
> > float, fmt must be @code{AV_PIX_FMT_GRAYF32}.
> > +
> >  Default value is @code{AV_PIX_FMT_RGB24}.
> >
> >  @end table
> > diff --git a/libavfilter/vf_dnn_processing.c
> > b/libavfilter/vf_dnn_processing.c
> > index ce976ec..963dd5e 100644
> > --- a/libavfilter/vf_dnn_processing.c
> > +++ b/libavfilter/vf_dnn_processing.c
> > @@ -70,10 +70,12 @@ static av_cold int init(AVFilterContext *context)
> >  {
> >  DnnProcessingContext *ctx = context->priv;
> >  int supported = 0;
> > -// as the first step, only rgb24 and bgr24 are supported
> > +// to support more formats
> >  const enum AVPixelFormat supported_pixel_fmts[] = {
> >  AV_PIX_FMT_RGB24,
> >  AV_PIX_FMT_BGR24,
> > +AV_PIX_FMT_GRAY8,
> > +AV_PIX_FMT_GRAYF32,
> >  };
> >  for (int i = 0; i < sizeof(supported_pixel_fmts) / sizeof(enum
> > AVPixelFormat); ++i) {
> >  if (supported_pixel_fmts[i] == ctx->fmt) {
> > @@ -156,14 +158,38 @@ static int config_input(AVFilterLink *inlink)
> >  return AVERROR(EIO);
> >  }
> >
> I think the filter should not check formats manually in the init function
> (unless I'm missing something), it would be best if you query for all the
> above supported formats in query_formats and later in config_input you make
> sure the expected model format matches the frame format.

I'm afraid it is too late if we find the format mismatch in function 
config_input.

For example, the dnn module is designed to accept BGR24 data, and the actual
format comes into config_input is RGB24 or YUV420P (we'll add yuv formats later 
in
supported pixel fmts) or something else such as GRAY8. We have two choices:

- return error, and the application ends.
  This is not what we want.
- return no_error, and do format conversion at the beginning of function 
filter_frame.
  It makes this filter complex, and our implementation for the conversion might 
not be the best optimized.
  My idea is to keep this filter simple. And the users can choose what they 
want to do conversion in the filters graph.

Regarding the below description in doc/filters.texi, the reason is that we do 
not have format such as rgbf32 and bgrf32, we have to do conversion within this 
filter.
"If the model handles RGB (or BGR) image and the data type of model input is 
float, fmt must be @code{AV_PIX_FMT_RGB24} (or @code{AV_PIX
_FMT_BGR24}, and this filter will do data type conversion internally."


Consider the filter vf_dnn_analytic in my plan, I think the conversion is 
necessary since mostly a scale down is needed.

> 
> 
> > -if (model_input.channels != 3) {
> > -av_log(ctx, AV_LOG_ERROR, "the model requires input channels
> > %d\n",
> > -   model_input.channels);
> > -return AVERROR(EIO);
> > -}
> > -if (model_input.dt != DNN_FLOAT && model_input.dt != DNN_UINT8) {
> > -av_log(ctx, AV_LOG_ERROR, "only support dnn models with input
> > data type as float32 and uint8.\n");
> > -return AVERROR(

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/hevc_mp4toannexb_bsf: Check that there is enough input left for nalu size

2019-12-13 Thread Andreas Rheinhardt
On Fri, Dec 13, 2019 at 3:07 AM Michael Niedermayer 
wrote:

> No testcase
>
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/hevc_mp4toannexb_bsf.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> b/libavcodec/hevc_mp4toannexb_bsf.c
> index baa93628ed..e0d20a550c 100644
> --- a/libavcodec/hevc_mp4toannexb_bsf.c
> +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> @@ -152,7 +152,9 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx,
> AVPacket *out)
>  extra_size= add_extradata * ctx->par_out->extradata_size;
>  got_irap |= is_irap;
>
> -if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) {
> +if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size ||
>

Up until now I thought that FFmpeg has some implicit assumptions: int
having 32bit being one of them (the log2 functions depend on this). And I
also thought that size_t being able to hold all the values of an unsigned
was one of these implicit assumptions, too. Am I wrong on this?

A testcase for the last condition is easy to produce by simply manipulating
the size field of a NAL unit.

- Andreas
___
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/4] avfilter/vf_interlace: restore lowpass mode constants

2019-12-13 Thread Marton Balint



On Fri, 6 Dec 2019, Marton Balint wrote:


The documentation still mentions numerical constants in addition to textual
ones. It is also wrong to use distinct modes as flags and it disallows us to
actually use the flags field for real flags in the future.


Ping for the series. Will apply soon.

Thanks,
Marton



Signed-off-by: Marton Balint 
---
libavfilter/tinterlace.h|  7 +++
libavfilter/vf_tinterlace.c | 13 +
2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/libavfilter/tinterlace.h b/libavfilter/tinterlace.h
index 5bcb9a583a..e204b61aa0 100644
--- a/libavfilter/tinterlace.h
+++ b/libavfilter/tinterlace.h
@@ -37,6 +37,12 @@
#define TINTERLACE_FLAG_CVLPF 2
#define TINTERLACE_FLAG_EXACT_TB 4

+enum VLPFilter {
+VLPF_OFF = 0,
+VLPF_LIN = 1,
+VLPF_CMP = 2,
+};
+
enum TInterlaceMode {
MODE_MERGE = 0,
MODE_DROP_EVEN,
@@ -59,6 +65,7 @@ typedef struct TInterlaceContext {
int mode;   ///< TInterlaceMode, interlace mode selected
AVRational preout_time_base;
int flags;  ///< flags affecting interlacing algorithm
+int lowpass;///< legacy interlace filter lowpass mode
int frame;  ///< number of the output frame
int vsub;   ///< chroma vertical subsampling
AVFrame *cur;
diff --git a/libavfilter/vf_tinterlace.c b/libavfilter/vf_tinterlace.c
index fc5d11e053..32b2ff9f5a 100644
--- a/libavfilter/vf_tinterlace.c
+++ b/libavfilter/vf_tinterlace.c
@@ -63,10 +63,10 @@ static const AVOption interlace_options[] = {
   { "scan",  "scanning mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64 = 
MODE_TFF}, 0, 1, FLAGS, "mode"},
   { "tff",   "top field first",  0, 
AV_OPT_TYPE_CONST, {.i64 = MODE_TFF}, INT_MIN, INT_MAX, FLAGS, .unit = "mode"},
   { "bff",   "bottom field first",   0, 
AV_OPT_TYPE_CONST, {.i64 = MODE_BFF}, INT_MIN, INT_MAX, FLAGS, .unit = "mode"},
-   { "lowpass",   "set vertical low-pass filter", OFFSET(flags), 
AV_OPT_TYPE_FLAGS,   {.i64 = TINTERLACE_FLAG_VLPF}, 0, 2, FLAGS, "flags" },
-   { "off",   "disable vertical low-pass filter", 0, 
AV_OPT_TYPE_CONST, {.i64 = 0}, INT_MIN, INT_MAX, FLAGS, "flags" },
-   { "linear","linear vertical low-pass filter",  0, 
AV_OPT_TYPE_CONST, {.i64 = TINTERLACE_FLAG_VLPF}, INT_MIN, INT_MAX, FLAGS, "flags" },
-   { "complex",   "complex vertical low-pass filter", 0, 
AV_OPT_TYPE_CONST, {.i64 = TINTERLACE_FLAG_CVLPF},INT_MIN, INT_MAX, FLAGS, "flags" },
+   { "lowpass",   "set vertical low-pass filter", OFFSET(lowpass), 
AV_OPT_TYPE_INT,   {.i64 = VLPF_LIN}, 0, 2, FLAGS, "lowpass" },
+   { "off",   "disable vertical low-pass filter", 0, 
AV_OPT_TYPE_CONST, {.i64 = VLPF_OFF}, INT_MIN, INT_MAX, FLAGS, "lowpass" },
+   { "linear","linear vertical low-pass filter",  0, 
AV_OPT_TYPE_CONST, {.i64 = VLPF_LIN}, INT_MIN, INT_MAX, FLAGS, "lowpass" },
+   { "complex",   "complex vertical low-pass filter", 0, 
AV_OPT_TYPE_CONST, {.i64 = VLPF_CMP}, INT_MIN, INT_MAX, FLAGS, "lowpass" },

   { NULL }
};
@@ -518,6 +518,11 @@ static int init_interlace(AVFilterContext *ctx)
if (tinterlace->mode <= MODE_BFF)
tinterlace->mode += MODE_INTERLEAVE_TOP;

+if (tinterlace->lowpass == VLPF_LIN)
+tinterlace->flags |= TINTERLACE_FLAG_VLPF;
+if (tinterlace->lowpass == VLPF_CMP)
+tinterlace->flags |= TINTERLACE_FLAG_CVLPF;
+
return 0;
}

--
2.16.4

___
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 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] FATE: add test for hevc files with 4 TUDepth(0..4) of cbf_cb/cbf_cr

2019-12-13 Thread James Almer
On 12/13/2019 4:05 AM, Fu, Linjie wrote:
>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of
>> Carl Eugen Hoyos
>> Sent: Friday, December 13, 2019 10:47
>> To: FFmpeg development discussions and patches > de...@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH, v2] FATE: add test for hevc files with 4
>> TUDepth(0..4) of cbf_cb/cbf_cr
>>
>> Am Fr., 13. Dez. 2019 um 03:43 Uhr schrieb Linjie Fu :
>>
>>> 5 cabac states for cbf_cb and cbf_cr are supported according to
>>> Table 9-4.
>>>
>>> Add a test for 64x64 4:4:4 8bit HEVC clips with TUDepth = 4, cbf_cr > 0.
>>> +fate-hevc-cabac-tudepth: CMD = framecrc -flags unaligned -i
>> $(TARGET_SAMPLES)/hevc/hevc-cabac-tudepth.hevc -pix_fmt yuv444p
>>
>> File name looks wrong to me...
>>
> I renamed the clips to *.hevc and try to be consistent with other files in 
> fate/hevc/.
> 
> (If it's not strictly required, cbf_cr_cb_TUDepth_4_circle.h265 may be more 
> straight
> forward and it has already been in fate-suite)

That's the name the sample has in the fate-suite already, so no reason
to change it. Especially if it's the better name.

> 
> -fate-hevc-cabac-tudepth: CMD = framecrc -flags unaligned -i 
> $(TARGET_SAMPLES)/hevc/hevc-cabac-tudepth.hevc -pix_fmt yuv444p
> +fate-hevc-cabac-tudepth: CMD = framecrc -flags unaligned -i 
> $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
> 
> Do I need to update this in a new version(v3)?

Yes, please.

> 
> - linjie
> ___
> 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 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 1/2] avfilter/vf_histogram: clean up code

2019-12-13 Thread quinkblack
From: Zhao Zhili 

---
 libavfilter/vf_histogram.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavfilter/vf_histogram.c b/libavfilter/vf_histogram.c
index 5185992de6..0d2d087beb 100644
--- a/libavfilter/vf_histogram.c
+++ b/libavfilter/vf_histogram.c
@@ -266,20 +266,20 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 const int is_chroma = (k == 1 || k == 2);
 const int dst_h = AV_CEIL_RSHIFT(outlink->h, (is_chroma ? 
h->odesc->log2_chroma_h : 0));
 const int dst_w = AV_CEIL_RSHIFT(outlink->w, (is_chroma ? 
h->odesc->log2_chroma_w : 0));
+const int plane = h->odesc->comp[k].plane;
+uint8_t *const data = out->data[plane];
+const int linesize = out->linesize[plane];
 
 if (h->histogram_size <= 256) {
 for (i = 0; i < dst_h ; i++)
-memset(out->data[h->odesc->comp[k].plane] +
-   i * out->linesize[h->odesc->comp[k].plane],
-   h->bg_color[k], dst_w);
+memset(data + i * linesize, h->bg_color[k], dst_w);
 } else {
 const int mult = h->mult;
 
-for (i = 0; i < dst_h ; i++)
-for (j = 0; j < dst_w; j++)
-AV_WN16(out->data[h->odesc->comp[k].plane] +
-i * out->linesize[h->odesc->comp[k].plane] + j * 2,
-h->bg_color[k] * mult);
+for (j = 0; j < dst_w; j++)
+AV_WN16(data + j * 2, h->bg_color[k] * mult);
+for (i = 1; i < dst_h; i++)
+memcpy(data + i * linesize, data, dst_w * 2);
 }
 }
 
-- 
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".

[FFmpeg-devel] [PATCH 2/2] avfilter/vf_histogram: fix color value

2019-12-13 Thread quinkblack
From: Zhao Zhili 

---
 libavfilter/vf_histogram.c |   4 +-
 tests/ref/fate/filter-histogram-levels | 100 -
 2 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/libavfilter/vf_histogram.c b/libavfilter/vf_histogram.c
index 0d2d087beb..d6928d6b08 100644
--- a/libavfilter/vf_histogram.c
+++ b/libavfilter/vf_histogram.c
@@ -185,9 +185,9 @@ static int query_formats(AVFilterContext *ctx)
 return 0;
 }
 
-static const uint8_t black_yuva_color[4] = { 0, 127, 127, 255 };
+static const uint8_t black_yuva_color[4] = { 0, 128, 128, 255 };
 static const uint8_t black_gbrp_color[4] = { 0, 0, 0, 255 };
-static const uint8_t white_yuva_color[4] = { 255, 127, 127, 255 };
+static const uint8_t white_yuva_color[4] = { 255, 128, 128, 255 };
 static const uint8_t white_gbrp_color[4] = { 255, 255, 255, 255 };
 
 static int config_input(AVFilterLink *inlink)
diff --git a/tests/ref/fate/filter-histogram-levels 
b/tests/ref/fate/filter-histogram-levels
index 697d7d19c0..e1329593e3 100644
--- a/tests/ref/fate/filter-histogram-levels
+++ b/tests/ref/fate/filter-histogram-levels
@@ -3,53 +3,53 @@
 #codec_id 0: rawvideo
 #dimensions 0: 256x636
 #sar 0: 1/1
-0,  0,  0,1,   488448, 0xc27a6cac
-0,  1,  1,1,   488448, 0xf00a152e
-0,  2,  2,1,   488448, 0x060b8c70
-0,  3,  3,1,   488448, 0xf75d6ee2
-0,  4,  4,1,   488448, 0xd7a7f06e
-0,  5,  5,1,   488448, 0x585281a5
-0,  6,  6,1,   488448, 0xb06e3ee8
-0,  7,  7,1,   488448, 0x201d0b8c
-0,  8,  8,1,   488448, 0x4e14e319
-0,  9,  9,1,   488448, 0x5aef5cca
-0, 10, 10,1,   488448, 0x57018668
-0, 11, 11,1,   488448, 0x2ad45b3f
-0, 12, 12,1,   488448, 0x62cc36b8
-0, 13, 13,1,   488448, 0x9e84585e
-0, 14, 14,1,   488448, 0xe6552e42
-0, 15, 15,1,   488448, 0x13b90c2c
-0, 16, 16,1,   488448, 0xf9557145
-0, 17, 17,1,   488448, 0x818340bc
-0, 18, 18,1,   488448, 0x5112c6e1
-0, 19, 19,1,   488448, 0x5d5b8f43
-0, 20, 20,1,   488448, 0xf2101ea6
-0, 21, 21,1,   488448, 0x4266af4d
-0, 22, 22,1,   488448, 0xb358806e
-0, 23, 23,1,   488448, 0xe336aa60
-0, 24, 24,1,   488448, 0x64fcc339
-0, 25, 25,1,   488448, 0x86e4b729
-0, 26, 26,1,   488448, 0x48c380d0
-0, 27, 27,1,   488448, 0xaee36fd3
-0, 28, 28,1,   488448, 0x20b84429
-0, 29, 29,1,   488448, 0x84d85542
-0, 30, 30,1,   488448, 0x94aea169
-0, 31, 31,1,   488448, 0x6278fa2c
-0, 32, 32,1,   488448, 0xaadf998d
-0, 33, 33,1,   488448, 0x29bba90d
-0, 34, 34,1,   488448, 0xef1117ad
-0, 35, 35,1,   488448, 0xd961e36d
-0, 36, 36,1,   488448, 0xff53296e
-0, 37, 37,1,   488448, 0x41f381f9
-0, 38, 38,1,   488448, 0x66fcfc2a
-0, 39, 39,1,   488448, 0x758bb472
-0, 40, 40,1,   488448, 0xefc6dc9e
-0, 41, 41,1,   488448, 0x77fccb69
-0, 42, 42,1,   488448, 0x7a1d82a4
-0, 43, 43,1,   488448, 0xc9d61a1b
-0, 44, 44,1,   488448, 0x8e689deb
-0, 45, 45,1,   488448, 0x52133e75
-0, 46, 46,1,   488448, 0xcc0a098e
-0, 47, 47,1,   488448, 0x045cd17f
-0, 48, 48,1,   488448, 0x97f89963
-0, 49, 49,1,   488448, 0xa1f835ff
+0,  0,  0,1,   488448, 0x0a904cf7
+0,  1,  1,1,   488448, 0x3820f56a
+0,  2,  2,1,   488448, 0x4e126cbb
+0,  3,  3,1,   488448, 0x3f734f2d
+0,  4,  4,1,   488448, 0x1fbdd0b9
+0,  5,  5,1,   488448, 0xa05961f0
+0,  6,  6,1,   488448, 0xf8751f33
+0,  7,  7,1,   488448, 0x6824ebc8
+0,  8,  8,1,   488448, 0x961bc364
+0,  9,  9,1,   488448, 0xa2f63d15
+0, 10, 10,1,   488448, 0x9f0866b3
+0, 11, 11,1,   488448, 0x72db3b8a
+0, 12, 12,1,   488448, 0xaad31703
+0, 13, 13,1,   488448, 0xe68b38a9
+0, 14,

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/vf_histogram: clean up code

2019-12-13 Thread Paul B Mahol
Please provide some explanation.

On 12/13/19, quinkbl...@foxmail.com  wrote:
> From: Zhao Zhili 
>
> ---
>  libavfilter/vf_histogram.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/libavfilter/vf_histogram.c b/libavfilter/vf_histogram.c
> index 5185992de6..0d2d087beb 100644
> --- a/libavfilter/vf_histogram.c
> +++ b/libavfilter/vf_histogram.c
> @@ -266,20 +266,20 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in)
>  const int is_chroma = (k == 1 || k == 2);
>  const int dst_h = AV_CEIL_RSHIFT(outlink->h, (is_chroma ?
> h->odesc->log2_chroma_h : 0));
>  const int dst_w = AV_CEIL_RSHIFT(outlink->w, (is_chroma ?
> h->odesc->log2_chroma_w : 0));
> +const int plane = h->odesc->comp[k].plane;
> +uint8_t *const data = out->data[plane];
> +const int linesize = out->linesize[plane];
>
>  if (h->histogram_size <= 256) {
>  for (i = 0; i < dst_h ; i++)
> -memset(out->data[h->odesc->comp[k].plane] +
> -   i * out->linesize[h->odesc->comp[k].plane],
> -   h->bg_color[k], dst_w);
> +memset(data + i * linesize, h->bg_color[k], dst_w);
>  } else {
>  const int mult = h->mult;
>
> -for (i = 0; i < dst_h ; i++)
> -for (j = 0; j < dst_w; j++)
> -AV_WN16(out->data[h->odesc->comp[k].plane] +
> -i * out->linesize[h->odesc->comp[k].plane] + j * 2,
> -h->bg_color[k] * mult);
> +for (j = 0; j < dst_w; j++)
> +AV_WN16(data + j * 2, h->bg_color[k] * mult);
> +for (i = 1; i < dst_h; i++)
> +memcpy(data + i * linesize, data, dst_w * 2);
>  }
>  }
>
> --
> 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".
___
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] avcodec/simple_idct_template: fix integer overflow

2019-12-13 Thread Paul B Mahol
Signed-off-by: Paul B Mahol 
---
 libavcodec/simple_idct_template.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/simple_idct_template.c 
b/libavcodec/simple_idct_template.c
index d8fcfd7c53..5ddd0b45a2 100644
--- a/libavcodec/simple_idct_template.c
+++ b/libavcodec/simple_idct_template.c
@@ -121,7 +121,7 @@ static inline void FUNC6(idctRowCondDC)(idctin *row, int 
extra_shift)
 // TODO: Add DC-only support for int32_t input
 #if IN_IDCT_DEPTH == 16
 #if HAVE_FAST_64BIT
-#define ROW0_MASK (0xLL << 48 * HAVE_BIGENDIAN)
+#define ROW0_MASK (0xULL << 48 * HAVE_BIGENDIAN)
 if (((AV_RN64A(row) & ~ROW0_MASK) | AV_RN64A(row+4)) == 0) {
 uint64_t temp;
 if (DC_SHIFT - extra_shift >= 0) {
-- 
2.17.1

___
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] fate/cbs: use the rawvideo muxer for AV1 tests

2019-12-13 Thread James Almer
On 12/11/2019 10:14 PM, James Almer wrote:
> The IVF muxer autoinserts the av1_metadata filter unconditionally, which is
> not desirable for these tests.
> 
> Signed-off-by: James Almer 
> ---
>  tests/fate/cbs.mak | 6 +++---
>  tests/ref/fate/cbs-av1-av1-1-b10-23-film_grain-50  | 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-02-allintra| 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-03-sizedown| 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-03-sizeup  | 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-04-cdfupdate   | 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-05-mv  | 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-06-mfmv| 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-22-svc-L1T2| 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-22-svc-L2T1| 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-22-svc-L2T2| 2 +-
>  tests/ref/fate/cbs-av1-av1-1-b8-23-film_grain-50   | 2 +-
>  tests/ref/fate/cbs-av1-decode_model| 2 +-
>  tests/ref/fate/cbs-av1-frames_refs_short_signaling | 2 +-
>  tests/ref/fate/cbs-av1-non_uniform_tiling  | 2 +-
>  tests/ref/fate/cbs-av1-seq_hdr_op_param_info   | 2 +-
>  tests/ref/fate/cbs-av1-switch_frame| 2 +-
>  17 files changed, 19 insertions(+), 19 deletions(-)

Applied.
___
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 3/4] avfilter/vf_dnn_processing: add format GRAY8 and GRAYF32 support

2019-12-13 Thread Pedro Arthur
Em sex., 13 de dez. de 2019 às 08:23, Guo, Yejun  escreveu:
>
> > From: Pedro Arthur [mailto:bygran...@gmail.com]
> > Sent: Friday, December 13, 2019 12:45 AM
> > To: FFmpeg development discussions and patches 
> > Cc: Guo, Yejun 
> > Subject: Re: [FFmpeg-devel] [PATCH 3/4] avfilter/vf_dnn_processing: add
> > format GRAY8 and GRAYF32 support
> > Hi,
> >
> > how should I test this patch?
>
> the fourth patch of this patch set is the fate test for this feature, so I 
> ignored comments here.
> I'll add the test descriptions back in v2.
>
> >
> > Em sex., 22 de nov. de 2019 às 04:57, Guo, Yejun 
> > escreveu:
> >
> > > Signed-off-by: Guo, Yejun 
> > > ---
> > >  doc/filters.texi|   8 ++-
> > >  libavfilter/vf_dnn_processing.c | 147
> > > ++--
> > >  2 files changed, 118 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/doc/filters.texi b/doc/filters.texi
> > > index 1f86ae1..c3f7997 100644
> > > --- a/doc/filters.texi
> > > +++ b/doc/filters.texi
> > > @@ -8992,7 +8992,13 @@ Set the input name of the dnn network.
> > >  Set the output name of the dnn network.
> > >
> > >  @item fmt
> > > -Set the pixel format for the Frame. Allowed values are
> > > @code{AV_PIX_FMT_RGB24}, and @code{AV_PIX_FMT_BGR24}.
> > > +Set the pixel format for the Frame, the value is determined by the input
> > > of the dnn network model.
> > >
> > This sentence is a bit confusing, also I think this property should be
> > removed. (I will explain bellow).
>
> sure, no problem.
>
> >
> > +
> > > +If the model handles RGB (or BGR) image and the data type of model input
> > > is uint8, fmt must be @code{AV_PIX_FMT_RGB24} (or
> > @code{AV_PIX_FMT_BGR24}.
> > > +If the model handles RGB (or BGR) image and the data type of model input
> > > is float, fmt must be @code{AV_PIX_FMT_RGB24} (or
> > @code{AV_PIX_FMT_BGR24},
> > > and this filter will do data type conversion internally.
> > > +If the model handles GRAY image and the data type of model input is
> > > uint8, fmt must be @code{AV_PIX_FMT_GRAY8}.
> > > +If the model handles GRAY image and the data type of model input is
> > > float, fmt must be @code{AV_PIX_FMT_GRAYF32}.
> > > +
> > >  Default value is @code{AV_PIX_FMT_RGB24}.
> > >
> > >  @end table
> > > diff --git a/libavfilter/vf_dnn_processing.c
> > > b/libavfilter/vf_dnn_processing.c
> > > index ce976ec..963dd5e 100644
> > > --- a/libavfilter/vf_dnn_processing.c
> > > +++ b/libavfilter/vf_dnn_processing.c
> > > @@ -70,10 +70,12 @@ static av_cold int init(AVFilterContext *context)
> > >  {
> > >  DnnProcessingContext *ctx = context->priv;
> > >  int supported = 0;
> > > -// as the first step, only rgb24 and bgr24 are supported
> > > +// to support more formats
> > >  const enum AVPixelFormat supported_pixel_fmts[] = {
> > >  AV_PIX_FMT_RGB24,
> > >  AV_PIX_FMT_BGR24,
> > > +AV_PIX_FMT_GRAY8,
> > > +AV_PIX_FMT_GRAYF32,
> > >  };
> > >  for (int i = 0; i < sizeof(supported_pixel_fmts) / sizeof(enum
> > > AVPixelFormat); ++i) {
> > >  if (supported_pixel_fmts[i] == ctx->fmt) {
> > > @@ -156,14 +158,38 @@ static int config_input(AVFilterLink *inlink)
> > >  return AVERROR(EIO);
> > >  }
> > >
> > I think the filter should not check formats manually in the init function
> > (unless I'm missing something), it would be best if you query for all the
> > above supported formats in query_formats and later in config_input you make
> > sure the expected model format matches the frame format.
>
> I'm afraid it is too late if we find the format mismatch in function 
> config_input.
>
> For example, the dnn module is designed to accept BGR24 data, and the actual
> format comes into config_input is RGB24 or YUV420P (we'll add yuv formats 
> later in
> supported pixel fmts) or something else such as GRAY8. We have two choices:
>
> - return error, and the application ends.
>   This is not what we want.
> - return no_error, and do format conversion at the beginning of function 
> filter_frame.
>   It makes this filter complex, and our implementation for the conversion 
> might not be the best optimized.
>   My idea is to keep this filter simple. And the users can choose what they 
> want to do conversion in the filters graph.
>
Indeed if the filter receives the format already converted is much
better, but if you already have to specify the format the model
expects as a parameter one could simply use the -pix_fmt to set the
format instead of having one more filter parameter.
Is there any downside if it uses "-pix_fmt" that "fmt" avoids?

> Regarding the below description in doc/filters.texi, the reason is that we do 
> not have format such as rgbf32 and bgrf32, we have to do conversion within 
> this filter.
> "If the model handles RGB (or BGR) image and the data type of model input is 
> float, fmt must be @code{AV_PIX_FMT_RGB24} (or @code{AV_PIX
> _FMT_BGR24}, and this filter will do data type conversion internall

Re: [FFmpeg-devel] [PATCH] avcodec/cbs_av1: avoid reset some members of loop filter

2019-12-13 Thread James Almer
On 12/13/2019 6:08 AM, Fei Wang wrote:
> According to spec 6.8.10, loop_filter_ref_deltas & loop_filter_mode_deltas
> should be keep same as its previous value if it is not presented in current
> syntax.
> 
> Signed-off-by: Fei Wang 
> ---
>  libavcodec/cbs_av1.h |  3 +++
>  libavcodec/cbs_av1_syntax_template.c | 24 +++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
> index 643e76793f..7267baaceb 100644
> --- a/libavcodec/cbs_av1.h
> +++ b/libavcodec/cbs_av1.h
> @@ -444,6 +444,9 @@ typedef struct CodedBitstreamAV1Context {
>  AV1ReferenceFrameState *ref;
>  AV1ReferenceFrameState read_ref[AV1_NUM_REF_FRAMES];
>  AV1ReferenceFrameState write_ref[AV1_NUM_REF_FRAMES];
> +
> +int8_t pre_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
> +int8_t pre_loop_filter_mode_deltas[2];
>  } CodedBitstreamAV1Context;
>  
>  
> diff --git a/libavcodec/cbs_av1_syntax_template.c 
> b/libavcodec/cbs_av1_syntax_template.c
> index f53955c52e..c9ac1dc600 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -819,6 +819,13 @@ static int 
> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>  CodedBitstreamAV1Context *priv = ctx->priv_data;
>  int i, err;
>  
> +memcpy(current->loop_filter_ref_deltas,
> +priv->pre_loop_filter_ref_deltas,
> +AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
> +memcpy(current->loop_filter_mode_deltas,
> +priv->pre_loop_filter_mode_deltas,
> +2 * sizeof(int8_t));
> +
>  if (priv->coded_lossless || current->allow_intrabc) {
>  infer(loop_filter_level[0], 0);
>  infer(loop_filter_level[1], 0);
> @@ -832,7 +839,15 @@ static int 
> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>  infer(loop_filter_ref_deltas[AV1_REF_FRAME_ALTREF2], -1);
>  for (i = 0; i < 2; i++)
>  infer(loop_filter_mode_deltas[i], 0);
> -return 0;
> +
> +memcpy(priv->pre_loop_filter_ref_deltas,
> +current->loop_filter_ref_deltas,
> +AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
> +memcpy(priv->pre_loop_filter_mode_deltas,
> +current->loop_filter_mode_deltas,
> +2 * sizeof(int8_t));
> +
> + return 0;
>  }
>  
>  fb(6, loop_filter_level[0]);
> @@ -865,6 +880,13 @@ static int 
> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>  }
>  }
>  
> +memcpy(priv->pre_loop_filter_ref_deltas,
> +current->loop_filter_ref_deltas,
> +AV1_TOTAL_REFS_PER_FRAME * sizeof(int8_t));
> +memcpy(priv->pre_loop_filter_mode_deltas,
> +current->loop_filter_mode_deltas,
> +2 * sizeof(int8_t));
> +
>  return 0;
>  }

You're trying to implement the load_previous() function defined in 6.8.2
but not following the requirement of using the values from
ref_frame_idx[primary_ref_frame]. You're instead loading the values from
whatever frame was last parsed.

Also, all this is for decoding. Within CBS, if it's not needed to get
correct parsing of some explicitly coded value in the bitstream, then
there's no reason to implement it.
CBS users can, if needed and for any given frame, take these values from
priv->ref[frame->ref_frame_idx[frame->primary_ref_frame]], effectively
implementing load_previous().
___
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 1/2] fftools: add global option to dump filter graph to stderr

2019-12-13 Thread Limin Wang

ping, please help to review it and push it. It's useful to check the auto insert
filters for debug.


On Tue, Nov 26, 2019 at 09:41:30AM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> It's useful for debugging filter graph purposes, now only lavfi can do that.
> 
> Reviewed-by: Carl Eugen Hoyos 
> Signed-off-by: Limin Wang 
> ---
>  doc/ffmpeg.texi | 4 
>  fftools/ffmpeg.h| 1 +
>  fftools/ffmpeg_filter.c | 7 +++
>  fftools/ffmpeg_opt.c| 3 +++
>  4 files changed, 15 insertions(+)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 92337d..f822a1025e 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -735,6 +735,10 @@ Technical note -- attachments are implemented as codec 
> extradata, so this
>  option can actually be used to extract extradata from any stream, not just
>  attachments.
>  
> +@item -dump_filtergraph (@emph{global})
> +Dump filter graph to stderr. It is off by default, the option is mostly 
> useful
> +for debugging filter graph purposes
> +
>  @item -noautorotate
>  Disable automatically rotating video based on file metadata.
>  
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 7b6f802082..5b8319da07 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -607,6 +607,7 @@ extern AVIOContext *progress_avio;
>  extern float max_error_rate;
>  extern char *videotoolbox_pixfmt;
>  
> +extern int dump_filtergraph;
>  extern int filter_nbthreads;
>  extern int filter_complex_nbthreads;
>  extern int vstats_version;
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 72838de1e2..edc15cff61 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -1109,6 +1109,13 @@ int configure_filtergraph(FilterGraph *fg)
>  if ((ret = avfilter_graph_config(fg->graph, NULL)) < 0)
>  goto fail;
>  
> +if (dump_filtergraph) {
> +char *dump = avfilter_graph_dump(fg->graph, NULL);
> +fputs(dump, stderr);
> +fflush(stderr);
> +av_free(dump);
> +}
> +
>  /* limit the lists of allowed formats to the ones selected, to
>   * make sure they stay the same if the filtergraph is reconfigured later 
> */
>  for (i = 0; i < fg->nb_outputs; i++) {
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 71063cc443..7616209a43 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -101,6 +101,7 @@ int copy_ts   = 0;
>  int start_at_zero = 0;
>  int copy_tb   = -1;
>  int debug_ts  = 0;
> +int dump_filtergraph  = 0;
>  int exit_on_error = 0;
>  int abort_on_flags= 0;
>  int print_stats   = -1;
> @@ -3535,6 +3536,8 @@ const OptionDef options[] = {
>  { "dump_attachment", HAS_ARG | OPT_STRING | OPT_SPEC |
>   OPT_EXPERT | OPT_INPUT, { .off 
> = OFFSET(dump_attachment) },
>  "extract an attachment into a file", "filename" },
> +{ "dump_filtergraph", OPT_BOOL,  { 
> &dump_filtergraph },
> +"dump filter graph to stderr" },
>  { "stream_loop", OPT_INT | HAS_ARG | OPT_EXPERT | OPT_INPUT |
>  OPT_OFFSET,  { .off 
> = OFFSET(loop) }, "set number of times input stream shall be looped", "loop 
> count" },
>  { "debug_ts",   OPT_BOOL | OPT_EXPERT,   { 
> &debug_ts },
> -- 
> 2.21.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 5/5] avcodec/hevc_mp4toannexb_bsf: Check that there is enough input left for nalu size

2019-12-13 Thread Michael Niedermayer
On Fri, Dec 13, 2019 at 12:24:04PM +0100, Andreas Rheinhardt wrote:
> On Fri, Dec 13, 2019 at 3:07 AM Michael Niedermayer 
> wrote:
> 
> > No testcase
> >
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/hevc_mp4toannexb_bsf.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> > b/libavcodec/hevc_mp4toannexb_bsf.c
> > index baa93628ed..e0d20a550c 100644
> > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > @@ -152,7 +152,9 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx,
> > AVPacket *out)
> >  extra_size= add_extradata * ctx->par_out->extradata_size;
> >  got_irap |= is_irap;
> >
> > -if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) {
> > +if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size ||
> >
> 
> Up until now I thought that FFmpeg has some implicit assumptions: int
> having 32bit being one of them (the log2 functions depend on this). And I

yes, that was from POSIX


> also thought that size_t being able to hold all the values of an unsigned
> was one of these implicit assumptions, too. Am I wrong on this?

I was asking myself the same, and i couldnt really find anything where we
stated that previously so i added a FFMIN.


> 
> A testcase for the last condition is easy to produce by simply manipulating
> the size field of a NAL unit.

yes, do you think we should create such a testcase for this fix ?

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."


signature.asc
Description: PGP signature
___
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 5/5] avcodec/hevc_mp4toannexb_bsf: Check that there is enough input left for nalu size

2019-12-13 Thread Andreas Rheinhardt
On Fri, Dec 13, 2019 at 5:56 PM Michael Niedermayer 
wrote:

> On Fri, Dec 13, 2019 at 12:24:04PM +0100, Andreas Rheinhardt wrote:
> > On Fri, Dec 13, 2019 at 3:07 AM Michael Niedermayer
> 
> > wrote:
> >
> > > No testcase
> > >
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/hevc_mp4toannexb_bsf.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> > > b/libavcodec/hevc_mp4toannexb_bsf.c
> > > index baa93628ed..e0d20a550c 100644
> > > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > > @@ -152,7 +152,9 @@ static int hevc_mp4toannexb_filter(AVBSFContext
> *ctx,
> > > AVPacket *out)
> > >  extra_size= add_extradata * ctx->par_out->extradata_size;
> > >  got_irap |= is_irap;
> > >
> > > -if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size)
> {
> > > +if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size
> ||
> > >
> >
> > Up until now I thought that FFmpeg has some implicit assumptions: int
> > having 32bit being one of them (the log2 functions depend on this). And I
>
> yes, that was from POSIX
>
>
> > also thought that size_t being able to hold all the values of an unsigned
> > was one of these implicit assumptions, too. Am I wrong on this?
>
> I was asking myself the same, and i couldnt really find anything where we
> stated that previously so i added a FFMIN.
>
> In this case we would have to add lots of checks before av_malloc and
other allocation functions that use size_t parameters in order to ensure
that no loss of information happens when an int (or unsigned) is converted
to size_t. In other words: We should not support such systems.

>
> >
> > A testcase for the last condition is easy to produce by simply
> manipulating
> > the size field of a NAL unit.
>
> yes, do you think we should create such a testcase for this fix ?
>

You mean a fate test? Why not?

- Andreas
___
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] avcodec/cbs_h2645: Fail on all 0 NAL units

2019-12-13 Thread Michael Niedermayer
On Thu, Dec 12, 2019 at 05:15:12PM -0500, Andriy Gelman wrote:
> On Thu, 12. Dec 18:43, Michael Niedermayer wrote:
> > Fixes: assertion failure
> > Fixes: 
> > 19286/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5707990724509696
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/cbs_h2645.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> > index 5f71d80584..5e0f4a9d98 100644
> > --- a/libavcodec/cbs_h2645.c
> > +++ b/libavcodec/cbs_h2645.c
> > @@ -568,7 +568,8 @@ static int 
> > cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
> >  // Remove trailing zeroes.
> >  while (size > 0 && nal->data[size - 1] == 0)
> >  --size;
> > -av_assert0(size > 0);
> 
> > +if (size == 0)
> > +return AVERROR_INVALIDDATA;
> 
> I suppose there could still be some valid nal units in this packet list.
> 
> Can you just continue in the loop and not insert this particular nal unit into
> frag? 

sure, will do that

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.


signature.asc
Description: PGP signature
___
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 4/5] avcodec/hevc_mp4toannexb_bsf: check that nalu size doesnt overflow

2019-12-13 Thread Michael Niedermayer
On Thu, Dec 12, 2019 at 07:36:50PM -0500, Andriy Gelman wrote:
> On Fri, 13. Dec 01:28, Michael Niedermayer wrote:
> > Fixes: Out of array access
> > Fixes: 
> > 19299/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_MP4TOANNEXB_fuzzer-5169193398042624
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/hevc_mp4toannexb_bsf.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c 
> > b/libavcodec/hevc_mp4toannexb_bsf.c
> > index d0f1b94f0e..baa93628ed 100644
> > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > @@ -152,8 +152,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, 
> > AVPacket *out)
> >  extra_size= add_extradata * ctx->par_out->extradata_size;
> >  got_irap |= is_irap;
> >  
> > -if (SIZE_MAX - nalu_size < 4 ||
> > -SIZE_MAX - 4 - nalu_size < extra_size) {
> > +if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) {
> >  ret = AVERROR_INVALIDDATA;
> >  goto fail;
> >  }
> > -- 
> > 2.24.0
> 
> I already sent a patch for this:
> http://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/253972.html

yes but it depends on "distant" code behaving in specific ways. I prefer
a single self contained check. Because its more robust to someone changing
that distant code.
So i would prefer to apply some "dumb" check. Do you agree ?


Thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides


signature.asc
Description: PGP signature
___
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, v3] FATE: add test for hevc files with 4 TUDepth(0..4) of cbf_cb/cbf_cr

2019-12-13 Thread Linjie Fu
5 cabac states for cbf_cb and cbf_cr are supported according to
Table 9-4.

Add a test for 64x64 4:4:4 8bit HEVC clips with TUDepth = 4, cbf_cr > 0.

Signed-off-by: Xu Guangxin 
Signed-off-by: Linjie Fu 
---
 tests/fate/hevc.mak   | 3 +++
 tests/ref/fate/hevc-cabac-tudepth | 6 ++
 2 files changed, 9 insertions(+)
 create mode 100644 tests/ref/fate/hevc-cabac-tudepth

diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
index 559c389..735949a 100644
--- a/tests/fate/hevc.mak
+++ b/tests/fate/hevc.mak
@@ -256,6 +256,9 @@ FATE_HEVC_FFPROBE-$(call DEMDEC, HEVC, HEVC) += 
fate-hevc-monochrome-crop
 fate-hevc-two-first-slice: CMD = threads=2 framemd5 -i 
$(TARGET_SAMPLES)/hevc/two_first_slice.mp4 -sws_flags bitexact -t 00:02.00 -an
 FATE_HEVC-$(call DEMDEC, MOV, HEVC) += fate-hevc-two-first-slice
 
+fate-hevc-cabac-tudepth: CMD = framecrc -flags unaligned -i 
$(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
+FATE_HEVC-$(call DEMDEC, HEVC, HEVC) += fate-hevc-cabac-tudepth
+
 FATE_SAMPLES_AVCONV += $(FATE_HEVC-yes)
 FATE_SAMPLES_FFPROBE += $(FATE_HEVC_FFPROBE-yes)
 
diff --git a/tests/ref/fate/hevc-cabac-tudepth 
b/tests/ref/fate/hevc-cabac-tudepth
new file mode 100644
index 000..ad874c3
--- /dev/null
+++ b/tests/ref/fate/hevc-cabac-tudepth
@@ -0,0 +1,6 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 64x64
+#sar 0: 0/1
+0,  0,  0,1,12288, 0x0127a0d9
-- 
2.7.4

___
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] FATE/hevc.mak: cosmetic for fate-hevc-paired-fields

2019-12-13 Thread Linjie Fu
Adjust the order of fate-hevc-paired-fields.

Signed-off-by: Linjie Fu 
---
 tests/fate/hevc.mak | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
index 735949a..35af3e4 100644
--- a/tests/fate/hevc.mak
+++ b/tests/fate/hevc.mak
@@ -226,9 +226,6 @@ $(foreach N,$(HEVC_SAMPLES_444_12BIT),$(eval $(call 
FATE_HEVC_TEST_444_12BIT,$(N
 fate-hevc-paramchange-yuv420p-yuv420p10: CMD = framecrc -vsync 0 -i 
$(TARGET_SAMPLES)/hevc/paramchange_yuv420p_yuv420p10.hevc -sws_flags 
area+accurate_rnd+bitexact
 FATE_HEVC += fate-hevc-paramchange-yuv420p-yuv420p10
 
-fate-hevc-paired-fields: CMD = probeframes -show_entries 
frame=interlaced_frame,top_field_first $(TARGET_SAMPLES)/hevc/paired_fields.hevc
-FATE_HEVC_FFPROBE-$(call DEMDEC, HEVC, HEVC) += fate-hevc-paired-fields
-
 tests/data/hevc-mp4.mov: TAG = GEN
 tests/data/hevc-mp4.mov: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data
$(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
@@ -250,6 +247,9 @@ FATE_HEVC-$(call DEMDEC, MOV, HEVC) += 
fate-hevc-extradata-reload
 
 fate-hevc-extradata-reload: CMD = framemd5 -i 
$(TARGET_SAMPLES)/hevc/extradata-reload-multi-stsd.mov -sws_flags bitexact
 
+fate-hevc-paired-fields: CMD = probeframes -show_entries 
frame=interlaced_frame,top_field_first $(TARGET_SAMPLES)/hevc/paired_fields.hevc
+FATE_HEVC_FFPROBE-$(call DEMDEC, HEVC, HEVC) += fate-hevc-paired-fields
+
 fate-hevc-monochrome-crop: CMD = probeframes -show_entries 
frame=width,height:stream=width,height 
$(TARGET_SAMPLES)/hevc/hevc-monochrome.hevc
 FATE_HEVC_FFPROBE-$(call DEMDEC, HEVC, HEVC) += fate-hevc-monochrome-crop
 
-- 
2.7.4

___
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]lavc/pnmdec: Fix 16bit decoding

2019-12-13 Thread Carl Eugen Hoyos
Hi!

Attached patch fixes decoding a sample provided on irc.

Please comment, Carl Eugen
From 9bf070aab1a88fb37db3c9322665edee9f90919f Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Fri, 13 Dec 2019 19:10:15 +0100
Subject: [PATCH] lavc/pnmdec: Fix 16bit decoding.

Regression since cdb5479c
Reported by irc user tTh from Mixart-Myrys
---
 libavcodec/pnmdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
index 958c5e43b0..dbcaef3884 100644
--- a/libavcodec/pnmdec.c
+++ b/libavcodec/pnmdec.c
@@ -143,7 +143,7 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data,
 v = (*s->bytestream++)&1;
 } else {
 /* read a sequence of digits */
-for (k = 0; k < 5 && c <= 9; k += 1) {
+for (k = 0; k < 6 && c <= 9; k += 1) {
 v = 10*v + c;
 c = (*s->bytestream++) - '0';
 }
-- 
2.23.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 5/5] avcodec/hevc_mp4toannexb_bsf: Check that there is enough input left for nalu size

2019-12-13 Thread Michael Niedermayer
On Fri, Dec 13, 2019 at 06:05:21PM +0100, Andreas Rheinhardt wrote:
> On Fri, Dec 13, 2019 at 5:56 PM Michael Niedermayer 
> wrote:
> 
> > On Fri, Dec 13, 2019 at 12:24:04PM +0100, Andreas Rheinhardt wrote:
> > > On Fri, Dec 13, 2019 at 3:07 AM Michael Niedermayer
> > 
> > > wrote:
> > >
> > > > No testcase
> > > >
> > > > Signed-off-by: Michael Niedermayer 
> > > > ---
> > > >  libavcodec/hevc_mp4toannexb_bsf.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c
> > > > b/libavcodec/hevc_mp4toannexb_bsf.c
> > > > index baa93628ed..e0d20a550c 100644
> > > > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > > > @@ -152,7 +152,9 @@ static int hevc_mp4toannexb_filter(AVBSFContext
> > *ctx,
> > > > AVPacket *out)
> > > >  extra_size= add_extradata * ctx->par_out->extradata_size;
> > > >  got_irap |= is_irap;
> > > >
> > > > -if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size)
> > {
> > > > +if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size
> > ||
> > > >
> > >
> > > Up until now I thought that FFmpeg has some implicit assumptions: int
> > > having 32bit being one of them (the log2 functions depend on this). And I
> >
> > yes, that was from POSIX
> >
> >
> > > also thought that size_t being able to hold all the values of an unsigned
> > > was one of these implicit assumptions, too. Am I wrong on this?
> >
> > I was asking myself the same, and i couldnt really find anything where we
> > stated that previously so i added a FFMIN.
> >
> > In this case we would have to add lots of checks before av_malloc and
> other allocation functions that use size_t parameters in order to ensure
> that no loss of information happens when an int (or unsigned) is converted
> to size_t. In other words: We should not support such systems.

If we would choose to support such platforms in the future then using our own
type thats the bigger of the 2 might make this relativly painless. But first
such a platform needs to be relevant for us and exist ...
And if we choose to assume things about these types, that should be in our
developer documentation.

But to return to the patch here, do you want me to remove the FFMIN ?

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


signature.asc
Description: PGP signature
___
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 4/5] avcodec/hevc_mp4toannexb_bsf: check that nalu size doesnt overflow

2019-12-13 Thread Andriy Gelman
On Fri, 13. Dec 18:51, Michael Niedermayer wrote:
> On Thu, Dec 12, 2019 at 07:36:50PM -0500, Andriy Gelman wrote:
> > On Fri, 13. Dec 01:28, Michael Niedermayer wrote:
> > > Fixes: Out of array access
> > > Fixes: 
> > > 19299/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_MP4TOANNEXB_fuzzer-5169193398042624
> > > 
> > > Found-by: continuous fuzzing process 
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/hevc_mp4toannexb_bsf.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/libavcodec/hevc_mp4toannexb_bsf.c 
> > > b/libavcodec/hevc_mp4toannexb_bsf.c
> > > index d0f1b94f0e..baa93628ed 100644
> > > --- a/libavcodec/hevc_mp4toannexb_bsf.c
> > > +++ b/libavcodec/hevc_mp4toannexb_bsf.c
> > > @@ -152,8 +152,7 @@ static int hevc_mp4toannexb_filter(AVBSFContext *ctx, 
> > > AVPacket *out)
> > >  extra_size= add_extradata * ctx->par_out->extradata_size;
> > >  got_irap |= is_irap;
> > >  
> > > -if (SIZE_MAX - nalu_size < 4 ||
> > > -SIZE_MAX - 4 - nalu_size < extra_size) {
> > > +if (FFMIN(INT_MAX, SIZE_MAX) < 4ULL + nalu_size + extra_size) {
> > >  ret = AVERROR_INVALIDDATA;
> > >  goto fail;
> > >  }
> > > -- 
> > > 2.24.0
> > 
> > I already sent a patch for this:
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/253972.html
> 
> yes but it depends on "distant" code behaving in specific ways. I prefer
> a single self contained check. Because its more robust to someone changing
> that distant code.
> So i would prefer to apply some "dumb" check. Do you agree ?

I agree it's more robust. I'll keep this in mind for future

Thanks,
-- 
Andriy
___
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] avcodec/cbs_av1: add missing valid range of values for num_cb_points and num_cr_points

2019-12-13 Thread James Almer
It is a requirement of bitstream conformance that num_cr_points is less than or 
equal to 10.
It is a requirement of bitstream conformance that num_cb_points is less than or 
equal to 10.

Signed-off-by: James Almer 
---
 libavcodec/cbs_av1.h | 8 
 libavcodec/cbs_av1_syntax_template.c | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index 643e76793f..9306bc59d6 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -260,11 +260,11 @@ typedef struct AV1RawFrameHeader {
 uint8_t  point_y_scaling[16];
 uint8_t  chroma_scaling_from_luma;
 uint8_t  num_cb_points;
-uint8_t  point_cb_value[16];
-uint8_t  point_cb_scaling[16];
+uint8_t  point_cb_value[10];
+uint8_t  point_cb_scaling[10];
 uint8_t  num_cr_points;
-uint8_t  point_cr_value[16];
-uint8_t  point_cr_scaling[16];
+uint8_t  point_cr_value[10];
+uint8_t  point_cr_scaling[10];
 uint8_t  grain_scaling_minus_8;
 uint8_t  ar_coeff_lag;
 uint8_t  ar_coeffs_y_plus_128[24];
diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index f53955c52e..848348af7d 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -1174,12 +1174,12 @@ static int 
FUNC(film_grain_params)(CodedBitstreamContext *ctx, RWContext *rw,
 infer(num_cb_points, 0);
 infer(num_cr_points, 0);
 } else {
-fb(4, num_cb_points);
+fc(4, num_cb_points, 0, 10);
 for (i = 0; i < current->num_cb_points; i++) {
 fbs(8, point_cb_value[i],   1, i);
 fbs(8, point_cb_scaling[i], 1, i);
 }
-fb(4, num_cr_points);
+fc(4, num_cr_points, 0, 10);
 for (i = 0; i < current->num_cr_points; i++) {
 fbs(8, point_cr_value[i],   1, i);
 fbs(8, point_cr_scaling[i], 1, i);
-- 
2.24.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] [PATCHv2] fate: Add an option for disabling the 2k/4k tests

2019-12-13 Thread James Almer
On 12/13/2019 5:34 AM, Martin Storsjö wrote:
> On Thu, 12 Dec 2019, James Almer wrote:
> 
>> On 12/12/2019 7:03 PM, Martin Storsjö wrote:
>>> On Wed, 11 Dec 2019, Martin Storsjö wrote:
>>>
 On Wed, 11 Dec 2019, Carl Eugen Hoyos wrote:

> Am Mi., 11. Dez. 2019 um 09:39 Uhr schrieb Martin Storsjö 
 :
>>
>> When testing on a memory limited system, these tests consume a
>> significant amount of memory and can often fail if testing by running
>> multiple processes in parallel.
>> ---
>> Adjusted to use ALLYES instead of a -yes-yes construct.
>>
>> Also moved the 2k tests to the same option.
>> ---
>>  configure | 3 +++
>>  tests/fate/seek.mak   | 3 ++-
>>  tests/fate/vcodec.mak | 5 +++--
>>  3 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index ca7137f341..922cd8d0ee 100755
>> --- a/configure
>> +++ b/configure
>> @@ -482,6 +482,7 @@ Developer options (useful when working on FFmpeg 
 itself):
>>    --ignore-tests=TESTS comma-separated list (without "fate-"
>> prefix
>>     in the name) of tests whose result is
>> ignored
>>    --enable-linux-perf  enable Linux Performance Monitor API
>> +  --disable-large-tests    disable tests that use a large amount of 
 memory
>
> I would have suggested to control this when running the tests, if the 
 configure
> setting makes sense, it should at least be possible to change the
> setting 
 when
> calling make.
> Or is that possible anyway?

 It's possible to do e.g. "make fate CONFIG_LARGE_TESTS=no"; any
 var=value assignment on the make command line overrides any
 var=othervalue assignment within the makefiles themselves, but that
 doesn't seem very convenient.

 But I'd like to have it as a configure option, to easily be able to
 set it e.g. in a fate setup.
>>>
>>> Any further opinions on this one - is it ok to go ahead with it in this
>>> form, or are changes requested?
>>
>> Configure option is fine if it can also be overridden from the command
>> line at runtime (like --samples and SAMPLES).
> 
> Well, it can be overridden at runtime, but it's not with a very
> convenient name (the CONFIG_* variable). Is that ok?

Yes, it's a developer debug option. As long as the possibility is there,
it's ok.
___
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] [PATCHv2] fate: Add an option for disabling the 2k/4k tests

2019-12-13 Thread Carl Eugen Hoyos


> Am 13.12.2019 um 09:34 schrieb Martin Storsjö :
> 
>> On Thu, 12 Dec 2019, James Almer wrote:
>> 
>>> On 12/12/2019 7:03 PM, Martin Storsjö wrote:
 On Wed, 11 Dec 2019, Martin Storsjö wrote:
> On Wed, 11 Dec 2019, Carl Eugen Hoyos wrote:
> 
> Am Mi., 11. Dez. 2019 um 09:39 Uhr schrieb Martin Storsjö 
 :
>> 
>> When testing on a memory limited system, these tests consume a
>> significant amount of memory and can often fail if testing by running
>> multiple processes in parallel.
>> ---
>> Adjusted to use ALLYES instead of a -yes-yes construct.
>> 
>> Also moved the 2k tests to the same option.
>> ---
>>  configure | 3 +++
>>  tests/fate/seek.mak   | 3 ++-
>>  tests/fate/vcodec.mak | 5 +++--
>>  3 files changed, 8 insertions(+), 3 deletions(-)
>> 
>> diff --git a/configure b/configure
>> index ca7137f341..922cd8d0ee 100755
>> --- a/configure
>> +++ b/configure
>> @@ -482,6 +482,7 @@ Developer options (useful when working on FFmpeg 
 itself):
>>--ignore-tests=TESTS comma-separated list (without "fate-"
>> prefix
>> in the name) of tests whose result is
>> ignored
>>--enable-linux-perf  enable Linux Performance Monitor API
>> +  --disable-large-testsdisable tests that use a large amount of 
 memory
> 
> I would have suggested to control this when running the tests, if the 
 configure
> setting makes sense, it should at least be possible to change the
> setting 
 when
> calling make.
> Or is that possible anyway?
 
 It's possible to do e.g. "make fate CONFIG_LARGE_TESTS=no"; any
 var=value assignment on the make command line overrides any
 var=othervalue assignment within the makefiles themselves, but that
 doesn't seem very convenient.
 
 But I'd like to have it as a configure option, to easily be able to
 set it e.g. in a fate setup.
>>> Any further opinions on this one - is it ok to go ahead with it in this
>>> form, or are changes requested?
>> 
>> Configure option is fine if it can also be overridden from the command
>> line at runtime (like --samples and SAMPLES).
> 
> Well, it can be overridden at runtime, but it's not with a very convenient 
> name (the CONFIG_* variable). Is that ok?

Ideally, this would be possible with:
make BIG=no fate

Carl Eugen
___
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/pnmdec: Fix 16bit decoding

2019-12-13 Thread Michael Niedermayer
On Fri, Dec 13, 2019 at 07:13:20PM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch fixes decoding a sample provided on irc.
> 
> Please comment, Carl Eugen

>  pnmdec.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> b5422d22058919ba63c80c3070660ab1f0908069  
> 0001-lavc-pnmdec-Fix-16bit-decoding.patch
> From 9bf070aab1a88fb37db3c9322665edee9f90919f Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos 
> Date: Fri, 13 Dec 2019 19:10:15 +0100
> Subject: [PATCH] lavc/pnmdec: Fix 16bit decoding.
> 
> Regression since cdb5479c
> Reported by irc user tTh from Mixart-Myrys
> ---
>  libavcodec/pnmdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c
> index 958c5e43b0..dbcaef3884 100644
> --- a/libavcodec/pnmdec.c
> +++ b/libavcodec/pnmdec.c
> @@ -143,7 +143,7 @@ static int pnm_decode_frame(AVCodecContext *avctx, void 
> *data,
>  v = (*s->bytestream++)&1;
>  } else {
>  /* read a sequence of digits */
> -for (k = 0; k < 5 && c <= 9; k += 1) {
> +for (k = 0; k < 6 && c <= 9; k += 1) {

should be ok

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato


signature.asc
Description: PGP signature
___
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] avcodec/simple_idct_template: fix integer overflow

2019-12-13 Thread Michael Niedermayer
On Fri, Dec 13, 2019 at 03:22:17PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/simple_idct_template.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

LGTM

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


signature.asc
Description: PGP signature
___
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] avcodec/cbs_h2645: Skip all 0 NAL units

2019-12-13 Thread Michael Niedermayer
Fixes: assertion failure
Fixes: 
19286/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5707990724509696

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/cbs_h2645.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 5f71d80584..90f77d0039 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -568,7 +568,10 @@ static int 
cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
 // Remove trailing zeroes.
 while (size > 0 && nal->data[size - 1] == 0)
 --size;
-av_assert0(size > 0);
+if (size == 0) {
+av_log(ctx->log_ctx, AV_LOG_WARNING, "Discarding empty 0 NAL 
unit\n");
+continue;
+}
 
 ref = (nal->data == nal->raw_data) ? frag->data_ref
: packet->rbsp.rbsp_buffer_ref;
-- 
2.24.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] [PATCHv2] fate: Add an option for disabling the 2k/4k tests

2019-12-13 Thread Martin Storsjö

On Fri, 13 Dec 2019, Carl Eugen Hoyos wrote:





Am 13.12.2019 um 09:34 schrieb Martin Storsjö :


On Thu, 12 Dec 2019, James Almer wrote:


On 12/12/2019 7:03 PM, Martin Storsjö wrote:

On Wed, 11 Dec 2019, Martin Storsjö wrote:

On Wed, 11 Dec 2019, Carl Eugen Hoyos wrote:

Am Mi., 11. Dez. 2019 um 09:39 Uhr schrieb Martin Storsjö 

:


When testing on a memory limited system, these tests consume a
significant amount of memory and can often fail if testing by running
multiple processes in parallel.
---
Adjusted to use ALLYES instead of a -yes-yes construct.

Also moved the 2k tests to the same option.
---
 configure | 3 +++
 tests/fate/seek.mak   | 3 ++-
 tests/fate/vcodec.mak | 5 +++--
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index ca7137f341..922cd8d0ee 100755
--- a/configure
+++ b/configure
@@ -482,6 +482,7 @@ Developer options (useful when working on FFmpeg 

itself):

   --ignore-tests=TESTS comma-separated list (without "fate-"
prefix
in the name) of tests whose result is
ignored
   --enable-linux-perf  enable Linux Performance Monitor API
+  --disable-large-testsdisable tests that use a large amount of 

memory


I would have suggested to control this when running the tests, if the 

configure

setting makes sense, it should at least be possible to change the
setting 

when

calling make.
Or is that possible anyway?


It's possible to do e.g. "make fate CONFIG_LARGE_TESTS=no"; any
var=value assignment on the make command line overrides any
var=othervalue assignment within the makefiles themselves, but that
doesn't seem very convenient.

But I'd like to have it as a configure option, to easily be able to
set it e.g. in a fate setup.

Any further opinions on this one - is it ok to go ahead with it in this
form, or are changes requested?


Configure option is fine if it can also be overridden from the command
line at runtime (like --samples and SAMPLES).


Well, it can be overridden at runtime, but it's not with a very convenient name 
(the CONFIG_* variable). Is that ok?


Ideally, this would be possible with:
make BIG=no fate


That requires a bit more extra intermediate variables.

One way of doing it would be this:

# Default, overriden by any "make BIG=no fate"
BIG=$(CONFIG_LARGE_TESTS)
...
TESTS-$(ENCDEC components)-$(BIG) += some-tests
TESTS += TESTS-yes-yes

While it earlier was requested to use $(ALLYES ...) instead of the 
-yes-yes construct.


Or to keep using ALLYES, we'd need yet another intermediate variable:

# Default, overriden by any "make BIG=no fate"
BIG=$(CONFIG_LARGE_TESTS)
# The same as BIG above, but with a CONFIG_ prefix
CONFIG_BIG=$(BIG)
...
TESTS-$(ALLYES components BIG) += some-tests
TESTS += TESTS-yes



James, what's your opinion on these two alternatives, if it should be 
configurable with a different variable name?


// Martin
___
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] avcodec/cbs_h2645: Skip all 0 NAL units

2019-12-13 Thread James Almer
On 12/13/2019 7:17 PM, Michael Niedermayer wrote:
> Fixes: assertion failure
> Fixes: 
> 19286/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5707990724509696
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/cbs_h2645.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 5f71d80584..90f77d0039 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -568,7 +568,10 @@ static int 
> cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
>  // Remove trailing zeroes.
>  while (size > 0 && nal->data[size - 1] == 0)
>  --size;
> -av_assert0(size > 0);
> +if (size == 0) {
> +av_log(ctx->log_ctx, AV_LOG_WARNING, "Discarding empty 0 NAL 
> unit\n");

IMO, no warning message, same as h2645_parse. Or if anything, make it
VERBOSE.

LGTM otherwise.

> +continue;
> +}
>  
>  ref = (nal->data == nal->raw_data) ? frag->data_ref
> : packet->rbsp.rbsp_buffer_ref;
> 

___
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] [PATCHv2] fate: Add an option for disabling the 2k/4k tests

2019-12-13 Thread James Almer
On 12/13/2019 7:22 PM, Martin Storsjö wrote:
> On Fri, 13 Dec 2019, Carl Eugen Hoyos wrote:
> 
>>
>>
>>> Am 13.12.2019 um 09:34 schrieb Martin Storsjö :
>>>
 On Thu, 12 Dec 2019, James Almer wrote:

> On 12/12/2019 7:03 PM, Martin Storsjö wrote:
>> On Wed, 11 Dec 2019, Martin Storsjö wrote:
>>> On Wed, 11 Dec 2019, Carl Eugen Hoyos wrote:
>>>
>>> Am Mi., 11. Dez. 2019 um 09:39 Uhr schrieb Martin Storsjö 
>> :

 When testing on a memory limited system, these tests consume a
 significant amount of memory and can often fail if testing by
 running
 multiple processes in parallel.
 ---
 Adjusted to use ALLYES instead of a -yes-yes construct.

 Also moved the 2k tests to the same option.
 ---
  configure | 3 +++
  tests/fate/seek.mak   | 3 ++-
  tests/fate/vcodec.mak | 5 +++--
  3 files changed, 8 insertions(+), 3 deletions(-)

 diff --git a/configure b/configure
 index ca7137f341..922cd8d0ee 100755
 --- a/configure
 +++ b/configure
 @@ -482,6 +482,7 @@ Developer options (useful when working on
 FFmpeg 
>> itself):
    --ignore-tests=TESTS comma-separated list (without "fate-"
 prefix
     in the name) of tests whose result is
 ignored
    --enable-linux-perf  enable Linux Performance Monitor API
 +  --disable-large-tests    disable tests that use a large
 amount of 
>> memory
>>>
>>> I would have suggested to control this when running the tests, if
>>> the 
>> configure
>>> setting makes sense, it should at least be possible to change the
>>> setting 
>> when
>>> calling make.
>>> Or is that possible anyway?
>>
>> It's possible to do e.g. "make fate CONFIG_LARGE_TESTS=no"; any
>> var=value assignment on the make command line overrides any
>> var=othervalue assignment within the makefiles themselves, but that
>> doesn't seem very convenient.
>>
>> But I'd like to have it as a configure option, to easily be able to
>> set it e.g. in a fate setup.
> Any further opinions on this one - is it ok to go ahead with it in
> this
> form, or are changes requested?

 Configure option is fine if it can also be overridden from the command
 line at runtime (like --samples and SAMPLES).
>>>
>>> Well, it can be overridden at runtime, but it's not with a very
>>> convenient name (the CONFIG_* variable). Is that ok?
>>
>> Ideally, this would be possible with:
>> make BIG=no fate
> 
> That requires a bit more extra intermediate variables.
> 
> One way of doing it would be this:
> 
> # Default, overriden by any "make BIG=no fate"
> BIG=$(CONFIG_LARGE_TESTS)
> ...
> TESTS-$(ENCDEC components)-$(BIG) += some-tests
> TESTS += TESTS-yes-yes
> 
> While it earlier was requested to use $(ALLYES ...) instead of the
> -yes-yes construct.
> 
> Or to keep using ALLYES, we'd need yet another intermediate variable:
> 
> # Default, overriden by any "make BIG=no fate"
> BIG=$(CONFIG_LARGE_TESTS)
> # The same as BIG above, but with a CONFIG_ prefix
> CONFIG_BIG=$(BIG)
> ...
> TESTS-$(ALLYES components BIG) += some-tests
> TESTS += TESTS-yes
> 
> 
> 
> James, what's your opinion on these two alternatives, if it should be
> configurable with a different variable name?

BIG is too generic and could be used for anything. LARGE_TESTS would be
better, and would get rid of the need to add a new custom CONFIG_
variable for the second example using ALLYES.
___
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] [PATCHv2] fate: Add an option for disabling the 2k/4k tests

2019-12-13 Thread Martin Storsjö

On Fri, 13 Dec 2019, James Almer wrote:


On 12/13/2019 7:22 PM, Martin Storsjö wrote:

On Fri, 13 Dec 2019, Carl Eugen Hoyos wrote:





Am 13.12.2019 um 09:34 schrieb Martin Storsjö :


On Thu, 12 Dec 2019, James Almer wrote:


On 12/12/2019 7:03 PM, Martin Storsjö wrote:

On Wed, 11 Dec 2019, Martin Storsjö wrote:

On Wed, 11 Dec 2019, Carl Eugen Hoyos wrote:

Am Mi., 11. Dez. 2019 um 09:39 Uhr schrieb Martin Storsjö 

:


When testing on a memory limited system, these tests consume a
significant amount of memory and can often fail if testing by
running
multiple processes in parallel.
---
Adjusted to use ALLYES instead of a -yes-yes construct.

Also moved the 2k tests to the same option.
---
 configure | 3 +++
 tests/fate/seek.mak   | 3 ++-
 tests/fate/vcodec.mak | 5 +++--
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index ca7137f341..922cd8d0ee 100755
--- a/configure
+++ b/configure
@@ -482,6 +482,7 @@ Developer options (useful when working on
FFmpeg 

itself):

   --ignore-tests=TESTS comma-separated list (without "fate-"
prefix
    in the name) of tests whose result is
ignored
   --enable-linux-perf  enable Linux Performance Monitor API
+  --disable-large-tests    disable tests that use a large
amount of 

memory


I would have suggested to control this when running the tests, if
the 

configure

setting makes sense, it should at least be possible to change the
setting 

when

calling make.
Or is that possible anyway?


It's possible to do e.g. "make fate CONFIG_LARGE_TESTS=no"; any
var=value assignment on the make command line overrides any
var=othervalue assignment within the makefiles themselves, but that
doesn't seem very convenient.

But I'd like to have it as a configure option, to easily be able to
set it e.g. in a fate setup.

Any further opinions on this one - is it ok to go ahead with it in
this
form, or are changes requested?


Configure option is fine if it can also be overridden from the command
line at runtime (like --samples and SAMPLES).


Well, it can be overridden at runtime, but it's not with a very
convenient name (the CONFIG_* variable). Is that ok?


Ideally, this would be possible with:
make BIG=no fate


That requires a bit more extra intermediate variables.

One way of doing it would be this:

# Default, overriden by any "make BIG=no fate"
BIG=$(CONFIG_LARGE_TESTS)
...
TESTS-$(ENCDEC components)-$(BIG) += some-tests
TESTS += TESTS-yes-yes

While it earlier was requested to use $(ALLYES ...) instead of the
-yes-yes construct.

Or to keep using ALLYES, we'd need yet another intermediate variable:

# Default, overriden by any "make BIG=no fate"
BIG=$(CONFIG_LARGE_TESTS)
# The same as BIG above, but with a CONFIG_ prefix
CONFIG_BIG=$(BIG)
...
TESTS-$(ALLYES components BIG) += some-tests
TESTS += TESTS-yes



James, what's your opinion on these two alternatives, if it should be
configurable with a different variable name?


BIG is too generic and could be used for anything. LARGE_TESTS would be
better, and would get rid of the need to add a new custom CONFIG_
variable for the second example using ALLYES.


I intentionally meant to use a different variable for that, to 
differentiate between the configure-generated CONFIG_LARGE_TESTS from 
config.mak and the one that is set to pick up a potential user-supplied 
value from e.g. LARGE_TESTS, otherwise falling back on the configure 
generated value - I'm not sure if that really works if the first and last 
variable are the same, or if that ends up as an infinite recursion 
otherwise (CONFIG_LARGE_TESTS expands to LARGE_TESTS which expands to 
CONFIG_LARGE_TESTS).


// Martin
___
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] [PATCHv2] fate: Add an option for disabling the 2k/4k tests

2019-12-13 Thread James Almer
On 12/13/2019 8:07 PM, Martin Storsjö wrote:
> On Fri, 13 Dec 2019, James Almer wrote:
> 
>> On 12/13/2019 7:22 PM, Martin Storsjö wrote:
>>> On Fri, 13 Dec 2019, Carl Eugen Hoyos wrote:
>>>


> Am 13.12.2019 um 09:34 schrieb Martin Storsjö :
>
>> On Thu, 12 Dec 2019, James Almer wrote:
>>
>>> On 12/12/2019 7:03 PM, Martin Storsjö wrote:
 On Wed, 11 Dec 2019, Martin Storsjö wrote:
> On Wed, 11 Dec 2019, Carl Eugen Hoyos wrote:
>
> Am Mi., 11. Dez. 2019 um 09:39 Uhr schrieb Martin Storsjö 
 :
>>
>> When testing on a memory limited system, these tests consume a
>> significant amount of memory and can often fail if testing by
>> running
>> multiple processes in parallel.
>> ---
>> Adjusted to use ALLYES instead of a -yes-yes construct.
>>
>> Also moved the 2k tests to the same option.
>> ---
>>  configure | 3 +++
>>  tests/fate/seek.mak   | 3 ++-
>>  tests/fate/vcodec.mak | 5 +++--
>>  3 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index ca7137f341..922cd8d0ee 100755
>> --- a/configure
>> +++ b/configure
>> @@ -482,6 +482,7 @@ Developer options (useful when working on
>> FFmpeg 
 itself):
>>    --ignore-tests=TESTS comma-separated list (without "fate-"
>> prefix
>>     in the name) of tests whose result is
>> ignored
>>    --enable-linux-perf  enable Linux Performance Monitor API
>> +  --disable-large-tests    disable tests that use a large
>> amount of 
 memory
>
> I would have suggested to control this when running the tests, if
> the 
 configure
> setting makes sense, it should at least be possible to change the
> setting 
 when
> calling make.
> Or is that possible anyway?

 It's possible to do e.g. "make fate CONFIG_LARGE_TESTS=no"; any
 var=value assignment on the make command line overrides any
 var=othervalue assignment within the makefiles themselves, but that
 doesn't seem very convenient.

 But I'd like to have it as a configure option, to easily be able to
 set it e.g. in a fate setup.
>>> Any further opinions on this one - is it ok to go ahead with it in
>>> this
>>> form, or are changes requested?
>>
>> Configure option is fine if it can also be overridden from the
>> command
>> line at runtime (like --samples and SAMPLES).
>
> Well, it can be overridden at runtime, but it's not with a very
> convenient name (the CONFIG_* variable). Is that ok?

 Ideally, this would be possible with:
 make BIG=no fate
>>>
>>> That requires a bit more extra intermediate variables.
>>>
>>> One way of doing it would be this:
>>>
>>> # Default, overriden by any "make BIG=no fate"
>>> BIG=$(CONFIG_LARGE_TESTS)
>>> ...
>>> TESTS-$(ENCDEC components)-$(BIG) += some-tests
>>> TESTS += TESTS-yes-yes
>>>
>>> While it earlier was requested to use $(ALLYES ...) instead of the
>>> -yes-yes construct.
>>>
>>> Or to keep using ALLYES, we'd need yet another intermediate variable:
>>>
>>> # Default, overriden by any "make BIG=no fate"
>>> BIG=$(CONFIG_LARGE_TESTS)
>>> # The same as BIG above, but with a CONFIG_ prefix
>>> CONFIG_BIG=$(BIG)
>>> ...
>>> TESTS-$(ALLYES components BIG) += some-tests
>>> TESTS += TESTS-yes
>>>
>>>
>>>
>>> James, what's your opinion on these two alternatives, if it should be
>>> configurable with a different variable name?
>>
>> BIG is too generic and could be used for anything. LARGE_TESTS would be
>> better, and would get rid of the need to add a new custom CONFIG_
>> variable for the second example using ALLYES.
> 
> I intentionally meant to use a different variable for that, to
> differentiate between the configure-generated CONFIG_LARGE_TESTS from
> config.mak and the one that is set to pick up a potential user-supplied
> value from e.g. LARGE_TESTS, otherwise falling back on the configure
> generated value - I'm not sure if that really works if the first and
> last variable are the same, or if that ends up as an infinite recursion
> otherwise (CONFIG_LARGE_TESTS expands to LARGE_TESTS which expands to
> CONFIG_LARGE_TESTS).

I still think just overriding using CONFIG_LARGE_TESTS from the command
line is more than enough for a developer debug option, but if others
want something shorter then your suggestion above for ALLYES is fine.
___
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 1/2] lavc/h2645_parse: Don't automatically remove nuh_layer_id > 0 packets

2019-12-13 Thread Andriy Gelman
On Thu, 05. Dec 22:15, Andriy Gelman wrote:
> From: Andriy Gelman 
> 
> HEVC standard supports multi-layer streams (ITU-T H.265 02/2018 Annex
> F). Each NAL unit belongs to a particular layer defined by nuh_layer_id
> in the header.
> 
> Currently, all NAL units that do not belong to a base layer are
> automatically removed in ff_h2645_packet_split(). Some data may
> therefore be lost when future filters/decoders are designed to support
> multi-layer streams.
> 
> A better approach is to forward nuh_layer_id > 0 packets and let blocks
> down the chain decide how to process them. The condition to remove
> packets has been moved to hevcdec and cbs.
> 
> Found-by: Andreas Rheinhardt 
> Signed-off-by: Andriy Gelman 
> ---
>  libavcodec/cbs_h2645.c   | 3 +++
>  libavcodec/h2645_parse.c | 7 +++
>  libavcodec/h2645_parse.h | 5 +
>  libavcodec/hevc_parse.c  | 2 ++
>  libavcodec/hevc_parser.c | 3 +++
>  libavcodec/hevcdec.c | 2 +-
>  6 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> index 88fa0029cd6..ae32078f246 100644
> --- a/libavcodec/cbs_h2645.c
> +++ b/libavcodec/cbs_h2645.c
> @@ -565,6 +565,9 @@ static int 
> cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
>  AVBufferRef *ref;
>  size_t size = nal->size;
>  
> +if (nal->nuh_layer_id > 0)
> +continue;
> +
>  // Remove trailing zeroes.
>  while (size > 0 && nal->data[size - 1] == 0)
>  --size;
> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
> index 4808f79a67f..0f3343004f9 100644
> --- a/libavcodec/h2645_parse.c
> +++ b/libavcodec/h2645_parse.c
> @@ -292,23 +292,22 @@ static int get_bit_length(H2645NAL *nal, int 
> skip_trailing_zeros)
>  static int hevc_parse_nal_header(H2645NAL *nal, void *logctx)
>  {
>  GetBitContext *gb = &nal->gb;
> -int nuh_layer_id;
>  
>  if (get_bits1(gb) != 0)
>  return AVERROR_INVALIDDATA;
>  
>  nal->type = get_bits(gb, 6);
>  
> -nuh_layer_id   = get_bits(gb, 6);
> +nal->nuh_layer_id = get_bits(gb, 6);
>  nal->temporal_id = get_bits(gb, 3) - 1;
>  if (nal->temporal_id < 0)
>  return AVERROR_INVALIDDATA;
>  
>  av_log(logctx, AV_LOG_DEBUG,
> "nal_unit_type: %d(%s), nuh_layer_id: %d, temporal_id: %d\n",
> -   nal->type, hevc_nal_unit_name(nal->type), nuh_layer_id, 
> nal->temporal_id);
> +   nal->type, hevc_nal_unit_name(nal->type), nal->nuh_layer_id, 
> nal->temporal_id);
>  
> -return nuh_layer_id == 0;
> +return 1;
>  }
>  
>  static int h264_parse_nal_header(H2645NAL *nal, void *logctx)
> diff --git a/libavcodec/h2645_parse.h b/libavcodec/h2645_parse.h
> index 2acf882d3da..3e47f86c53b 100644
> --- a/libavcodec/h2645_parse.h
> +++ b/libavcodec/h2645_parse.h
> @@ -56,6 +56,11 @@ typedef struct H2645NAL {
>   */
>  int temporal_id;
>  
> +/*
> + * HEVC only, identifier of layer to which nal unit belongs
> + */
> +int nuh_layer_id;
> +
>  int skipped_bytes;
>  int skipped_bytes_pos_size;
>  int *skipped_bytes_pos;
> diff --git a/libavcodec/hevc_parse.c b/libavcodec/hevc_parse.c
> index dddb293df64..29dfd479f38 100644
> --- a/libavcodec/hevc_parse.c
> +++ b/libavcodec/hevc_parse.c
> @@ -37,6 +37,8 @@ static int hevc_decode_nal_units(const uint8_t *buf, int 
> buf_size, HEVCParamSets
>  
>  for (i = 0; i < pkt.nb_nals; i++) {
>  H2645NAL *nal = &pkt.nals[i];
> +if (nal->nuh_layer_id > 0)
> +continue;
>  
>  /* ignore everything except parameter sets and VCL NALUs */
>  switch (nal->type) {
> diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
> index b444b999550..87d5dba4f5e 100644
> --- a/libavcodec/hevc_parser.c
> +++ b/libavcodec/hevc_parser.c
> @@ -202,6 +202,9 @@ static int parse_nal_units(AVCodecParserContext *s, const 
> uint8_t *buf,
>  H2645NAL *nal = &ctx->pkt.nals[i];
>  GetBitContext *gb = &nal->gb;
>  
> +if (nal->nuh_layer_id > 0)
> +continue;
> +
>  switch (nal->type) {
>  case HEVC_NAL_VPS:
>  ff_hevc_decode_nal_vps(gb, avctx, ps);
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 8f1c162acee..bcd8e67944a 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -3077,7 +3077,7 @@ static int decode_nal_units(HEVCContext *s, const 
> uint8_t *buf, int length)
>  
>  if (s->avctx->skip_frame >= AVDISCARD_ALL ||
>  (s->avctx->skip_frame >= AVDISCARD_NONREF
> -&& ff_hevc_nal_is_nonref(nal->type)))
> +&& ff_hevc_nal_is_nonref(nal->type)) || nal->nuh_layer_id > 0)
>  continue;
>  
>  ret = decode_nal_unit(s, nal);
> -- 
> 2.24.0

ping for both patches

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

To un

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/vf_histogram: clean up code

2019-12-13 Thread zhilizhao
Hi Paul,

> On Dec 13, 2019, at 10:00 PM, Paul B Mahol  wrote:
> 
> Please provide some explanation.

No functional changes, just use temporary variables to make code more readable,
and may improve a little bit of performance.

> 
> On 12/13/19, quinkbl...@foxmail.com  wrote:
>> From: Zhao Zhili 
>> 
>> ---
>> libavfilter/vf_histogram.c | 16 
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/libavfilter/vf_histogram.c b/libavfilter/vf_histogram.c
>> index 5185992de6..0d2d087beb 100644
>> --- a/libavfilter/vf_histogram.c
>> +++ b/libavfilter/vf_histogram.c
>> @@ -266,20 +266,20 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
>> *in)
>> const int is_chroma = (k == 1 || k == 2);
>> const int dst_h = AV_CEIL_RSHIFT(outlink->h, (is_chroma ?
>> h->odesc->log2_chroma_h : 0));
>> const int dst_w = AV_CEIL_RSHIFT(outlink->w, (is_chroma ?
>> h->odesc->log2_chroma_w : 0));
>> +const int plane = h->odesc->comp[k].plane;
>> +uint8_t *const data = out->data[plane];
>> +const int linesize = out->linesize[plane];
>> 
>> if (h->histogram_size <= 256) {
>> for (i = 0; i < dst_h ; i++)
>> -memset(out->data[h->odesc->comp[k].plane] +
>> -   i * out->linesize[h->odesc->comp[k].plane],
>> -   h->bg_color[k], dst_w);
>> +memset(data + i * linesize, h->bg_color[k], dst_w);
>> } else {
>> const int mult = h->mult;
>> 
>> -for (i = 0; i < dst_h ; i++)
>> -for (j = 0; j < dst_w; j++)
>> -AV_WN16(out->data[h->odesc->comp[k].plane] +
>> -i * out->linesize[h->odesc->comp[k].plane] + j * 2,
>> -h->bg_color[k] * mult);
>> +for (j = 0; j < dst_w; j++)
>> +AV_WN16(data + j * 2, h->bg_color[k] * mult);
>> +for (i = 1; i < dst_h; i++)
>> +memcpy(data + i * linesize, data, dst_w * 2);
>> }
>> }
>> 
>> --
>> 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".
> ___
> 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 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".