Re: [FFmpeg-devel] [PATCH] lavc/x265: set preferred_transfer_characteristics for HLG
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
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
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
> 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> 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
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
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
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
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
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
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
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
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
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
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".