[FFmpeg-devel] [PATCH V2] avcodec/libx265: add support for ROI-based encoding
Signed-off-by: Guo, Yejun
---
libavcodec/libx265.c | 67
1 file changed, 67 insertions(+)
diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 27c90b3..73a7e62 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -285,6 +285,65 @@ static av_cold int libx265_encode_init(AVCodecContext
*avctx)
return 0;
}
+static av_cold int libx265_encode_set_roi(libx265Context *ctx, const AVFrame
*frame, x265_picture* pic)
+{
+AVFrameSideData *sd = av_frame_get_side_data(frame,
AV_FRAME_DATA_REGIONS_OF_INTEREST);
+if (sd) {
+if (ctx->params->rc.aqMode == X265_AQ_NONE) {
+av_log(ctx, AV_LOG_WARNING, "Adaptive quantization must be enabled
to use ROI encoding, skipping ROI.\n");
+} else {
+// 8x8 block when qg-size is 8, 16*16 block otherwise
+int mb_size = (ctx->params->rc.qgSize == 8) ? 8 : 16;
+int mbx = (frame->width + mb_size - 1) / mb_size;
+int mby = (frame->height + mb_size - 1) / mb_size;
+int nb_rois;
+AVRegionOfInterest* roi;
+float* qoffsets; // will be freed after encode is called
+qoffsets = av_mallocz_array(mbx * mby, sizeof(*qoffsets));
+if (!qoffsets)
+return AVERROR(ENOMEM);
+
+nb_rois = sd->size / sizeof(AVRegionOfInterest);
+roi = (AVRegionOfInterest*)sd->data;
+for (int count = 0; count < nb_rois; count++) {
+int starty = FFMIN(mby, roi->top / mb_size);
+int endy = FFMIN(mby, (roi->bottom + mb_size - 1)/ mb_size);
+int startx = FFMIN(mbx, roi->left / mb_size);
+int endx = FFMIN(mbx, (roi->right + mb_size - 1)/ mb_size);
+float qoffset;
+
+if (roi->qoffset.den == 0) {
+av_free(qoffsets);
+av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den
should not be zero.\n");
+return AVERROR(EINVAL);
+}
+qoffset = roi->qoffset.num * 1.0f / roi->qoffset.den;
+qoffset = av_clipf(qoffset, -1.0f, 1.0f);
+
+// qp range of x265 is from 0 to 51, just choose 25 as the
scale value,
+// so the range of final qoffset is [-25.0, 25.0].
+qoffset = qoffset * 25;
+
+for (int y = starty; y < endy; y++) {
+for (int x = startx; x < endx; x++) {
+qoffsets[x + y*mbx] = qoffset;
+}
+}
+
+if (roi->self_size == 0) {
+av_free(qoffsets);
+av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size
should be set to sizeof(AVRegionOfInterest).\n");
+return AVERROR(EINVAL);
+}
+roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
+}
+
+pic->quantOffsets = qoffsets;
+}
+}
+return 0;
+}
+
static int libx265_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
const AVFrame *pic, int *got_packet)
{
@@ -314,10 +373,18 @@ static int libx265_encode_frame(AVCodecContext *avctx,
AVPacket *pkt,
pic->pict_type == AV_PICTURE_TYPE_P ? X265_TYPE_P :
pic->pict_type == AV_PICTURE_TYPE_B ? X265_TYPE_B :
X265_TYPE_AUTO;
+
+ret = libx265_encode_set_roi(ctx, pic, );
+if (ret < 0)
+return ret;
}
ret = ctx->api->encoder_encode(ctx->encoder, , ,
pic ? : NULL, _out);
+
+if (x265pic.quantOffsets)
+av_freep();
+
if (ret < 0)
return AVERROR_EXTERNAL;
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/libx265: add support for ROI-based encoding
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Derek Buitenhuis
> Sent: Friday, January 18, 2019 10:42 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libx265: add support for ROI-
> based encoding
>
> On 18/01/2019 15:53, Guo, Yejun wrote:
>
> [...]
>
> > +static av_cold int libx265_encode_set_roi(libx265Context *ctx, const
> > +AVFrame *frame, x265_picture* pic) {
> > +// From x265.h:
> > +/* An array of quantizer offsets to be applied to this image during
> encoding.
> > +* These are added on top of the decisions made by rateControl.
> > +* Adaptive quantization must be enabled to use this feature. These
> quantizer
> > +* offsets should be given for each 16x16 block (8x8 block, when qg-size
> is 8).
> > +* Behavior if quant offsets differ between encoding passes is
> > undefined.
> > +*/
>
> I don't think you need to copy the API documentation from x265.
ok, I'll remove it.
>
> > +// 25 is a number that I think it is a possible proper
> > scale value.
> > +qoffset = qoffset * 25;
>
> Is this based off anything?
the qp range of x265 is 0 to 51, I just choose around half of the range to be
25, so the final qoffset range will be [-25, 25].
I'll add into the note. Comments are welcome if it does not make sense.
(I'll also update notes for libx264, after this patch is done).
>
> > +
> > +if (x265pic.quantOffsets) {
> > +av_free(x265pic.quantOffsets);
> > +x265pic.quantOffsets = NULL;
> > +}
>
> av_freep();
thanks, will change to it.
>
> - Derek
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the documentation wording
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Carl Eugen Hoyos
> Sent: Monday, January 21, 2019 10:13 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the
> documentation wording
>
> 2019-01-21 3:06 GMT+01:00, Guo, Yejun :
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> >> Of Michael Niedermayer
> >> Sent: Sunday, January 20, 2019 6:39 AM
> >> To: FFmpeg development discussions and patches >> de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the
> >> documentation wording
> >>
> >> On Fri, Jan 18, 2019 at 12:38:20PM +0100, Carl Eugen Hoyos wrote:
> >> > Hi!
> >> >
> >> > Attached patch tries to improve the ROI documentation wording, C
> >> > structs cannot return errors.
> >> >
> >> > Please comment, Carl Eugen
> >>
> >> > frame.h |4 ++--
> >> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >> > 888a2470d43113b08b0ef3ddd03bda528f873ccc
> >> > 0001-lavu-frame-Try-to-improve-the-documentation-wording.patch
> >> > From 4abd545e7ab463c97bf816b270544eee25c4755b Mon Sep 17
> 00:00:00
> >> 2001
> >> > From: Carl Eugen Hoyos
> >> > Date: Fri, 18 Jan 2019 12:35:44 +0100
> >> > Subject: [PATCH] lavu/frame: Try to improve the documentation
> wording.
> >> >
> >> > C structs cannot return errors.
> >> > ---
> >> > libavutil/frame.h |4 ++--
> >> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/libavutil/frame.h b/libavutil/frame.h index
> >> > 8aa3e88..1990320 100644
> >> > --- a/libavutil/frame.h
> >> > +++ b/libavutil/frame.h
> >> > @@ -218,8 +218,8 @@ typedef struct AVFrameSideData {
> >> > * if the codec requires an alignment.
> >> > * If the regions overlap, the last value in the list will be used.
> >> > *
> >> > - * qoffset is quant offset, and base rule here:
> >> > - * returns EINVAL if AVRational.den is zero.
> >> > + * qoffset is quantization offset:
> >> > + * Encoders will return EINVAL if qoffset.den is zero.
> >> > * the value (num/den) range is [-1.0, 1.0], clamp to +-1.0 if out
> >> > of range.
> >> > * 0 means no picture quality change,
> >> > * negative offset asks for better quality (and the best with
> >> > value -1.0),
> >>
> >> The field specific documentation should be added to each of the
> >> fields using correct doxygen syntax.
> >>
> >> About EINVAL, i would tend to document what is valid and what is not
> >> instead of documenting what some code will do with invalid values.
> >> The user has no reason to ever use invalid values so that information
> >> should have no value. And just has some chance to be incorrect now or
> >> later
> >>
> >> Also the wording of the unchanged parts sound funky, some native
> >> english speaker should check and reword more of this i think.
> >
> > Sorry for the wording
>
> Didn't I ask you to improve it?
I got your point only after seeing this patch. And your patch is better.
>
> Please send a patch, Carl Eugen
For the unchanged funky parts, I did try hard to make the description clear,
but can hardly improve the wording as a native speaker in a short time.
Anyway, before see an improvement patch, I'll keep it in my mind and improve it
once I get new ideas for the rewording.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] lavc/qsvenc: set BRCParamMultiplier to aviod BRC overflow
> From: Li, Zhong
> Sent: Monday, January 14, 2019 3:12 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Li, Zhong
> Subject: [PATCH v2] lavc/qsvenc: set BRCParamMultiplier to aviod BRC
> overflow
>
> Fix ticket #7663
>
> Reviewed-by Carl Eugen Hoyos Reviewed-by
> Hendrik Leppkes
> Signed-off-by: Zhong Li
> ---
> libavcodec/qsvenc.c | 41 +++--
> 1 file changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> e3b5a72..ba9bcf1 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -158,8 +158,8 @@ static void dump_video_param(AVCodecContext
> *avctx, QSVEncContext *q, #endif
> ) {
> av_log(avctx, AV_LOG_VERBOSE,
> - "BufferSizeInKB: %"PRIu16"; InitialDelayInKB: %"PRIu16";
> TargetKbps: %"PRIu16"; MaxKbps: %"PRIu16"\n",
> - info->BufferSizeInKB, info->InitialDelayInKB,
> info->TargetKbps, info->MaxKbps);
> + "BufferSizeInKB: %"PRIu16"; InitialDelayInKB: %"PRIu16";
> TargetKbps: %"PRIu16"; MaxKbps: %"PRIu16";
> BRCParamMultiplier: %"PRIu16"\n",
> + info->BufferSizeInKB, info->InitialDelayInKB,
> + info->TargetKbps, info->MaxKbps, info->BRCParamMultiplier);
> } else if (info->RateControlMethod == MFX_RATECONTROL_CQP) {
> av_log(avctx, AV_LOG_VERBOSE, "QPI: %"PRIu16";
> QPP: %"PRIu16"; QPB: %"PRIu16"\n",
> info->QPI, info->QPP, info->QPB); @@ -167,8 +167,8
> @@ static void dump_video_param(AVCodecContext *avctx,
> QSVEncContext *q, #if QSV_HAVE_AVBR
> else if (info->RateControlMethod == MFX_RATECONTROL_AVBR) {
> av_log(avctx, AV_LOG_VERBOSE,
> - "TargetKbps: %"PRIu16"; Accuracy: %"PRIu16";
> Convergence: %"PRIu16"\n",
> - info->TargetKbps, info->Accuracy, info->Convergence);
> + "TargetKbps: %"PRIu16"; Accuracy: %"PRIu16";
> Convergence: %"PRIu16"; BRCParamMultiplier: %"PRIu16"\n",
> + info->TargetKbps, info->Accuracy, info->Convergence,
> + info->BRCParamMultiplier);
> }
> #endif
> #if QSV_HAVE_LA
> @@ -178,8 +178,8 @@ static void dump_video_param(AVCodecContext
> *avctx, QSVEncContext *q, #endif
> ) {
> av_log(avctx, AV_LOG_VERBOSE,
> - "TargetKbps: %"PRIu16";
> LookAheadDepth: %"PRIu16"\n",
> - info->TargetKbps, co2->LookAheadDepth);
> + "TargetKbps: %"PRIu16"; LookAheadDepth: %"PRIu16";
> BRCParamMultiplier: %"PRIu16"\n",
> + info->TargetKbps, co2->LookAheadDepth,
> + info->BRCParamMultiplier);
> }
> #endif
> #if QSV_HAVE_ICQ
> @@ -451,6 +451,8 @@ static int init_video_param(AVCodecContext *avctx,
> QSVEncContext *q)
> avctx->sw_pix_fmt :
> avctx->pix_fmt;
> const AVPixFmtDescriptor *desc;
> float quant;
> +int target_bitrate_kbps, max_bitrate_kbps, brc_param_multiplier;
> +int buffer_size_in_kilobytes, initial_delay_in_kilobytes;
> int ret;
> mfxVersion ver;
>
> @@ -552,16 +554,25 @@ static int init_video_param(AVCodecContext
> *avctx, QSVEncContext *q)
> if (ret < 0)
> return ret;
>
> +//libmfx BRC parameters are 16 bits thus maybe overflow, then
> BRCParamMultiplier is needed
> +buffer_size_in_kilobytes = avctx->rc_buffer_size / 8000;
> +initial_delay_in_kilobytes = avctx->rc_initial_buffer_occupancy / 1000;
> +target_bitrate_kbps= avctx->bit_rate / 1000;
> +max_bitrate_kbps = avctx->rc_max_rate / 1000;
> +brc_param_multiplier = (FFMAX(FFMAX3(target_bitrate_kbps,
> max_bitrate_kbps, buffer_size_in_kilobytes),
> + initial_delay_in_kilobytes) +
> + 0x1) / 0x1;
> +
> switch (q->param.mfx.RateControlMethod) {
> case MFX_RATECONTROL_CBR:
> case MFX_RATECONTROL_VBR:
> #if QSV_HAVE_VCM
> case MFX_RATECONTROL_VCM:
> #endif
> -q->param.mfx.BufferSizeInKB = avctx->rc_buffer_size / 8000;
> -q->param.mfx.InitialDelayInKB =
> avctx->rc_initial_buffer_occupancy / 1000;
> -q->param.mfx.TargetKbps = avctx->bit_rate / 1000;
> -q->param.mfx.MaxKbps = avctx->rc_max_rate / 1000;
> +q->param.mfx.BufferSizeInKB = buffer_size_in_kilobytes /
> brc_param_multiplier;
> +q->param.mfx.InitialDelayInKB = initial_delay_in_kilobytes /
> brc_param_multiplier;
> +q->param.mfx.TargetKbps = target_bitrate_kbps /
> brc_param_multiplier;
> +q->param.mfx.MaxKbps = max_bitrate_kbps /
> brc_param_multiplier;
> +q->param.mfx.BRCParamMultiplier = brc_param_multiplier;
> break;
> case MFX_RATECONTROL_CQP:
> quant = avctx->global_quality / FF_QP2LAMBDA; @@ -573,15
> +584,17 @@ static int init_video_param(AVCodecContext *avctx,
> QSVEncContext *q)
> break;
> #if QSV_HAVE_AVBR
> case MFX_RATECONTROL_AVBR:
> -q->param.mfx.TargetKbps =
Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the documentation wording
2019-01-21 3:06 GMT+01:00, Guo, Yejun :
>
>
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>> Of Michael Niedermayer
>> Sent: Sunday, January 20, 2019 6:39 AM
>> To: FFmpeg development discussions and patches > de...@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the
>> documentation wording
>>
>> On Fri, Jan 18, 2019 at 12:38:20PM +0100, Carl Eugen Hoyos wrote:
>> > Hi!
>> >
>> > Attached patch tries to improve the ROI documentation wording, C
>> > structs cannot return errors.
>> >
>> > Please comment, Carl Eugen
>>
>> > frame.h |4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> > 888a2470d43113b08b0ef3ddd03bda528f873ccc
>> > 0001-lavu-frame-Try-to-improve-the-documentation-wording.patch
>> > From 4abd545e7ab463c97bf816b270544eee25c4755b Mon Sep 17 00:00:00
>> 2001
>> > From: Carl Eugen Hoyos
>> > Date: Fri, 18 Jan 2019 12:35:44 +0100
>> > Subject: [PATCH] lavu/frame: Try to improve the documentation wording.
>> >
>> > C structs cannot return errors.
>> > ---
>> > libavutil/frame.h |4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/libavutil/frame.h b/libavutil/frame.h index
>> > 8aa3e88..1990320 100644
>> > --- a/libavutil/frame.h
>> > +++ b/libavutil/frame.h
>> > @@ -218,8 +218,8 @@ typedef struct AVFrameSideData {
>> > * if the codec requires an alignment.
>> > * If the regions overlap, the last value in the list will be used.
>> > *
>> > - * qoffset is quant offset, and base rule here:
>> > - * returns EINVAL if AVRational.den is zero.
>> > + * qoffset is quantization offset:
>> > + * Encoders will return EINVAL if qoffset.den is zero.
>> > * the value (num/den) range is [-1.0, 1.0], clamp to +-1.0 if out of
>> > range.
>> > * 0 means no picture quality change,
>> > * negative offset asks for better quality (and the best with value
>> > -1.0),
>>
>> The field specific documentation should be added to each of the fields
>> using
>> correct doxygen syntax.
>>
>> About EINVAL, i would tend to document what is valid and what is not
>> instead of documenting what some code will do with invalid values.
>> The user has no reason to ever use invalid values so that information
>> should
>> have no value. And just has some chance to be incorrect now or later
>>
>> Also the wording of the unchanged parts sound funky, some native english
>> speaker should check and reword more of this i think.
>
> Sorry for the wording
Didn't I ask you to improve it?
Please send a patch, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the documentation wording
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Sunday, January 20, 2019 6:39 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the
> documentation wording
>
> On Fri, Jan 18, 2019 at 12:38:20PM +0100, Carl Eugen Hoyos wrote:
> > Hi!
> >
> > Attached patch tries to improve the ROI documentation wording, C
> > structs cannot return errors.
> >
> > Please comment, Carl Eugen
>
> > frame.h |4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 888a2470d43113b08b0ef3ddd03bda528f873ccc
> > 0001-lavu-frame-Try-to-improve-the-documentation-wording.patch
> > From 4abd545e7ab463c97bf816b270544eee25c4755b Mon Sep 17 00:00:00
> 2001
> > From: Carl Eugen Hoyos
> > Date: Fri, 18 Jan 2019 12:35:44 +0100
> > Subject: [PATCH] lavu/frame: Try to improve the documentation wording.
> >
> > C structs cannot return errors.
> > ---
> > libavutil/frame.h |4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavutil/frame.h b/libavutil/frame.h index
> > 8aa3e88..1990320 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -218,8 +218,8 @@ typedef struct AVFrameSideData {
> > * if the codec requires an alignment.
> > * If the regions overlap, the last value in the list will be used.
> > *
> > - * qoffset is quant offset, and base rule here:
> > - * returns EINVAL if AVRational.den is zero.
> > + * qoffset is quantization offset:
> > + * Encoders will return EINVAL if qoffset.den is zero.
> > * the value (num/den) range is [-1.0, 1.0], clamp to +-1.0 if out of
> > range.
> > * 0 means no picture quality change,
> > * negative offset asks for better quality (and the best with value
> > -1.0),
>
> The field specific documentation should be added to each of the fields using
> correct doxygen syntax.
>
> About EINVAL, i would tend to document what is valid and what is not
> instead of documenting what some code will do with invalid values.
> The user has no reason to ever use invalid values so that information should
> have no value. And just has some chance to be incorrect now or later
>
> Also the wording of the unchanged parts sound funky, some native english
> speaker should check and reword more of this i think.
Sorry for the wording, hope it would be better.
For the struct field doxygen syntax, I got the comment to be 'above the
typedef', I probably mis-understood it.
Will study doxygen syntax and be more careful about it in later patches. And
don't plan to change this one since rewording is needed.
>
> thanks
>
> [...]
>
>
> --
> Michael GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If you drop bombs on a foreign country and kill a hundred thousand innocent
> people, expect your government to call the consequence "unprovoked
> inhuman terrorist attacks" and use it to justify dropping more bombs and
> killing more people. The technology changed, the idea is old.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/huffyuvdec: Check slice_offset/size
Fixes: out of array access
Fixes:
12447/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HYMT_fuzzer-5201623956062208
Fixes:
12458/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HYMT_fuzzer-5705567736168448
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/huffyuvdec.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c
index 8226c07743..27f650d7bf 100644
--- a/libavcodec/huffyuvdec.c
+++ b/libavcodec/huffyuvdec.c
@@ -1268,6 +1268,11 @@ static int decode_frame(AVCodecContext *avctx, void
*data, int *got_frame,
if (nb_slices > 1) {
slice_offset = AV_RL32(avpkt->data + slices_info_offset + slice *
8);
slice_size = AV_RL32(avpkt->data + slices_info_offset + slice * 8
+ 4);
+
+if (slice_offset < 0 || slice_size <= 0 || (slice_offset&3) ||
+slice_offset + (int64_t)slice_size > buf_size)
+return AVERROR_INVALIDDATA;
+
y_offset = height - (slice + 1) * slice_height;
s->bdsp.bswap_buf((uint32_t *)s->bitstream_buffer,
(const uint32_t *)(buf + slice_offset),
slice_size / 4);
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] avutil/mem: Optimize fill32() by unrolling and using 64bit
2019-01-20 23:37 GMT+01:00, Michael Niedermayer :
> On Sun, Jan 20, 2019 at 10:33:26PM +0100, Carl Eugen Hoyos wrote:
>> 2019-01-20 22:22 GMT+01:00, Michael Niedermayer :
>> > ffmpeg | branch: master | Michael Niedermayer |
>> > Thu
>> > Jan 17 22:35:10 2019 +0100| [12b1338be376a3e5fb606d9fe41b58dc4a9e62c7]
>> > |
>> > committer: Michael Niedermayer
>> >
>> > avutil/mem: Optimize fill32() by unrolling and using 64bit
>> >
>> > Reviewed-by: Marton Balint
>> > Signed-off-by: Michael Niedermayer
>> >
>> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=12b1338be376a3e5fb606d9fe41b58dc4a9e62c7
>> > ---
>> >
>> > libavutil/mem.c | 12
>> > 1 file changed, 12 insertions(+)
>> >
>> > diff --git a/libavutil/mem.c b/libavutil/mem.c
>> > index 6149755a6b..88fe09b179 100644
>> > --- a/libavutil/mem.c
>> > +++ b/libavutil/mem.c
>> > @@ -399,6 +399,18 @@ static void fill32(uint8_t *dst, int len)
>> > {
>> > uint32_t v = AV_RN32(dst - 4);
>> >
>> > +#if HAVE_FAST_64BIT
>>
>> I suspect this should be !X86_32
>
>>
>> > +uint64_t v2= v + ((uint64_t)v<<32);
>> > +while (len >= 32) {
>> > +AV_WN64(dst , v2);
>> > +AV_WN64(dst+ 8, v2);
>> > +AV_WN64(dst+16, v2);
>> > +AV_WN64(dst+24, v2);
>> > +dst += 32;
>> > +len -= 32;
>> > +}
>>
>> How can I test the performance of this function?
>
> with the testcase from the fuzzer (it should be substantially
> faster in this case with teh next commit)
> it should also be possible to test it with some fate tests
> as this is used by some.
I cannot measure any speed difference for the (lengthened)
nuv and cscd fate samples with your patch, so I don't think
this questions warrants further investigation.
Thank you for the abort() suggestion, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] avutil/mem: Optimize fill32() by unrolling and using 64bit
2019-01-20 23:04 GMT+01:00, Hendrik Leppkes :
> On Sun, Jan 20, 2019 at 10:38 PM Carl Eugen Hoyos
> wrote:
>>
>> 2019-01-20 22:22 GMT+01:00, Michael Niedermayer :
>> > ffmpeg | branch: master | Michael Niedermayer |
>> > Thu
>> > Jan 17 22:35:10 2019 +0100| [12b1338be376a3e5fb606d9fe41b58dc4a9e62c7] |
>> > committer: Michael Niedermayer
>> >
>> > avutil/mem: Optimize fill32() by unrolling and using 64bit
>> >
>> > Reviewed-by: Marton Balint
>> > Signed-off-by: Michael Niedermayer
>> >
>> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=12b1338be376a3e5fb606d9fe41b58dc4a9e62c7
>> > ---
>> >
>> > libavutil/mem.c | 12
>> > 1 file changed, 12 insertions(+)
>> >
>> > diff --git a/libavutil/mem.c b/libavutil/mem.c
>> > index 6149755a6b..88fe09b179 100644
>> > --- a/libavutil/mem.c
>> > +++ b/libavutil/mem.c
>> > @@ -399,6 +399,18 @@ static void fill32(uint8_t *dst, int len)
>> > {
>> > uint32_t v = AV_RN32(dst - 4);
>> >
>> > +#if HAVE_FAST_64BIT
>>
>> I suspect this should be !X86_32
>
> fast_64bit is set on any native 64-bit platform.
I know, this is the reason for my question.
> If you can prove that its faster on some 32-bit
> platforms as well, numbers shall be required.
Really? Well, this was the reason for my question
above.
Note that last time it was claimed that "all 32-bit
platforms are slower", it turned out to be wrong
(or at least unreproducible).
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] avutil/mem: Optimize fill32() by unrolling and using 64bit
On Sun, Jan 20, 2019 at 10:33:26PM +0100, Carl Eugen Hoyos wrote:
> 2019-01-20 22:22 GMT+01:00, Michael Niedermayer :
> > ffmpeg | branch: master | Michael Niedermayer | Thu
> > Jan 17 22:35:10 2019 +0100| [12b1338be376a3e5fb606d9fe41b58dc4a9e62c7] |
> > committer: Michael Niedermayer
> >
> > avutil/mem: Optimize fill32() by unrolling and using 64bit
> >
> > Reviewed-by: Marton Balint
> > Signed-off-by: Michael Niedermayer
> >
> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=12b1338be376a3e5fb606d9fe41b58dc4a9e62c7
> > ---
> >
> > libavutil/mem.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/libavutil/mem.c b/libavutil/mem.c
> > index 6149755a6b..88fe09b179 100644
> > --- a/libavutil/mem.c
> > +++ b/libavutil/mem.c
> > @@ -399,6 +399,18 @@ static void fill32(uint8_t *dst, int len)
> > {
> > uint32_t v = AV_RN32(dst - 4);
> >
> > +#if HAVE_FAST_64BIT
>
> I suspect this should be !X86_32
>
> > +uint64_t v2= v + ((uint64_t)v<<32);
> > +while (len >= 32) {
> > +AV_WN64(dst , v2);
> > +AV_WN64(dst+ 8, v2);
> > +AV_WN64(dst+16, v2);
> > +AV_WN64(dst+24, v2);
> > +dst += 32;
> > +len -= 32;
> > +}
>
> How can I test the performance of this function?
with the testcase from the fuzzer (it should be substantially
faster in this case with teh next commit)
it should also be possible to test it with some fate tests
as this is used by some.
You can add a abort() and run fate to see which tests use it
[...]
--
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
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] avutil/mem: Optimize fill32() by unrolling and using 64bit
On Sun, Jan 20, 2019 at 10:38 PM Carl Eugen Hoyos wrote:
>
> 2019-01-20 22:22 GMT+01:00, Michael Niedermayer :
> > ffmpeg | branch: master | Michael Niedermayer | Thu
> > Jan 17 22:35:10 2019 +0100| [12b1338be376a3e5fb606d9fe41b58dc4a9e62c7] |
> > committer: Michael Niedermayer
> >
> > avutil/mem: Optimize fill32() by unrolling and using 64bit
> >
> > Reviewed-by: Marton Balint
> > Signed-off-by: Michael Niedermayer
> >
> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=12b1338be376a3e5fb606d9fe41b58dc4a9e62c7
> > ---
> >
> > libavutil/mem.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/libavutil/mem.c b/libavutil/mem.c
> > index 6149755a6b..88fe09b179 100644
> > --- a/libavutil/mem.c
> > +++ b/libavutil/mem.c
> > @@ -399,6 +399,18 @@ static void fill32(uint8_t *dst, int len)
> > {
> > uint32_t v = AV_RN32(dst - 4);
> >
> > +#if HAVE_FAST_64BIT
>
> I suspect this should be !X86_32
>
fast_64bit is set on any native 64-bit platform. If you can prove that
its faster on some 32-bit platforms as well, numbers shall be
required.
- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [FFmpeg-cvslog] avutil/mem: Optimize fill32() by unrolling and using 64bit
2019-01-20 22:22 GMT+01:00, Michael Niedermayer :
> ffmpeg | branch: master | Michael Niedermayer | Thu
> Jan 17 22:35:10 2019 +0100| [12b1338be376a3e5fb606d9fe41b58dc4a9e62c7] |
> committer: Michael Niedermayer
>
> avutil/mem: Optimize fill32() by unrolling and using 64bit
>
> Reviewed-by: Marton Balint
> Signed-off-by: Michael Niedermayer
>
>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=12b1338be376a3e5fb606d9fe41b58dc4a9e62c7
> ---
>
> libavutil/mem.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index 6149755a6b..88fe09b179 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -399,6 +399,18 @@ static void fill32(uint8_t *dst, int len)
> {
> uint32_t v = AV_RN32(dst - 4);
>
> +#if HAVE_FAST_64BIT
I suspect this should be !X86_32
> +uint64_t v2= v + ((uint64_t)v<<32);
> +while (len >= 32) {
> +AV_WN64(dst , v2);
> +AV_WN64(dst+ 8, v2);
> +AV_WN64(dst+16, v2);
> +AV_WN64(dst+24, v2);
> +dst += 32;
> +len -= 32;
> +}
How can I test the performance of this function?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/motion_est: remove duplicate function
On Sun, Jan 20, 2019 at 07:35:18PM +0100, Marton Balint wrote: > > > On Sun, 20 Jan 2019, Michael Niedermayer wrote: > > >On Sat, Jan 19, 2019 at 11:48:39PM +0100, Marton Balint wrote: > >>Signed-off-by: Marton Balint > >>--- > >> libavcodec/motion_est.c | 4 +-- > >> libavcodec/motion_est_template.c | 62 > >> +--- > >> 2 files changed, 3 insertions(+), 63 deletions(-) > > > >please check if the compiler optimizes the size constant out after this > >change > > The compiler inlines the function before and after the change as well. I > can't see notable changes in the disassembly of interlaced_search and > h263_mv4_search. Is this enough, or something else should be checked? I am > not sure how... If the inlined code is used with only one size value then its probably ok. you could also put some marker with asm() in the code where size is used and then look at the .s file generated if the size is optimized out or still read as a variable thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "Nothing to hide" only works if the folks in power share the values of you and everyone you know entirely and always will -- Tom Scott signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] avcodec/tiff: Check for 12bit gray fax
On Sat, Jan 12, 2019 at 11:28:00PM +0100, Michael Niedermayer wrote: > Fixes: Assertion failure > Fixes: > 11898/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-5759794191794176 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/tiff.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply patchset [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many that live deserve death. And some that die deserve life. Can you give it to them? Then do not be too eager to deal out death in judgement. For even the very wise cannot see all ends. -- Gandalf signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()
On Sat, Jan 19, 2019 at 12:28:25AM +0100, Marton Balint wrote:
>
>
> On Thu, 17 Jan 2019, Michael Niedermayer wrote:
>
> >On Wed, Jan 16, 2019 at 08:00:22PM +0100, Marton Balint wrote:
> >>
> >>
> >>On Tue, 15 Jan 2019, Michael Niedermayer wrote:
> >>
> >>>On Sun, Dec 30, 2018 at 07:15:49PM +0100, Marton Balint wrote:
>
>
> On Fri, 28 Dec 2018, Michael Niedermayer wrote:
>
> >On Wed, Dec 26, 2018 at 10:16:47PM +0100, Marton Balint wrote:
> >>
> >>
> >>On Wed, 26 Dec 2018, Paul B Mahol wrote:
> >>
> >>>On 12/26/18, Michael Niedermayer wrote:
> On Wed, Dec 26, 2018 at 04:32:17PM +0100, Paul B Mahol wrote:
> >On 12/25/18, Michael Niedermayer wrote:
> >>Fixes: Timeout
> >>Fixes:
> >>11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> >>Before: Executed
> >>clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> >>in 11294 ms
> >>After : Executed
> >>clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> >>in 4249 ms
> >>
> >>Found-by: continuous fuzzing process
> >>https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>Signed-off-by: Michael Niedermayer
> >>---
> >>libavutil/imgutils.c | 6 ++
> >>1 file changed, 6 insertions(+)
> >>
> >>diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> >>index 4938a7ef67..cc38f1e878 100644
> >>--- a/libavutil/imgutils.c
> >>+++ b/libavutil/imgutils.c
> >>@@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t
> >>dst_size,
> >>uint8_t *clear,
> >> }
> >> } else if (clear_size == 4) {
> >> uint32_t val = AV_RN32(clear);
> >>+uint64_t val8 = val * 0x10001ULL;
> >>+for (; dst_size >= 32; dst_size -= 32) {
> >>+AV_WN64(dst , val8); AV_WN64(dst+ 8, val8);
> >>+AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
> >>+dst += 32;
> >>+}
> >> for (; dst_size >= 4; dst_size -= 4) {
> >> AV_WN32(dst, val);
> >> dst += 4;
> >>--
> >>2.20.1
> >>
> >
> >NAK, implement special memset function instead.
>
> I can move the added loop into a seperate function, if thats what you
> suggest ?
> >>>
> >>>No, don't do that.
> >>>
> All the code is already in a "special" memset though, this is
> memset_bytes()
>
> >>>
> >>>I guess function is less useful if its static. So any duplicate should
> >>>be avoided in codebase.
> >>
> >>Isn't av_memcpy_backptr does almost exactly what is needed here? That
> >>can
> >>also be optimized further if needed.
> >
> >av_memcpy_backptr() copies data with overlap, its more like a recursive
> >memmove().
>
> So? As far as I see the memset_bytes function in imgutils.c can be
> replaced
> with this:
>
> if (clear_size > dst_size)
> clear_size = dst_size;
> memcpy(dst, clear, clear_size);
> av_memcpy_backptr(dst + clear_size, clear_size, dst_size - clear_size);
>
> I am not against an av_memset_bytes API addition, but I believe it should
> share code with av_memcpy_backptr to avoid duplication.
> >>>
> >>>ive implemented this, it does not seem to be really faster in the testcase
> >>
> >>I guess it is not faster because you have not applied your original
> >>optimalization to fill32 in libavutil/mem.c. Could you compare speed after
> >>optimizing that the same way your original patch did it with imgutils
> >>memset_bytes?
> >
> >sure, that makes it faster:
>
> Thanks, both patches LGTM.
will apply
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Observe your enemies, for they first find out your faults. -- Antisthenes
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/motion_est: remove duplicate function
On Sun, 20 Jan 2019, Michael Niedermayer wrote: On Sat, Jan 19, 2019 at 11:48:39PM +0100, Marton Balint wrote: Signed-off-by: Marton Balint --- libavcodec/motion_est.c | 4 +-- libavcodec/motion_est_template.c | 62 +--- 2 files changed, 3 insertions(+), 63 deletions(-) please check if the compiler optimizes the size constant out after this change The compiler inlines the function before and after the change as well. I can't see notable changes in the disassembly of interlaced_search and h263_mv4_search. Is this enough, or something else should be checked? I am not sure how... Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/4] zmbvenc: don't sum the entropy when blocks are equal
lör 2019-01-19 klockan 22:40 + skrev Matthew Fearnley: > > On Tue, 25 Dec 2018 at 09:35, Tomas Härdin wrote: > > > lör 2018-12-22 klockan 15:32 + skrev Matthew Fearnley: > > > > > > > > > > > > > > > > Note that bw,bh aren't guaranteed to equal ZMBV_BLOCK, so > > > > `histogram[0] == > > > > > bw*bh` would have to be used to guard against those (literal) edge > > > > cases. > > > > > > > > Right, yes. But if we have block sizes other than ZMBV_BLOCK^2 then we > > > > need score tables for those sizes too. > > > > > > I’ve thought about that a bit. I wondered if it would be worth it given: > > > - the extra code, memory and logic needed > > > > If you have a huge amount of DOS captures to optimize then it might be > > worth it, else probably questionable > > > > > - it would only improve the edge blocks > > > > I imagine large blocks would be good for scenes with mostly global > > motion. You cut down on the number of MVs and thus the amount of data > > zlib has to compress, if the block size is a good fit. > > > > > - the existing score table isn’t catastrophically bad for short blocks, > > > > and would still favour blocks with more common pixels. > > > > > > It would be better from a correctness perspective though, and effects on > > > > running time should be negligible. > > > > Good point. There's also no telling whether the current model is > > actually an accurate prediction of how zlib behaves :) > > > > > > Hi. Just to say, I tried setting up additional score_tabs for the > bottom/right partial blocks. Unfortunately, after implementing it and > ironing out the bugs, the new score tables actually caused a slight > increase in file size! > After a little informal investigation with a couple of videos, my findings > were that increasing the divisor - '(i / (double)(ZMBV_BLOCK * ZMBV_BLOCK * > some_factor))' - would generally lead to slight compression improvements. > Given the score table is still "valid" for smaller blocks, and given the > extra complexity of adding the score tables, plus the fact that it may > generally hurt the compression, I'm inclined to leave it with the one score > table. But there may be potential to improve the current compression > method in future, by somehow tuning the divisor for better results > generally. Huh, that's strange. Sounds like something that warrants a comment in the code. I also see we have an answer do Carl's question: you're still experimenting with this :) I think we can hold off on pushing anything until you feel happy with the result /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
