Re: [FFmpeg-devel] [PATCH v2] avformat/hls Implement support for using AVSEEK_FLAG_BACKWARD when seeking
On Sat, Jun 5, 2021 at 10:41 PM Gustav Grusell
wrote:
> Before, seeking in hls streams would always seek to the next keyframe
> after the given timestamp.
> With this fix, if AVSEEK_FLAG_BACKWARD is set, seeking will be to the
> first keyframe of the
> segment containing the given timestamp. This fixes #7485.
>
> Signed-off-by: Gustav Grusell
> ---
> libavformat/hls.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 8fc6924c90..fb8c9b071a 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2197,6 +2197,15 @@ static int hls_read_packet(AVFormatContext *s,
> AVPacket *pkt)
> break;
> }
>
> +/* If AVSEEK_FLAG_BACKWARD set and not
> AVSEEK_FLAG_ANY,
> + * we return the first keyframe encountered */
> +if (pls->seek_flags & AVSEEK_FLAG_BACKWARD &&
> +!(pls->seek_flags & AVSEEK_FLAG_ANY) &&
> +pls->pkt->flags & AV_PKT_FLAG_KEY) {
> +pls->seek_timestamp = AV_NOPTS_VALUE;
> +break;
> +}
> +
> tb = get_timebase(pls);
> ts_diff = av_rescale_rnd(pls->pkt->dts, AV_TIME_BASE,
> tb.den, AV_ROUND_DOWN) -
> --
> 2.25.1
>
>
Any further comments on this?
Btw, it seems the v2 patch got picked up as a separate patch by patchwork.
Is this how it is supposed to work or did I miss something when doing git
send-email ?
cheers,
Gustav
___
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 v2 2/2] lavfi/vf_vpp_qsv: fix the time_base for outlink
Since commit 89ffcd1, the status pts of the output link is set to a value in the input link time base, not in the output link time base when EOF is reached. Usually this pst value is larger than the required one because the output link time base is more greater than the input link time base. When "-vf vpp_qsv,fps" is used, user has to wait a long time for the ending of the pipeline because fps filter output a huge number of frames until the wrong status pts is hit. The issue can be triggered with the command below (use a clip with 1000 frames in this case): $> time ffmpeg -hwaccel qsv -c:v hevc_qsv -i input.h265 -vf "vpp_qsv=w=1920:h=1080,fps=fps=30" -f null - ... [out_0_0 @ 0x564ccd27e020] 1000 buffers queued in out_0_0, something may be wrong. frame=40119596 fps=88080 q=-0.0 Lsize=N/A time=371:28:39.96 bitrate=N/A speed=2.94e+03x video:17238889kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown real9m7.451s user2m34.102s sys 0m39.734s In order to avoid the above issue, the same time base for input and ouput links is used in this patch. Fixes ticket #9286 --- v2: update the commit log libavfilter/vf_vpp_qsv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c index b9ab5c6490..74d1d51e7c 100644 --- a/libavfilter/vf_vpp_qsv.c +++ b/libavfilter/vf_vpp_qsv.c @@ -303,7 +303,7 @@ static int config_output(AVFilterLink *outlink) outlink->w = vpp->out_width; outlink->h = vpp->out_height; outlink->frame_rate = vpp->framerate; -outlink->time_base = av_inv_q(vpp->framerate); +outlink->time_base = inlink->time_base; param.filter_frame = NULL; param.num_ext_buf = 0; -- 2.25.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".
[FFmpeg-devel] [PATCH v2 1/2] lavc/qsvdec: fix pts
The time base used for compressed bitstream and video frame in the SDK
is { 1, 9 }. [1][2]
This can avoid the error message below from the muxer.
$> ffmpeg -hwaccel qsv -c:v hevc_qsv -i input.h265 -f null -
...
[null @ 0x561c24f6f2f0] Application provided invalid, non monotonically
increasing dts to muxer in stream 0: 2 >= 2
[1]https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/mediasdk-man.md#mfxbitstream
[2]https://github.com/Intel-Media-SDK/MediaSDK/blob/master/doc/mediasdk-man.md#mfxframedata
---
libavcodec/qsvdec.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index f543defb18..622750927c 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -46,6 +46,16 @@
#include "qsv.h"
#include "qsv_internal.h"
+static const AVRational mfx_tb = { 1, 9 };
+
+#define PTS_TO_MFX_PTS(pts, pts_tb) ((pts) == AV_NOPTS_VALUE ? \
+MFX_TIMESTAMP_UNKNOWN : pts_tb.num ? \
+av_rescale_q(pts, pts_tb, mfx_tb) : pts)
+
+#define MFX_PTS_TO_PTS(mfx_pts, pts_tb) ((mfx_pts) == MFX_TIMESTAMP_UNKNOWN ? \
+AV_NOPTS_VALUE : pts_tb.num ? \
+av_rescale_q(mfx_pts, mfx_tb, pts_tb) : mfx_pts)
+
typedef struct QSVContext {
// the session used for decoding
mfxSession session;
@@ -308,7 +318,7 @@ static int qsv_decode_header(AVCodecContext *avctx,
QSVContext *q,
bs.Data = avpkt->data;
bs.DataLength = avpkt->size;
bs.MaxLength = bs.DataLength;
-bs.TimeStamp = avpkt->pts;
+bs.TimeStamp = PTS_TO_MFX_PTS(avpkt->pts, avctx->pkt_timebase);
if (avctx->field_order == AV_FIELD_PROGRESSIVE)
bs.DataFlag |= MFX_BITSTREAM_COMPLETE_FRAME;
} else
@@ -456,7 +466,7 @@ static int qsv_decode(AVCodecContext *avctx, QSVContext *q,
bs.Data = avpkt->data;
bs.DataLength = avpkt->size;
bs.MaxLength = bs.DataLength;
-bs.TimeStamp = avpkt->pts;
+bs.TimeStamp = PTS_TO_MFX_PTS(avpkt->pts, avctx->pkt_timebase);
if (avctx->field_order == AV_FIELD_PROGRESSIVE)
bs.DataFlag |= MFX_BITSTREAM_COMPLETE_FRAME;
}
@@ -544,7 +554,7 @@ static int qsv_decode(AVCodecContext *avctx, QSVContext *q,
outsurf = &out_frame->surface;
-frame->pts = outsurf->Data.TimeStamp;
+frame->pts = MFX_PTS_TO_PTS(outsurf->Data.TimeStamp,
avctx->pkt_timebase);
frame->repeat_pict =
outsurf->Info.PicStruct & MFX_PICSTRUCT_FRAME_TRIPLING ? 4 :
@@ -748,6 +758,9 @@ static av_cold int qsv_decode_init(AVCodecContext *avctx)
goto fail;
}
+if (!avctx->pkt_timebase.num)
+av_log(avctx, AV_LOG_WARNING, "Invalid pkt_timebase, passing
timestamps as-is.\n");
+
return 0;
fail:
qsv_decode_close(avctx);
--
2.25.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 1/2] hwcontext_vulkan: dynamically load functions
> > Jun 10, 2021, 12:27 by d...@lynne.ee: > > > Jun 10, 2021, 03:38 by wenbin.c...@intel.com: > > > >>> Jun 8, 2021, 07:38 by wenbin.c...@intel.com: > >>> > >>> >> Apr 29, 2021, 03:52 by d...@lynne.ee: > >>> >> > >>> >> > This patch allows for alternative loader implementations. > >>> >> > > >>> >> > Patch attached. > >>> >> > > >>> >> > >>> >> Forgot to fix a flag, v2 attached. > >>> >> > >>> > > >>> > Hi Lynne: > >>> > I tried the following command: > >>> > "ffmpeg -init_hw_device vulkan=vul:0 -filter_hw_device vul -i > 1080p.264 -vf > >>> "hwupload=extra_hw_frames=16,scale_vulkan=w=1920:h1080" -f null" > >>> > It report a segmentation fault at > >>> > https://github.com/FFmpeg/FFmpeg/blob/282b9f4cba7ca361c43ac9f94031a > >>> 43109df0a4f/libavutil/hwcontext_vulkan.c#L1018 > >>> > The function pointer vk->CreateCommandPool is NULL. > >>> > I build on ffmpeg master. > >>> > > >>> > >>> Weird. What Vulkan drivers and what OS was used? > >>> If CreateCommandPool is NULL then initialization probably > >>> wouldn't even happen. > >>> > >> > >> I use linux ubuntu 20.04 and I get the vulkan driver from: > https://www.lunarg.com/vulkan-sdk/. > >> I see the load_functions() is called three times, but none of them set > "has_dev" argument to 1. > >> I tried to change the code in > https://github.com/FFmpeg/FFmpeg/blob/591b88e6787c4e678237f02a5042 > 1d101abd25c2/libavutil/hwcontext_vulkan.c#L1352 to "load_functions(ctx, 1, > 1)". The problem is unseen. > >> > > > > Thanks for testing, fix pushed. > > > > Out of curiosity, are you planning on contributing to the Vulkan code? No, I am working on vaapi and qsv, but I will submit some patches to vulkan, because I plan to encode vulkan frame using vaapi and qsv encoder with "hwmap" filter. ___ 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 5/5] lavfi/dnn: Fill Task using Common Function
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Shubhanshu Saxena
> Sent: 2021年6月6日 2:08
> To: ffmpeg-devel@ffmpeg.org
> Cc: Shubhanshu Saxena
> Subject: [FFmpeg-devel] [PATCH V2 5/5] lavfi/dnn: Fill Task using Common
> Function
>
> This commit adds a common function for filling the TaskItems
> in all three backends.
>
> Signed-off-by: Shubhanshu Saxena
> ---
> libavfilter/dnn/dnn_backend_common.c | 20
> libavfilter/dnn/dnn_backend_common.h | 15 +++
> libavfilter/dnn/dnn_backend_openvino.c | 23 +++
> 3 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/libavfilter/dnn/dnn_backend_common.c
> b/libavfilter/dnn/dnn_backend_common.c
> index a522ab5650..4d9d3f79b1 100644
> --- a/libavfilter/dnn/dnn_backend_common.c
> +++ b/libavfilter/dnn/dnn_backend_common.c
> @@ -49,3 +49,23 @@ int ff_check_exec_params(void *ctx, DNNBackendType
> backend, DNNFunctionType func
>
> return 0;
> }
> +
> +DNNReturnType ff_dnn_fill_task(TaskItem *task, DNNExecBaseParams
> *exec_params, void *backend_model, int async, int do_ioproc) {
> +if (task == NULL || exec_params == NULL || backend_model == NULL)
> +return DNN_ERROR;
> +if (do_ioproc != 0 && do_ioproc != 1)
> +return DNN_ERROR;
> +if (async != 0 && async != 1)
> +return DNN_ERROR;
> +
> +task->do_ioproc = do_ioproc;
> +task->async = async;
> +task->input_name = exec_params->input_name;
> +task->in_frame = exec_params->in_frame;
> +task->out_frame = exec_params->out_frame;
> +task->model = backend_model;
> +task->nb_output = exec_params->nb_output;
> +task->output_names = exec_params->output_names;
> +
> +return DNN_SUCCESS;
> +}
> diff --git a/libavfilter/dnn/dnn_backend_common.h
> b/libavfilter/dnn/dnn_backend_common.h
> index d962312c16..df59615f40 100644
> --- a/libavfilter/dnn/dnn_backend_common.h
> +++ b/libavfilter/dnn/dnn_backend_common.h
> @@ -48,4 +48,19 @@ typedef struct InferenceItem {
>
> int ff_check_exec_params(void *ctx, DNNBackendType backend,
> DNNFunctionType func_type, DNNExecBaseParams *exec_params);
>
> +/**
> + * Fill the Task for Backend Execution. It should be called after
> + * checking execution parameters using ff_check_exec_params.
> + *
> + * @param task pointer to the allocated task
> + * @param exec_param pointer to execution parameters
> + * @param backend_model void pointer to the backend model
> + * @param async flag for async execution. Must be 0 or 1
> + * @param do_ioproc flag for IO processing. Must be 0 or 1
> + *
> + * @retval DNN_SUCCESS if successful
> + * @retval DNN_ERROR if flags are invalid or any parameter is NULL
> + */
> +DNNReturnType ff_dnn_fill_task(TaskItem *task, DNNExecBaseParams
> *exec_params, void *backend_model, int async, int do_ioproc);
> +
> #endif
> diff --git a/libavfilter/dnn/dnn_backend_openvino.c
> b/libavfilter/dnn/dnn_backend_openvino.c
> index c2487c35be..709a772a4d 100644
> --- a/libavfilter/dnn/dnn_backend_openvino.c
> +++ b/libavfilter/dnn/dnn_backend_openvino.c
> @@ -793,14 +793,9 @@ DNNReturnType ff_dnn_execute_model_ov(const
> DNNModel *model, DNNExecBaseParams *
> }
> }
>
> -task.do_ioproc = 1;
> -task.async = 0;
> -task.input_name = exec_params->input_name;
> -task.in_frame = exec_params->in_frame;
> -task.output_names = &exec_params->output_names[0];
> -task.out_frame = exec_params->out_frame ? exec_params->out_frame :
> exec_params->in_frame;
> -task.nb_output = exec_params->nb_output;
> -task.model = ov_model;
> +if (ff_dnn_fill_task(&task, exec_params, ov_model, 0, 1) !=
> DNN_SUCCESS) {
> +return DNN_ERROR;
> +}
>
> if (extract_inference_from_task(ov_model->model->func_type, &task,
> ov_model->inference_queue, exec_params) != DNN_SUCCESS) {
> av_log(ctx, AV_LOG_ERROR, "unable to extract inference from
> task.\n");
> @@ -841,14 +836,10 @@ DNNReturnType
> ff_dnn_execute_model_async_ov(const DNNModel *model, DNNExecBasePa
> return DNN_ERROR;
> }
>
> -task->do_ioproc = 1;
> -task->async = 1;
> -task->input_name = exec_params->input_name;
> -task->in_frame = exec_params->in_frame;
> -task->output_names = &exec_params->output_names[0];
> -task->out_frame = exec_params->out_frame ?
> exec_params->out_frame : exec_params->in_frame;
> -task->nb_output = exec_params->nb_output;
> -task->model = ov_model;
> +if (ff_dnn_fill_task(task, exec_params, ov_model, 1, 1) != DNN_SUCCESS)
> {
> +return DNN_ERROR;
> +}
> +
> if (ff_queue_push_back(ov_model->task_queue, task) < 0) {
> av_freep(&task);
> av_log(ctx, AV_LOG_ERROR, "unable to push back task_queue.\n");
will push tomorrow if there's no objection, thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscrib
Re: [FFmpeg-devel] [PATCH] avcodec: Pass the HDR10+ metadata to the packet side data in VP9 encoder
Mohammad Izadi:
> HDR10+ metadata is stored in the bit stream for HEVC. The story is different
> for VP9 and cannot store the metadata in the bit stream. HDR10+ should be
> passed to packet side data an stored in the container (mkv) for VP9.
>
> This CL is taking HDR10+ from AVFrame side data in libvpxenc and is passing
> it to the AVPacket side data.
> ---
> doc/APIchanges | 2 +
> libavcodec/avpacket.c | 1 +
> libavcodec/decode.c| 1 +
> libavcodec/libvpxenc.c | 99 ++
> libavcodec/packet.h| 8
> libavcodec/version.h | 4 +-
> 6 files changed, 113 insertions(+), 2 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 55171311ed..bba5b06c6a 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,8 @@ libavutil: 2021-04-27
>
>
> API changes, most recent first:
> +2021-05-25 - 8c88a66d3c - lavc 59.2.100 - packet.h
> + Add AV_PKT_DATA_DYNAMIC_HDR10_PLUS
>
> 2021-xx-xx - xx - lavc 59.1.100 - avcodec.h codec.h
>Move av_get_profile_name() from avcodec.h to codec.h.
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 7383d12d3e..800bee3489 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -289,6 +289,7 @@ const char *av_packet_side_data_name(enum
> AVPacketSideDataType type)
> case AV_PKT_DATA_ICC_PROFILE:return "ICC Profile";
> case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration
> record";
> case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE ST 12-1:2014
> timecode";
> +case AV_PKT_DATA_DYNAMIC_HDR10_PLUS: return "HDR10+ Dynamic
> Metadata (SMPTE 2094-40)";
> }
> return NULL;
> }
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 75bc7ad98e..40f688e40c 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1488,6 +1488,7 @@ int ff_decode_frame_props(AVCodecContext *avctx,
> AVFrame *frame)
> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC },
> { AV_PKT_DATA_ICC_PROFILE,AV_FRAME_DATA_ICC_PROFILE
> },
> { AV_PKT_DATA_S12M_TIMECODE,
> AV_FRAME_DATA_S12M_TIMECODE },
> +{ AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
> AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
> };
>
> if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >=
> sizeof(*pkt))
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index 66bad444d0..e2e6c60b68 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -64,6 +64,11 @@ struct FrameListData {
> struct FrameListData *next;
> };
>
> +typedef struct FrameHDR10Plus {
> +int64_t pts;
> +AVBufferRef *hdr10_plus;
> +} FrameHDR10Plus;
> +
> typedef struct VPxEncoderContext {
> AVClass *class;
> struct vpx_codec_ctx encoder;
> @@ -121,6 +126,8 @@ typedef struct VPxEncoderContext {
> int tune_content;
> int corpus_complexity;
> int tpl_model;
> +int discard_hdr10_plus;
> +AVFifoBuffer *hdr10_plus_fifo;
> /**
> * If the driver does not support ROI then warn the first time we
> * encounter a frame with ROI side data.
> @@ -316,6 +323,55 @@ static av_cold void free_frame_list(struct FrameListData
> *list)
> }
> }
>
> +static av_cold int add_hdr10_plus(AVFifoBuffer *fifo, struct FrameHDR10Plus
> *data)
> +{
> +int err = av_fifo_grow(fifo, sizeof(*data));
> +if (err < 0)
> +return err;
> +av_fifo_generic_write(fifo, data, sizeof(*data), NULL);
> +return 0;
> +}
> +
> +static av_cold void free_hdr10_plus(struct FrameHDR10Plus *p)
> +{
> +if (!p)
> +return;
> +av_buffer_unref(&p->hdr10_plus);
> +av_free(p);
> +}
> +
> +static av_cold void free_hdr10_plus_fifo(AVFifoBuffer **fifo)
> +{
> +FrameHDR10Plus *frame_hdr10_plus = NULL;
> +while (av_fifo_size(*fifo) >= sizeof(FrameHDR10Plus)) {
> +av_fifo_generic_read(*fifo, frame_hdr10_plus,
> sizeof(FrameHDR10Plus), NULL);
> +free_hdr10_plus(frame_hdr10_plus);
You apparently overlooked/ignored my email here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281220.html
> +}
> +av_fifo_freep(fifo);
> +}
> +
> +static int copy_hdr10_plus_to_pkt(AVFifoBuffer *fifo, AVPacket *pkt)
> +{
> +FrameHDR10Plus frame_hdr10_plus;
> +uint8_t *data;
> +if (!pkt)
> +return 0;
> +if (av_fifo_size(fifo) < sizeof(frame_hdr10_plus))
> +return 0;
> +
> +av_fifo_generic_read(fifo, &frame_hdr10_plus, sizeof(frame_hdr10_plus),
> NULL);
> +if (!frame_hdr10_plus.hdr10_plus || frame_hdr10_plus.pts != pkt->pts)
> +return 0;
> +
> +data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
> frame_hdr10_plus.hdr10_plus->size);
> +if (!data)
> +return AVERROR(ENOMEM);
> +
> +memcpy(data, frame_hdr10_plus.hdr10_plus->data,
> frame_hdr10_plus.hdr10_plus->size);
Leak (and
Re: [FFmpeg-devel] [PATCH v3] lavc/aomdec: Allow RGB decoding
чт, 10 июн. 2021 г., 20:33 James Zern : > On Thu, Jun 10, 2021 at 9:26 AM Валерий Заподовников > wrote: > > > > >This doesn't match the check in libdav1d.c. > > > > I do not really care what libdav1d.c does, since it uses internal to AV1 > spec DAV1D_MC_IDENTITY (and frame->colorspace). This should have been a no > go into FFmpeg, but it was not somehow. FFmpeg can handle all of this in by > itself, there is no need to use examples from AV1 spec, which are wrong by > omission: see for yourself. > https://github.com/FFmpeg/FFmpeg/blob/c8778606b3811da6bd58ca6b73d2446bd430013e/libavcodec/libdav1d.c#L299 > > > > It would be better to put this on the list for others to be able to > comment on. > > > This is sRGB definition from AV1, not R'G'B' (or even RGB in case of > linear transfer) and thus is total garbage. What if the file is tagged with > DCI-P3 transfer and/or primaries? I am not going to fix libdav1d.c, since > as I said, it uses internal names and internal deliberations which are > unacceptable. > > > > Again. AV1 spec is really quite hillarious in that part. For example it > does mention XYZ, but does not check for it anywhere, try Ctrl-F here: > https://aomediacodec.github.io/av1-spec/av1-spec.pdf > > > > Thus I did implement filtering this out too, since FFmpeg does support > XYZ pixel format (and maybe somebody one day will implement it too). > > > > Listen, my patch is correct. AV1 spec is also telling the same, as it > shows all the possible primaries and transfers. But it only looks into sRGB > scenario, since your people (Google, etc) were not capable to implement > normal icc color management, but FFmpeg with mpv can do it and thus no need > to concentrate on sRGB like your people did. I mean you can fix AV1 spec if > you want, but IMHO it is correct already, this is color spaces info 101 and > AV1 is depending on ITU-R H.273. > Okay, I will put it into the mailing list. P.S. Dunno what that guy was thinking. I am not going to inline my comments any more than I want to. I mean sure I do not top post, but just since this is just waste of Internet traffic, and so is inlining in most cases (but right here it is COOL and very needed). > ___ 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] avformat/rpl: The associative law doesnt hold in C
Add () to avoid undefined behavior Fixes: signed integer overflow: 9223372036854775790 + 57 cannot be represented in type 'long' Fixes: 34983/clusterfuzz-testcase-minimized-ffmpeg_dem_RPL_fuzzer-5765822923538432 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavformat/rpl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/rpl.c b/libavformat/rpl.c index fac377b68e..296cb2c984 100644 --- a/libavformat/rpl.c +++ b/libavformat/rpl.c @@ -103,7 +103,7 @@ static AVRational read_fps(const char* line, int* error) // Truncate any numerator too large to fit into an int64_t if (num > (INT64_MAX - 9) / 10 || den > INT64_MAX / 10) break; -num = 10 * num + *line - '0'; +num = 10 * num + (*line - '0'); den *= 10; } if (!num) -- 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 v4] avformat/mpegtsenc: enable muxing of ARIB captions
On Mon, Jun 7, 2021 at 8:09 PM Jan Ekström wrote: > > From: zheng qian > > Writes a general ARIB stream identifier descriptor, as well > as a data component descriptor which also includes a > pre-defined additional_arib_caption_info structure. > > Signed-off-by: zheng qian > --- Applied as f38458089f28df73a7badf459117d668ce988ca6 Jan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Removal of Valerii Zapodovnikov
Hi again, some of you might have realized that I've removed Valerii Zapodovnikov from our mailing list after [1]. I did this in my function as a mailing list admin because I think our guidelines for the mailing list are mandatory for everyone to be welcome here. Plain rejection of these guidelines makes you not welcome here by definition. Any bot would have made the same decision. Let me point out that this case differs from the "usual" (sorry, dear community) personally offensive misbehavior against others which is "just" in conflict with parts of the guidelines - and shall be prosecuted by the community and not an admin. That's my thoughts, as admin for this community. I made that decision and I stand by it. Unfortunately, as this identity is reappearing, I think this mail is needed because I have just told him a second time why he got himself banned and will do so without further ado as long as this identity cares to come back. However, I'm serving this community as an admin and I'm not free of error or misjudgement. Therefore, - if another admin feels to overrule my decision, or - if enough trusted members of this community frown upon me in this thread to exceed my personal subjective limit... ... then I will be totally fine to revoke any bans on that identity and release that one into our community. Thanks, Thilo [1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281206.html I think we grant any defendant a statement which I received in a private mail. Attaching the original .eml: " You do understand that I inlined my answer "No" to you? I mean it can be recognised as trolling, maybe... But certainly not the BS that you did here, WHILE I was trying to help you because I thought .patch will not be recognised by patchwork, as it usually is if you do not configure MIME. This is disgusting behaviour coming from you, but this is not the first time and certainly not the last, this is Internet after all, where people like you (admins) think too much of themselfes, I DO EVEN KNOW WHO you are. I cannot be banned though, I am sorry I can just switch to one of my hundreds of mail accounts, hacked and not AS I DID on many other platforms from wikipedia to twitter (the last one, oogh, 3 times, LOL). It is not very comfortable for me to inline my anwsers on gmail for android as I always write on the run and I have to very carefully click in right spot and press enter. BUT it is certainly possible and I can even do it every time, but: I do not need to send anymore patches so I will continue to "Plain reject these guidelines". Bye. P.S. please resend to mailing list, though I could have used one of my other emails. " No further comments on that from me. -Thilo --- Begin Message --- чт, 10 июн. 2021 г., 23:29 Thilo Borgmann : > Hi all, > > some of you might have realized that I've removed Valerii Zapodovnikov > from our mailing list after [1]. > > I did this in my function as a mailing list admin because I think our > guidelines for the mailing list are mandatory for everyone to be welcome > here. > Plain rejection of these guidelines makes you not welcome here by > definition. > Any bot would have made the same decision. > > Let me point out that this case differs from the "usual" (sorry, dear > community) personally offensive misbehavior against others which is "just" > in conflict with parts of the guidelines - and shall be prosecuted by the > community and not an admin. > > That's my thoughts, as admin for this community. I made that decision and > I stand by it. > > Unfortunately, as this identity is reappearing, I think this mail is > needed because I have just told him a second time why he got himself banned > and will do so without further ado as long as this identity cares to come > back. > > However, I'm serving this community as an admin and I'm not free of error > or misjudgement. > Therefore, > > - if another admin feels to overrule my decision, or > - if enough trusted members of this community frown upon me in this thread > to exceed my personal subjective limit... > > ... then I will be totally fine to revoke any bans on that identity and > release that one into our community. > > Thanks, > Thilo > > > [1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281206.html > ___ > 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". > You do understand that I inlined my answer "No" to you? I mean it can be recognised as trolling, maybe... But certainly not the BS that you did here, WHILE I was trying to help you because I thought .patch will not be recognised by patchwork, as it usually is if you do not configure MIME. This is disgusting behaviour coming from you, but this is not the first time and certainly not the last, this is Internet
Re: [FFmpeg-devel] [PATCH v2] avfilter/vf_overlay_cuda: support expression of x y position
Applied, thanks! smime.p7s Description: S/MIME Cryptographic 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] [RFC] Removal of Valerii Zapodovnikov
Hi all, some of you might have realized that I've removed Valerii Zapodovnikov from our mailing list after [1]. I did this in my function as a mailing list admin because I think our guidelines for the mailing list are mandatory for everyone to be welcome here. Plain rejection of these guidelines makes you not welcome here by definition. Any bot would have made the same decision. Let me point out that this case differs from the "usual" (sorry, dear community) personally offensive misbehavior against others which is "just" in conflict with parts of the guidelines - and shall be prosecuted by the community and not an admin. That's my thoughts, as admin for this community. I made that decision and I stand by it. Unfortunately, as this identity is reappearing, I think this mail is needed because I have just told him a second time why he got himself banned and will do so without further ado as long as this identity cares to come back. However, I'm serving this community as an admin and I'm not free of error or misjudgement. Therefore, - if another admin feels to overrule my decision, or - if enough trusted members of this community frown upon me in this thread to exceed my personal subjective limit... ... then I will be totally fine to revoke any bans on that identity and release that one into our community. Thanks, Thilo [1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281206.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] lavc/aomdec: Allow RGB decoding
Am 10.06.21 um 20:26 schrieb Balling: > чт, 10 июн. 2021 г., 20:33 James Zern : > >> On Thu, Jun 10, 2021 at 9:26 AM Валерий Заподовников >> wrote: >>> This doesn't match the check in libdav1d.c. >>> >>> I do not really care what libdav1d.c does, since it uses internal to AV1 >> spec DAV1D_MC_IDENTITY (and frame->colorspace). This should have been a no >> go into FFmpeg, but it was not somehow. FFmpeg can handle all of this in by >> itself, there is no need to use examples from AV1 spec, which are wrong by >> omission: see for yourself. >> https://github.com/FFmpeg/FFmpeg/blob/c8778606b3811da6bd58ca6b73d2446bd430013e/libavcodec/libdav1d.c#L299 >>> >> >> It would be better to put this on the list for others to be able to >> comment on. >> >>> This is sRGB definition from AV1, not R'G'B' (or even RGB in case of >> linear transfer) and thus is total garbage. What if the file is tagged with >> DCI-P3 transfer and/or primaries? I am not going to fix libdav1d.c, since >> as I said, it uses internal names and internal deliberations which are >> unacceptable. >>> >>> Again. AV1 spec is really quite hillarious in that part. For example it >> does mention XYZ, but does not check for it anywhere, try Ctrl-F here: >> https://aomediacodec.github.io/av1-spec/av1-spec.pdf >>> >>> Thus I did implement filtering this out too, since FFmpeg does support >> XYZ pixel format (and maybe somebody one day will implement it too). >>> >>> Listen, my patch is correct. AV1 spec is also telling the same, as it >> shows all the possible primaries and transfers. But it only looks into sRGB >> scenario, since your people (Google, etc) were not capable to implement >> normal icc color management, but FFmpeg with mpv can do it and thus no need >> to concentrate on sRGB like your people did. I mean you can fix AV1 spec if >> you want, but IMHO it is correct already, this is color spaces info 101 and >> AV1 is depending on ITU-R H.273. >> > > > Okay, I will put it into the mailing list. > P.S. Dunno what that guy was thinking. I am not going to inline my comments > any more than I want to. I mean sure I do not top post, but just since this > is just waste of Internet traffic, and so is inlining in most cases (but > right here it is COOL and very needed). You are banned by identity not by mail address. You are banned because you explicitly rejected the guidelines for development and this mailing list in [1]. Therefore this mailing list is no place for you. It's as simple as that. If you desire to be welcome here you are now obliged to explicitly accept these rules. We imply acceptance only for new subscribers (identities). -Thilo [1] https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281206.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat: make AVStream.pts_wrap_bits public
On 6/10/2021 3:18 PM, Michael Niedermayer wrote:
On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
It can be useful to library users, and is currently being used by ffmpeg.c
Suggested-by: Hendrik Leppkes
Signed-off-by: James Almer
---
doc/APIchanges | 3 +++
libavformat/avformat.h | 17 +++--
libavformat/version.h | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index c46f4d5304..1b25bddd43 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@ libavutil: 2021-04-27
API changes, most recent first:
+2021-06-09 - xx - lavf 59.3.100 - avformat.h
+ Add pts_wrap_bits to AVStream
+
2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
Constified AVCodecParserContext.parser.
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 094683f12a..0d12d5b0d2 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -980,17 +980,14 @@ typedef struct AVStream {
*/
AVCodecParameters *codecpar;
-/*
- * All fields below this line are not part of the public API. They
- * may not be used outside of libavformat and can be changed and
- * removed at will.
- * Internal note: be aware that physically removing these fields
- * will break ABI. Replace removed fields with dummy fields, and
- * add new fields to AVStreamInternal.
- *
+/**
+ * Number of bits in pts. Used for wrapping control.
+ *
+ * - demuxing: set by libavformat
+ * - muxing: set by libavformat
+ *
*/
-
-int pts_wrap_bits; /**< number of bits in pts (used for wrapping control)
*/
+int pts_wrap_bits;
/**
* An opaque field for libavformat internal usage.
The "All fields below this line..." thing should be moved down instead of
removed as i realize that this was not the last field
No, the remaining field is the AVStreamInternal opaque one. Like in
other structs, it's public in the sense its offset is fixed, but should
not be accessed by the user.
One of purposes of this patch is precisely get rid of that ugly notice,
so new fields are added directly at the end, and existing offsets can
remain fixed within a given soname.
thx
[...]
___
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] avformat: make AVStream.pts_wrap_bits public
On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
> It can be useful to library users, and is currently being used by ffmpeg.c
>
> Suggested-by: Hendrik Leppkes
> Signed-off-by: James Almer
> ---
> doc/APIchanges | 3 +++
> libavformat/avformat.h | 17 +++--
> libavformat/version.h | 4 ++--
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index c46f4d5304..1b25bddd43 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,9 @@ libavutil: 2021-04-27
>
> API changes, most recent first:
>
> +2021-06-09 - xx - lavf 59.3.100 - avformat.h
> + Add pts_wrap_bits to AVStream
> +
> 2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
>Constified AVCodecParserContext.parser.
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 094683f12a..0d12d5b0d2 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -980,17 +980,14 @@ typedef struct AVStream {
> */
> AVCodecParameters *codecpar;
>
> -/*
> - * All fields below this line are not part of the public API. They
> - * may not be used outside of libavformat and can be changed and
> - * removed at will.
> - * Internal note: be aware that physically removing these fields
> - * will break ABI. Replace removed fields with dummy fields, and
> - * add new fields to AVStreamInternal.
> - *
> +/**
> + * Number of bits in pts. Used for wrapping control.
> + *
> + * - demuxing: set by libavformat
> + * - muxing: set by libavformat
> + *
> */
> -
> -int pts_wrap_bits; /**< number of bits in pts (used for wrapping
> control) */
> +int pts_wrap_bits;
>
> /**
> * An opaque field for libavformat internal usage.
The "All fields below this line..." thing should be moved down instead of
removed as i realize that this was not the last field
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The worst form of inequality is to try to make unequal things equal.
-- Aristotle
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] avformat: make AVStream.pts_wrap_bits public
On Wed, Jun 09, 2021 at 03:07:41PM -0300, James Almer wrote:
> It can be useful to library users, and is currently being used by ffmpeg.c
>
> Suggested-by: Hendrik Leppkes
> Signed-off-by: James Almer
> ---
> doc/APIchanges | 3 +++
> libavformat/avformat.h | 17 +++--
> libavformat/version.h | 4 ++--
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index c46f4d5304..1b25bddd43 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,9 @@ libavutil: 2021-04-27
>
> API changes, most recent first:
>
> +2021-06-09 - xx - lavf 59.3.100 - avformat.h
> + Add pts_wrap_bits to AVStream
> +
> 2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
>Constified AVCodecParserContext.parser.
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 094683f12a..0d12d5b0d2 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -980,17 +980,14 @@ typedef struct AVStream {
> */
> AVCodecParameters *codecpar;
>
> -/*
> - * All fields below this line are not part of the public API. They
> - * may not be used outside of libavformat and can be changed and
> - * removed at will.
> - * Internal note: be aware that physically removing these fields
> - * will break ABI. Replace removed fields with dummy fields, and
> - * add new fields to AVStreamInternal.
> - *
> +/**
> + * Number of bits in pts. Used for wrapping control.
pts and dts or in timestamps, its not just pts (no need to fix it in this patch
as its just copied as i realize) but it should be fixed when its made public
patch LGTM
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
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: Pass the HDR10+ metadata to the packet side data in VP9 encoder
HDR10+ metadata is stored in the bit stream for HEVC. The story is different
for VP9 and cannot store the metadata in the bit stream. HDR10+ should be
passed to packet side data an stored in the container (mkv) for VP9.
This CL is taking HDR10+ from AVFrame side data in libvpxenc and is passing it
to the AVPacket side data.
---
doc/APIchanges | 2 +
libavcodec/avpacket.c | 1 +
libavcodec/decode.c| 1 +
libavcodec/libvpxenc.c | 99 ++
libavcodec/packet.h| 8
libavcodec/version.h | 4 +-
6 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 55171311ed..bba5b06c6a 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,8 @@ libavutil: 2021-04-27
API changes, most recent first:
+2021-05-25 - 8c88a66d3c - lavc 59.2.100 - packet.h
+ Add AV_PKT_DATA_DYNAMIC_HDR10_PLUS
2021-xx-xx - xx - lavc 59.1.100 - avcodec.h codec.h
Move av_get_profile_name() from avcodec.h to codec.h.
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 7383d12d3e..800bee3489 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -289,6 +289,7 @@ const char *av_packet_side_data_name(enum
AVPacketSideDataType type)
case AV_PKT_DATA_ICC_PROFILE:return "ICC Profile";
case AV_PKT_DATA_DOVI_CONF: return "DOVI configuration
record";
case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE ST 12-1:2014
timecode";
+case AV_PKT_DATA_DYNAMIC_HDR10_PLUS: return "HDR10+ Dynamic
Metadata (SMPTE 2094-40)";
}
return NULL;
}
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 75bc7ad98e..40f688e40c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1488,6 +1488,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame
*frame)
{ AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC },
{ AV_PKT_DATA_ICC_PROFILE,AV_FRAME_DATA_ICC_PROFILE },
{ AV_PKT_DATA_S12M_TIMECODE, AV_FRAME_DATA_S12M_TIMECODE
},
+{ AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
};
if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >=
sizeof(*pkt))
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index 66bad444d0..e2e6c60b68 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -64,6 +64,11 @@ struct FrameListData {
struct FrameListData *next;
};
+typedef struct FrameHDR10Plus {
+int64_t pts;
+AVBufferRef *hdr10_plus;
+} FrameHDR10Plus;
+
typedef struct VPxEncoderContext {
AVClass *class;
struct vpx_codec_ctx encoder;
@@ -121,6 +126,8 @@ typedef struct VPxEncoderContext {
int tune_content;
int corpus_complexity;
int tpl_model;
+int discard_hdr10_plus;
+AVFifoBuffer *hdr10_plus_fifo;
/**
* If the driver does not support ROI then warn the first time we
* encounter a frame with ROI side data.
@@ -316,6 +323,55 @@ static av_cold void free_frame_list(struct FrameListData
*list)
}
}
+static av_cold int add_hdr10_plus(AVFifoBuffer *fifo, struct FrameHDR10Plus
*data)
+{
+int err = av_fifo_grow(fifo, sizeof(*data));
+if (err < 0)
+return err;
+av_fifo_generic_write(fifo, data, sizeof(*data), NULL);
+return 0;
+}
+
+static av_cold void free_hdr10_plus(struct FrameHDR10Plus *p)
+{
+if (!p)
+return;
+av_buffer_unref(&p->hdr10_plus);
+av_free(p);
+}
+
+static av_cold void free_hdr10_plus_fifo(AVFifoBuffer **fifo)
+{
+FrameHDR10Plus *frame_hdr10_plus = NULL;
+while (av_fifo_size(*fifo) >= sizeof(FrameHDR10Plus)) {
+av_fifo_generic_read(*fifo, frame_hdr10_plus, sizeof(FrameHDR10Plus),
NULL);
+free_hdr10_plus(frame_hdr10_plus);
+}
+av_fifo_freep(fifo);
+}
+
+static int copy_hdr10_plus_to_pkt(AVFifoBuffer *fifo, AVPacket *pkt)
+{
+FrameHDR10Plus frame_hdr10_plus;
+uint8_t *data;
+if (!pkt)
+return 0;
+if (av_fifo_size(fifo) < sizeof(frame_hdr10_plus))
+return 0;
+
+av_fifo_generic_read(fifo, &frame_hdr10_plus, sizeof(frame_hdr10_plus),
NULL);
+if (!frame_hdr10_plus.hdr10_plus || frame_hdr10_plus.pts != pkt->pts)
+return 0;
+
+data = av_packet_new_side_data(pkt, AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
frame_hdr10_plus.hdr10_plus->size);
+if (!data)
+return AVERROR(ENOMEM);
+
+memcpy(data, frame_hdr10_plus.hdr10_plus->data,
frame_hdr10_plus.hdr10_plus->size);
+
+return 0;
+}
+
static av_cold int codecctl_int(AVCodecContext *avctx,
enum vp8e_enc_control_id id, int val)
{
@@ -384,6 +440,8 @@ static av_cold int vpx_free(AVCodecContext *avctx)
av_freep(&ctx->twopass_stats.buf);
av_freep(&avctx->stats_out);
free_frame_list(ctx->coded_frame_list);
+if (ctx->hdr10_plus_fifo)
+free_hdr10_plus_fifo(&ctx->hdr10_plus_fifo
Re: [FFmpeg-devel] [PATCH] avcodec: Pass the HDR10+ metadata to the packet side data in VP9 encoder
On Tue, Jun 8, 2021 at 12:01 PM Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:
> Andreas Rheinhardt:
> > Mohammad Izadi:
> >> HDR10+ metadata is stored in the bit stream for HEVC. The story is
> different for VP9 and cannot store the metadata in the bit stream. HDR10+
> should be passed to packet side data an stored in the container (mkv) for
> VP9.
> >>
> >> This CL is taking HDR10+ from AVFrame side data in libvpxenc and is
> passing it to the AVPacket side data.
> >> ---
> >> doc/APIchanges | 2 +
> >> libavcodec/avpacket.c | 1 +
> >> libavcodec/decode.c| 1 +
> >> libavcodec/libvpxenc.c | 99 ++
> >> libavcodec/packet.h| 8
> >> libavcodec/version.h | 4 +-
> >> 6 files changed, 113 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index c46f4d5304..60995579e5 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -13,6 +13,8 @@ libavutil: 2021-04-27
> >>
> >>
> >> API changes, most recent first:
> >> +2021-05-25 - 8c88a66d3c - lavc 59.2.100 - packet.h
> >> + Add AV_PKT_DATA_DYNAMIC_HDR10_PLUS
> >>
> >> 2021-04-27 - cb3ac722f4 - lavc 59.0.100 - avcodec.h
> >>Constified AVCodecParserContext.parser.
> >> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> >> index 7383d12d3e..800bee3489 100644
> >> --- a/libavcodec/avpacket.c
> >> +++ b/libavcodec/avpacket.c
> >> @@ -289,6 +289,7 @@ const char *av_packet_side_data_name(enum
> AVPacketSideDataType type)
> >> case AV_PKT_DATA_ICC_PROFILE:return "ICC Profile";
> >> case AV_PKT_DATA_DOVI_CONF: return "DOVI
> configuration record";
> >> case AV_PKT_DATA_S12M_TIMECODE: return "SMPTE ST
> 12-1:2014 timecode";
> >> +case AV_PKT_DATA_DYNAMIC_HDR10_PLUS: return "HDR10+
> Dynamic Metadata (SMPTE 2094-40)";
> >> }
> >> return NULL;
> >> }
> >> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> >> index 75bc7ad98e..40f688e40c 100644
> >> --- a/libavcodec/decode.c
> >> +++ b/libavcodec/decode.c
> >> @@ -1488,6 +1488,7 @@ int ff_decode_frame_props(AVCodecContext *avctx,
> AVFrame *frame)
> >> { AV_PKT_DATA_A53_CC, AV_FRAME_DATA_A53_CC
> },
> >> { AV_PKT_DATA_ICC_PROFILE,
> AV_FRAME_DATA_ICC_PROFILE },
> >> { AV_PKT_DATA_S12M_TIMECODE,
> AV_FRAME_DATA_S12M_TIMECODE },
> >> +{ AV_PKT_DATA_DYNAMIC_HDR10_PLUS,
> AV_FRAME_DATA_DYNAMIC_HDR_PLUS },
> >> };
> >>
> >> if (IS_EMPTY(pkt) && av_fifo_size(avctx->internal->pkt_props) >=
> sizeof(*pkt))
> >> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> >> index 66bad444d0..d15cf29766 100644
> >> --- a/libavcodec/libvpxenc.c
> >> +++ b/libavcodec/libvpxenc.c
> >> @@ -64,6 +64,11 @@ struct FrameListData {
> >> struct FrameListData *next;
> >> };
> >>
> >> +typedef struct FrameHDR10Plus {
> >> +int64_t pts;
> >> +AVBufferRef *hdr10_plus;
> >> +} FrameHDR10Plus;
> >> +
> >> typedef struct VPxEncoderContext {
> >> AVClass *class;
> >> struct vpx_codec_ctx encoder;
> >> @@ -121,6 +126,8 @@ typedef struct VPxEncoderContext {
> >> int tune_content;
> >> int corpus_complexity;
> >> int tpl_model;
> >> +int discard_hdr10_plus;
> >> +AVFifoBuffer *hdr10_plus_fifo;
> >> /**
> >> * If the driver does not support ROI then warn the first time we
> >> * encounter a frame with ROI side data.
> >> @@ -316,6 +323,55 @@ static av_cold void free_frame_list(struct
> FrameListData *list)
> >> }
> >> }
> >>
> >> +static av_cold int add_hdr10_plus(AVFifoBuffer *fifo, struct
> FrameHDR10Plus *data)
> >> +{
> >> +int err = av_fifo_grow(fifo, sizeof(*data));
> >> +if (err < 0)
> >> +return err;
> >> +av_fifo_generic_write(fifo, data, sizeof(*data), NULL);
> >> +return 0;
> >> +}
> >> +
> >> +static av_cold void free_hdr10_plus(struct FrameHDR10Plus *p)
> >> +{
> >> +if (!p)
> >> +return;
> >> +av_buffer_unref(&p->hdr10_plus);
> >> +av_free(p);
> >> +}
> >> +
> >> +static av_cold void free_hdr10_plus_fifo(AVFifoBuffer **fifo)
> >> +{
> >> +FrameHDR10Plus *frame_hdr10_plus = NULL;
> >> +while (av_fifo_size(*fifo) >= sizeof(FrameHDR10Plus)) {
> >> +av_fifo_generic_read(*fifo, frame_hdr10_plus,
> sizeof(FrameHDR10Plus), NULL);
> >
> > Did you ever test this with the fifo being nonempty? (It will segfault.)
> >
> >> +free_hdr10_plus(frame_hdr10_plus);
> >> +}
> >> +av_fifo_freep(fifo);
> >> +}
> >> +
> >> +static int copy_hdr10_plus_to_pkt(AVFifoBuffer *fifo, AVPacket *pkt)
> >> +{
> >> +FrameHDR10Plus *frame_hdr10_plus;
> >> +uint8_t *data;
> >> +if (av_fifo_size(fifo) < 1)
> >> +return 0;
> >> +
> >> +av_fifo_generic_read(fifo, frame_hdr10_plus,
> sizeof(*frame_hdr10_plus), NULL);
>
> And so will this here in all likelihood, because you try to write
> sizeof(*frame_hdr10_
Re: [FFmpeg-devel] [PATCH 20/24] sws: add a function for scaling dst slices
Quoting Michael Niedermayer (2021-06-01 15:02:27) > On Mon, May 31, 2021 at 09:55:11AM +0200, Anton Khirnov wrote: > > Currently existing sws_scale() accepts as input a user-determined slice > > of input data and produces an indeterminate number of output lines. > > swscale() should return the number of lines output > it does "return dstY - lastDstY;" But you do not know the number of lines beforehand. I suppose one could assume that the line counts will always be the same for any run with the same parameters (strictly speaking this is not guaranteed) and store them after the first frame, but then the first scale call is not parallel. And it would be quite ugly. > > > > Since the calling code does not know the amount of output, it cannot > > easily parallelize scaling by calling sws_scale() simultaneously on > > different parts of the frame. > > > > Add a new function - sws_scale_dst_slice() - that accepts as input the > > entire input frame and produces a specified slice of the output. This > > function can be called simultaneously on different slices of the output > > frame (using different sws contexts) to implement slice threading. > > an API that would allow starting before the whole frame is available > would have reduced latency and better cache locality. Maybe that can > be added later too but i wanted to mention it because the documentation > exlicitly says "entire input" That would require some way of querying how much input is required for each line. I dot not feel sufficiently familiar with sws architecture to see an obvious way of implementing this. And then making use of this information would require a significantly more sophisticated way of dispatching work to threads. Or are you proposing some specific alternative way of implementing this? > > Also there are a few tables between the multiple SwsContext which are > identical, it would be ideal if they can be shared between threads > I guess such sharing would need to be implemented before the API is > stable otherwise adding it later would require application to be changed In my tests, the differences are rather small. E.g. scaling 2500x3000->3000x3000 with 32 threads uses only ~15% more memory than with 1 thread. And I do not see an obvious way to implement this that would be worth the extra complexity. Do you? -- Anton Khirnov ___ 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] sws: rename SwsContext.swscale to convert_unscaled
That function pointer is now used only for unscaled conversion.
---
libswscale/aarch64/swscale_unscaled.c | 2 +-
libswscale/arm/swscale_unscaled.c | 6 +--
libswscale/ppc/yuv2yuv_altivec.c | 4 +-
libswscale/swscale.c | 5 +-
libswscale/swscale_internal.h | 6 +--
libswscale/swscale_unscaled.c | 68 +--
libswscale/utils.c| 4 +-
7 files changed, 48 insertions(+), 47 deletions(-)
diff --git a/libswscale/aarch64/swscale_unscaled.c
b/libswscale/aarch64/swscale_unscaled.c
index c7a2a1037d..b3093bbc9d 100644
--- a/libswscale/aarch64/swscale_unscaled.c
+++ b/libswscale/aarch64/swscale_unscaled.c
@@ -102,7 +102,7 @@ DECLARE_FF_NVX_TO_ALL_RGBX_FUNCS(nv21)
&& !(c->srcH & 1)
\
&& !(c->srcW & 15)
\
&& !accurate_rnd)
\
-c->swscale = ifmt##_to_##ofmt##_neon_wrapper;
\
+c->convert_unscaled = ifmt##_to_##ofmt##_neon_wrapper;
\
} while (0)
#define SET_FF_NVX_TO_ALL_RGBX_FUNC(nvx, NVX, accurate_rnd) do {
\
diff --git a/libswscale/arm/swscale_unscaled.c
b/libswscale/arm/swscale_unscaled.c
index e41f294eac..910610b86e 100644
--- a/libswscale/arm/swscale_unscaled.c
+++ b/libswscale/arm/swscale_unscaled.c
@@ -147,7 +147,7 @@ DECLARE_FF_NVX_TO_ALL_RGBX_FUNCS(nv21)
&& !(c->srcH & 1)
\
&& !(c->srcW & 15)
\
&& !accurate_rnd) {
\
-c->swscale = ifmt##_to_##ofmt##_neon_wrapper;
\
+c->convert_unscaled = ifmt##_to_##ofmt##_neon_wrapper;
\
}
\
} while (0)
@@ -163,8 +163,8 @@ static void get_unscaled_swscale_neon(SwsContext *c) {
if (c->srcFormat == AV_PIX_FMT_RGBA
&& c->dstFormat == AV_PIX_FMT_NV12
&& (c->srcW >= 16)) {
-c->swscale = accurate_rnd ? rgbx_to_nv12_neon_32_wrapper
-: rgbx_to_nv12_neon_16_wrapper;
+c->convert_unscaled = accurate_rnd ? rgbx_to_nv12_neon_32_wrapper
+ : rgbx_to_nv12_neon_16_wrapper;
}
SET_FF_NVX_TO_ALL_RGBX_FUNC(nv12, NV12, accurate_rnd);
diff --git a/libswscale/ppc/yuv2yuv_altivec.c b/libswscale/ppc/yuv2yuv_altivec.c
index 2b1c5dd3b8..728e4d210c 100644
--- a/libswscale/ppc/yuv2yuv_altivec.c
+++ b/libswscale/ppc/yuv2yuv_altivec.c
@@ -196,9 +196,9 @@ av_cold void ff_get_unscaled_swscale_ppc(SwsContext *c)
// unscaled YV12 -> packed YUV, we want speed
if (dstFormat == AV_PIX_FMT_YUYV422)
-c->swscale = yv12toyuy2_unscaled_altivec;
+c->convert_unscaled = yv12toyuy2_unscaled_altivec;
else if (dstFormat == AV_PIX_FMT_UYVY422)
-c->swscale = yv12touyvy_unscaled_altivec;
+c->convert_unscaled = yv12touyvy_unscaled_altivec;
}
#endif /* HAVE_ALTIVEC */
}
diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 4b577ef263..75226a29e4 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -987,8 +987,9 @@ int attribute_align_arg sws_scale(struct SwsContext *c,
if (srcSliceY_internal + srcSliceH == c->srcH)
c->sliceDir = 0;
-if (c->swscale)
-ret = c->swscale(c, src2, srcStride2, srcSliceY_internal, srcSliceH,
dst2, dstStride2);
+if (c->convert_unscaled)
+ret = c->convert_unscaled(c, src2, srcStride2, srcSliceY_internal,
srcSliceH,
+ dst2, dstStride2);
else
ret = swscale(c, src2, srcStride2, srcSliceY_internal, srcSliceH,
dst2, dstStride2);
diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index 02a45e7adb..a4813940d6 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -290,7 +290,7 @@ typedef struct SwsContext {
* Note that src, dst, srcStride, dstStride will be copied in the
* sws_scale() wrapper so they can be freely modified here.
*/
-SwsFunc swscale;
+SwsFunc convert_unscaled;
int srcW; ///< Width of source luma/alpha planes.
int srcH; ///< Height of source luma/alpha planes.
int dstH; ///< Height of destination luma/alpha planes.
@@ -864,8 +864,8 @@ extern const int32_t ff_yuv2rgb_coeffs[11][4];
extern const AVClass ff_sws_context_class;
/**
- * Set c->swscale to an unscaled converter if one exists for the specifi
Re: [FFmpeg-devel] [PATCH] lavu/mem: un-inline av_size_mult()
On 6/10/2021 12:01 PM, Anton Khirnov wrote:
Quoting James Almer (2021-05-31 13:56:20)
On 5/31/2021 6:26 AM, Anton Khirnov wrote:
There seems to be no compelling reason for it to be inline.
---
libavutil/mem.c | 18 ++
libavutil/mem.h | 19 +--
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/libavutil/mem.c b/libavutil/mem.c
index a52d33d4a6..063635fb22 100644
--- a/libavutil/mem.c
+++ b/libavutil/mem.c
@@ -547,3 +547,21 @@ void av_fast_mallocz(void *ptr, unsigned int *size, size_t
min_size)
{
fast_malloc(ptr, size, min_size, 1);
}
+
+int av_size_mult(size_t a, size_t b, size_t *r)
+{
+size_t t;
+
+#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) ||
AV_HAS_BUILTIN(__builtin_mul_overflow)
+if (__builtin_mul_overflow(a, b, &t))
+return AVERROR(EINVAL);
+#else
+t = a * b;
+/* Hack inspired from glibc: don't try the division if nelem and elsize
+ * are both less than sqrt(SIZE_MAX). */
+if ((a | b) >= ((size_t)1 << (sizeof(size_t) * 4)) && a && t / a != b)
+return AVERROR(EINVAL);
+#endif
+*r = t;
+return 0;
+}
diff --git a/libavutil/mem.h b/libavutil/mem.h
index c876111afb..e9d343eaf0 100644
--- a/libavutil/mem.h
+++ b/libavutil/mem.h
@@ -31,7 +31,6 @@
#include
#include "attributes.h"
-#include "error.h"
#include "avutil.h"
#include "version.h"
@@ -672,23 +671,7 @@ void *av_dynarray2_add(void **tab_ptr, int *nb_ptr, size_t elem_size,
* @param[out] r Pointer to the result of the operation
* @return 0 on success, AVERROR(EINVAL) on overflow
*/
You should also move the doxy.
Why?
Sorry, for whatever reason i read this as moving the function to mem.c
and not just the implementation (Like i did with ff_fast_malloc).
___
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] lavu/mem: un-inline av_size_mult()
Quoting James Almer (2021-05-31 13:56:20)
> On 5/31/2021 6:26 AM, Anton Khirnov wrote:
> > There seems to be no compelling reason for it to be inline.
> > ---
> > libavutil/mem.c | 18 ++
> > libavutil/mem.h | 19 +--
> > 2 files changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/libavutil/mem.c b/libavutil/mem.c
> > index a52d33d4a6..063635fb22 100644
> > --- a/libavutil/mem.c
> > +++ b/libavutil/mem.c
> > @@ -547,3 +547,21 @@ void av_fast_mallocz(void *ptr, unsigned int *size,
> > size_t min_size)
> > {
> > fast_malloc(ptr, size, min_size, 1);
> > }
> > +
> > +int av_size_mult(size_t a, size_t b, size_t *r)
> > +{
> > +size_t t;
> > +
> > +#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) ||
> > AV_HAS_BUILTIN(__builtin_mul_overflow)
> > +if (__builtin_mul_overflow(a, b, &t))
> > +return AVERROR(EINVAL);
> > +#else
> > +t = a * b;
> > +/* Hack inspired from glibc: don't try the division if nelem and elsize
> > + * are both less than sqrt(SIZE_MAX). */
> > +if ((a | b) >= ((size_t)1 << (sizeof(size_t) * 4)) && a && t / a != b)
> > +return AVERROR(EINVAL);
> > +#endif
> > +*r = t;
> > +return 0;
> > +}
> > diff --git a/libavutil/mem.h b/libavutil/mem.h
> > index c876111afb..e9d343eaf0 100644
> > --- a/libavutil/mem.h
> > +++ b/libavutil/mem.h
> > @@ -31,7 +31,6 @@
> > #include
> >
> > #include "attributes.h"
> > -#include "error.h"
> > #include "avutil.h"
> > #include "version.h"
> >
> > @@ -672,23 +671,7 @@ void *av_dynarray2_add(void **tab_ptr, int *nb_ptr,
> > size_t elem_size,
> >* @param[out] r Pointer to the result of the operation
> >* @return 0 on success, AVERROR(EINVAL) on overflow
> >*/
>
> You should also move the doxy.
Why?
--
Anton Khirnov
___
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/3] avcodec/faxcompr: Check if bits are available before reading in cmode == 9 || cmode == 10
Fixes: Timeout
Fixes:
34950/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-5686764151898112
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/faxcompr.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/libavcodec/faxcompr.c b/libavcodec/faxcompr.c
index d44de2485d..45e0c482d7 100644
--- a/libavcodec/faxcompr.c
+++ b/libavcodec/faxcompr.c
@@ -304,7 +304,10 @@ static int decode_group3_2d_line(AVCodecContext *avctx,
GetBitContext *gb,
mode = !mode;
}
} else if (cmode == 9 || cmode == 10) {
-int xxx = get_bits(gb, 3);
+int xxx;
+if (get_bits_left(gb) < 3)
+return AVERROR_INVALIDDATA;
+xxx = get_bits(gb, 3);
if (cmode == 9 && xxx == 7) {
int ret;
int pix_left = width - offs;
--
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".
[FFmpeg-devel] [PATCH 1/3] avformat/utils: Avoid overflow in codec_info_duration computation for subtitles
Fixes: signed integer overflow: 9223126845747118112 - -2594073385365397472
cannot be represented in type 'long'
Fixes:
34936/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6739888002170880
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavformat/utils.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 0fc55c8da2..0c64a7c6fb 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -3843,7 +3843,9 @@ int avformat_find_stream_info(AVFormatContext *ic,
AVDictionary **options)
break;
}
if (pkt->duration) {
-if (avctx->codec_type == AVMEDIA_TYPE_SUBTITLE && pkt->pts !=
AV_NOPTS_VALUE && st->start_time != AV_NOPTS_VALUE && pkt->pts >=
st->start_time) {
+if (avctx->codec_type == AVMEDIA_TYPE_SUBTITLE && pkt->pts !=
AV_NOPTS_VALUE && st->start_time != AV_NOPTS_VALUE && pkt->pts >= st->start_time
+&& (uint64_t)pkt->pts - st->start_time < INT64_MAX
+) {
st->internal->info->codec_info_duration = FFMIN(pkt->pts -
st->start_time, st->internal->info->codec_info_duration + pkt->duration);
} else
st->internal->info->codec_info_duration += pkt->duration;
--
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".
[FFmpeg-devel] [PATCH 3/3] avcodec/faxcompr: Check available bits in decode_uncompressed()
Fixes: Timeout
Fixes:
34950/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-5686764151898112
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/faxcompr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavcodec/faxcompr.c b/libavcodec/faxcompr.c
index 45e0c482d7..44c1f6f6b9 100644
--- a/libavcodec/faxcompr.c
+++ b/libavcodec/faxcompr.c
@@ -144,6 +144,8 @@ static int decode_uncompressed(AVCodecContext *avctx,
GetBitContext *gb,
return AVERROR_INVALIDDATA;
}
cwi = 10 - av_log2(cwi);
+if (get_bits_left(gb) < cwi + 1)
+return AVERROR_INVALIDDATA;
skip_bits(gb, cwi + 1);
if (cwi > 5) {
newmode = get_bits1(gb);
--
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 0/4] avdevice/dshow: implement capabilities API
Let me respond on two levels. Before exploring the design space of a separation of libavdevice and libavformat below, I think it is important to first comment on the current state (and whether the AVDevice Capabilities part of my patch series should be blocked by this discussion). Importantly, I would suppose that any reorganization of libavdevice and libavformat and redesign of the libavdevice API must aim to offer at least the same functionality as the current API, that is, an avdevice should be able to be queried for what devices it offers (get_device_list), should for each device provide information about what formats it accepts/can provide (create_device_capabilities/free_device_capabilities) and should be able to be controlled through the API (control_message). Perhaps these take different forms, but same functionality should be offered. As such, having AVDevice Capabilities API implemented for one of the devices should help, not hamper, redesign efforts because it shows how this API would actually be used in practice. Fundamental changes such as a new avdevice API will be backwards incompatible no matter what, so having one more bit of important functionality (create_device_capabilities/free_device_capabilities) implemented doesn't create a larger threshold to initiating such a redesign effort. Instead, it forces that all the current API functionality is thought out as well during the redesign effort and nothing is forgotten. I thus argue that its a good thing to bring back the AVDevice Capabilities API, since it helps, not hinders the redesign effort. And lets not forget it offers users of the current API functionality (me at least) they need now, not at some indeterminate timepoint in the future. On Wed, Jun 9, 2021 at 10:33 PM Anton Khirnov wrote: > Look through the threads > [...] Thanks for the pointers! > The problem is that libavdevice is a separate library from libavformat, > but fundamentally depends on accessing libavformat internals. Ah ok, so this is at first instance about cleanup/separation, not necessarily about adding new functionality (I do see Mark's list of opportunities that a new API offer, copied below). I see Nicolas argue this entanglement of internals is not a problem in practice, and i suppose there is a certain amount of taste involved here. Nothing wrong with that. I guess for me personally that it is a little funky to have to add/change things in AVFormat when changing the AVDevice API, and that it may be good to for the longer term look at disentangling them. I will get back to that below, in response to some quotes of Mark's messages last January. Mark's (non-exhaustive) list of opportunities a libavdevice API redesign offers (numbered by me): On 20/01/2021 12:41, Mark Thompson wrote: > 1. Handle frames as well as packets. >1a. Including hardware frames - DRM objects from KMS/V4L2, D3D surfaces from Windows desktop duplication (which doesn't currently exist but should). > 2. Clear core option set - currently almost everything is set by inconsistent private options; things like pixel/sample format, frame/sample rate, geometry and hardware device should be common options to all. > 3. Asynchronicity - a big annoyance in current recording scenarios with the ffmpeg utility is that both audio and video capture block, and do so on the same thread which results in skipped frames. > 4. Capability probing - the existing method of options which log the capabilities are not very useful for API users. 1 and 3 i cannot speak to, but 4 is indeed what i ran into: the current state of most avdevices is not useful at all for an API user like me when it comes to capability probing (not a reason though to get rid of the whole API, but to wonder why it wasn't implemented. while nobody apparently bothered to do it before me, i think there will be more than just me who will actually use it). Currently I'd have to issue device specific options on a not-yet opened device, listen to the log output, parse it, etc. But the current API already solves this, if only it was implemented. A clear core option set would be nice indeed. And the AVDevice Capabilities API actually offers a start at that, since it lists a bunch of options that should be relevant to query (and set) for each device in the form of ff_device_capabilities (in my patchset), or av_device_capabilities before Andreas' patch removing it in January. I don't think its complete, but its a good starting point. Mark Thompson (2021-01-25): > * Many of those are using it via the ffmpeg utility, but not all. Indeed, i am an (aspiring) API user, of the dshow device specifically, and possibly v4l2 later (but my project is Windows-only right now). Currently hampered by lack of some API not being implemented for dshow, hence my patch set. > * The libavdevice API is the libavformat API because it was originally > split out from libavformat, and it has the nice property that devices > and files end up being interchangable in so
[FFmpeg-devel] [PATCH 2/2] avfilter/vf_drawtext: add rtctime
Compared to gmtime and localtime, rtctime has higher resolution.
For example, it can be used to show the end-to-end latency. On
the same host, publish a stream by:
./ffmpeg \
-re -f lavfi -i "color=color=blue:size=1280x720:rate=60" \
-c:v libx264 \
-tune zerolatency \
-vf "drawtext=text='push %{rtctime\:hms} pts
%{pts\:hms}':x=10:y=(h-th)/2:fontsize=30:box=1:boxcolor=white:boxborderw=30" \
-f flv $url
Use ffplay to show the latency:
./ffplay -threads 1 \
-vf "drawtext=text='play %{rtctime\:hms} pts
%{pts\:hms}':x=10:y=th:fontsize=30:box=1:boxcolor=white:boxborderw=30,setpts=PTS*4/5"
\
"$url"
---
doc/filters.texi | 3 +++
libavfilter/version.h | 2 +-
libavfilter/vf_drawtext.c | 24
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 78faf767cf..b2d4660327 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -10953,6 +10953,9 @@ It can accept an argument: a strftime() format string.
The time at which the filter is running, expressed in the local time zone.
It can accept an argument: a strftime() format string.
+@item rtctime
+The time at which the filter is running, can use the same format as @samp{pts}.
+
@item metadata
Frame metadata. Takes one or two arguments.
diff --git a/libavfilter/version.h b/libavfilter/version.h
index f12bc876ae..5052681653 100644
--- a/libavfilter/version.h
+++ b/libavfilter/version.h
@@ -31,7 +31,7 @@
#define LIBAVFILTER_VERSION_MAJOR 8
#define LIBAVFILTER_VERSION_MINOR 0
-#define LIBAVFILTER_VERSION_MICRO 101
+#define LIBAVFILTER_VERSION_MICRO 102
#define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index f7b9c25e62..e20b556a87 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -51,6 +51,7 @@
#include "libavutil/opt.h"
#include "libavutil/random_seed.h"
#include "libavutil/parseutils.h"
+#include "libavutil/time.h"
#include "libavutil/timecode.h"
#include "libavutil/time_internal.h"
#include "libavutil/tree.h"
@@ -959,12 +960,11 @@ static int func_pict_type(AVFilterContext *ctx, AVBPrint
*bp,
return 0;
}
-static int func_pts(AVFilterContext *ctx, AVBPrint *bp,
-char *fct, unsigned argc, char **argv, int tag)
+static int func_time_common(AVFilterContext *ctx, AVBPrint *bp,
+char *fct, unsigned argc, char **argv, int tag,
+double pts)
{
-DrawTextContext *s = ctx->priv;
const char *fmt;
-double pts = s->var_values[VAR_T];
int ret;
fmt = argc >= 1 ? argv[0] : "flt";
@@ -1019,6 +1019,21 @@ static int func_pts(AVFilterContext *ctx, AVBPrint *bp,
return 0;
}
+static int func_pts(AVFilterContext *ctx, AVBPrint *bp,
+char *fct, unsigned argc, char **argv, int tag)
+{
+DrawTextContext *s = ctx->priv;
+double pts = s->var_values[VAR_T];
+return func_time_common(ctx, bp, fct, argc, argv, tag, pts);
+}
+
+static int func_rtctime(AVFilterContext *ctx, AVBPrint *bp,
+char *fct, unsigned argc, char **argv, int tag)
+{
+double pts = av_gettime() / 100.0;
+return func_time_common(ctx, bp, fct, argc, argv, tag, pts);
+}
+
static int func_frame_num(AVFilterContext *ctx, AVBPrint *bp,
char *fct, unsigned argc, char **argv, int tag)
{
@@ -1153,6 +1168,7 @@ static const struct drawtext_function {
{ "pts", 0, 3, 0, func_pts },
{ "gmtime",0, 1, 'G', func_strftime },
{ "localtime", 0, 1, 'L', func_strftime },
+{ "rtctime", 0, 3, 0, func_rtctime },
{ "frame_num", 0, 0, 0, func_frame_num },
{ "n", 0, 0, 0, func_frame_num },
{ "metadata", 1, 2, 0, func_metadata },
--
2.31.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".
[FFmpeg-devel] [PATCH 1/2] avfilter/vf_drawtext: don't assign twice consecutively to the same value
--- libavfilter/vf_drawtext.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c index c4c09894e4..f7b9c25e62 100644 --- a/libavfilter/vf_drawtext.c +++ b/libavfilter/vf_drawtext.c @@ -1052,7 +1052,7 @@ static int func_strftime(AVFilterContext *ctx, AVBPrint *bp, if (tag == 'L') localtime_r(&now, &tm); else -tm = *gmtime_r(&now, &tm); +gmtime_r(&now, &tm); av_bprint_strftime(bp, fmt, &tm); return 0; } -- 2.31.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] [RFC PATCH] ffmpeg_videotoolbox: skip memory copy if hwaccel_output_format match
"zhilizhao(赵志立)" 于2021年6月10日周四 下午12:15写道：
>
> Ping.
>
> > On Apr 27, 2021, at 1:09 PM, Zhao Zhili wrote:
> >
> > From: zhilizhao
> >
> > Simple test results:
> >
> > Command:
> > ./ffmpeg -y -hwaccel videotoolbox -hwaccel_output_format videotoolbox_vld \
> > -i test.mp4 -an -c:v h264_videotoolbox -benchmark out.mp4
> >
> > Before:
> > frame= 1221 fps= 66 q=-0.0 Lsize=3144kB time=00:00:20.33
> > bitrate=1266.6kbits/s dup=4 drop=0 speed= 1.1x
> > bench: utime=2.714s stime=1.218s rtime=18.574s
> >
> > After:
> > frame= 1221 fps=137 q=-0.0 Lsize=3144kB time=00:00:20.33
> > bitrate=1266.4kbits/s dup=4 drop=0 speed=2.28x
> > bench: utime=1.450s stime=1.440s rtime=8.924s
> >
> > It has limited usecase since there is no video filter support, so
> > a log message is added to notify the user.
> > ---
> > It has a good performance with limited usecases like transcoding without
> > autoscale/autorotate, and I don't know if it break some special usecase,
> > since the RFC.
> >
> > fftools/ffmpeg_videotoolbox.c | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/fftools/ffmpeg_videotoolbox.c b/fftools/ffmpeg_videotoolbox.c
> > index a6b78d0f7d..4ba8618539 100644
> > --- a/fftools/ffmpeg_videotoolbox.c
> > +++ b/fftools/ffmpeg_videotoolbox.c
> > @@ -29,6 +29,7 @@
> >
> > typedef struct VTContext {
> > AVFrame *tmp_frame;
> > +int log_once;
> > } VTContext;
> >
> > char *videotoolbox_pixfmt;
> > @@ -44,6 +45,13 @@ static int videotoolbox_retrieve_data(AVCodecContext *s,
> > AVFrame *frame)
> > int linesize[4] = { 0 };
> > int planes, ret, i;
> >
> > +if (frame->format == ist->hwaccel_output_format) {
> > +av_log_once(s, AV_LOG_INFO, AV_LOG_TRACE, &vt->log_once,
> > +"There is no video filter for videotoolbox pix_fmt now, remove
> > the "
> > +"-hwaccel_output_format option if video filter doesn't
> > work\n");
> > +return 0;
> > +}
> > +
> > av_frame_unref(vt->tmp_frame);
> >
> > switch (pixel_format) {
> > --
> > 2.31.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".
> >
>
> ___
> 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".
pushed
Thanks
Steven
___
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/2] hwcontext_vulkan: dynamically load functions
Jun 10, 2021, 12:27 by d...@lynne.ee: > Jun 10, 2021, 03:38 by wenbin.c...@intel.com: > >>> Jun 8, 2021, 07:38 by wenbin.c...@intel.com: >>> >>> >> Apr 29, 2021, 03:52 by d...@lynne.ee: >>> >> >>> >> > This patch allows for alternative loader implementations. >>> >> > >>> >> > Patch attached. >>> >> > >>> >> >>> >> Forgot to fix a flag, v2 attached. >>> >> >>> > >>> > Hi Lynne: >>> > I tried the following command: >>> > "ffmpeg -init_hw_device vulkan=vul:0 -filter_hw_device vul -i 1080p.264 >>> > -vf >>> "hwupload=extra_hw_frames=16,scale_vulkan=w=1920:h1080" -f null" >>> > It report a segmentation fault at >>> https://github.com/FFmpeg/FFmpeg/blob/282b9f4cba7ca361c43ac9f94031a >>> 43109df0a4f/libavutil/hwcontext_vulkan.c#L1018 >>> > The function pointer vk->CreateCommandPool is NULL. >>> > I build on ffmpeg master. >>> > >>> >>> Weird. What Vulkan drivers and what OS was used? >>> If CreateCommandPool is NULL then initialization probably >>> wouldn't even happen. >>> >> >> I use linux ubuntu 20.04 and I get the vulkan driver from: >> https://www.lunarg.com/vulkan-sdk/. >> I see the load_functions() is called three times, but none of them set >> "has_dev" argument to 1. >> I tried to change the code in >> https://github.com/FFmpeg/FFmpeg/blob/591b88e6787c4e678237f02a50421d101abd25c2/libavutil/hwcontext_vulkan.c#L1352 >> to "load_functions(ctx, 1, 1)". The problem is unseen. >> > > Thanks for testing, fix pushed. > Out of curiosity, are you planning on contributing to the Vulkan code? ___ 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/2] hwcontext_vulkan: dynamically load functions
Jun 10, 2021, 03:38 by wenbin.c...@intel.com: >> Jun 8, 2021, 07:38 by wenbin.c...@intel.com: >> >> >> Apr 29, 2021, 03:52 by d...@lynne.ee: >> >> >> >> > This patch allows for alternative loader implementations. >> >> > >> >> > Patch attached. >> >> > >> >> >> >> Forgot to fix a flag, v2 attached. >> >> >> > >> > Hi Lynne: >> > I tried the following command: >> > "ffmpeg -init_hw_device vulkan=vul:0 -filter_hw_device vul -i 1080p.264 -vf >> "hwupload=extra_hw_frames=16,scale_vulkan=w=1920:h1080" -f null" >> > It report a segmentation fault at >> https://github.com/FFmpeg/FFmpeg/blob/282b9f4cba7ca361c43ac9f94031a >> 43109df0a4f/libavutil/hwcontext_vulkan.c#L1018 >> > The function pointer vk->CreateCommandPool is NULL. >> > I build on ffmpeg master. >> > >> >> Weird. What Vulkan drivers and what OS was used? >> If CreateCommandPool is NULL then initialization probably >> wouldn't even happen. >> > > I use linux ubuntu 20.04 and I get the vulkan driver from: > https://www.lunarg.com/vulkan-sdk/. > I see the load_functions() is called three times, but none of them set > "has_dev" argument to 1. > I tried to change the code in > https://github.com/FFmpeg/FFmpeg/blob/591b88e6787c4e678237f02a50421d101abd25c2/libavutil/hwcontext_vulkan.c#L1352 > to "load_functions(ctx, 1, 1)". The problem is unseen. > Thanks for testing, fix pushed. ___ 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 25/35] avutil/opt: add av_opt_to_string
On Thu, Jun 10, 2021 at 11:39 AM Diederick C. Niehorster
wrote:
> So in av_opt_get(), I'd do something like this:
> AVBPrint buf;
> av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
> // 1. call internal print function, with &buf
> // ...
> // 2. provide output to caller
> if (!av_bprint_is_complete(&buf))
> {
> av_bprint_finalize(&buf,NULL);
> return AVERROR(ENOMEM);
> }
> ret = av_bprint_finalize(&buf,out_val);
> return ret<0 ? ret : 0;
>
> I'll experiment with that.
Ok, i have experimented with this, and hit some problems quickly.
There are at least the following issues:
1. how to deal with AV_OPT_ALLOW_NULL in av_opt_get, if its guts use a
move over to AVBPrint? leaving AVBPrint empty is not the same as a
null, and does not signal null.
2. some cases forward to other functions, most importantly
AV_OPT_TYPE_DICT, which uses av_dict_get_string. These do not play
with AVBPrint, but just plain char** or similar. Should i use a local
buffer in that case and then copy/print its contents into the
AVBPrint? That seems to not improve the situation.
This is assuming we'd want to reuse the guts of av_opt_get for this
new function, which i think we would. Not doing so would mean a lot of
code duplication.
And there is also that this would be the only av_opt_* function
returning output through a AVBPrint instead of plain arrays. Is that
inconsistency in the API worth usually avoiding a dynamic allocation
in code that'll not be used in a critical path?
Cheers,
Dee
___
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 25/35] avutil/opt: add av_opt_to_string
(NB: I have reorganized your reply a bit to make it for me to respond to)
On Wed, Jun 9, 2021 at 1:54 PM Nicolas George wrote:
>
> The critical part is the API of the new public function, because this we
> cannot fix later.
>
> Unfortunately, to get a good API, you will not be able to reuse the
> implementation of av_opt_get() as it is. Therefore, you will probably
> need two changes: (1) change the implementation of av_opt_get() and (2)
> add the new function.
>
> Whether this should be done in one or two patches depends on the
> specifics, on the amount of code in each change and how much they cover
> each other.
>
> In the meantime, as I suggested, I think using AVBPrint is the best
> option:
>
> void av_num_opt_bprint(AVBPrint *bp, AVOptionType type, double val);
Ok, see if i understand this correctly. To use AVBPrint in this new
API (which when reading up on it, seems like a good idea since it does
something similar to C++'s std::string's small string optimization,
and many string here will be very short, so no dynamic allocations), i
would need to change the guts of av_opt_get() that i would like to
reuse to take an AVBPrint. That would be pretty similar to the current
uint8_t buf[128] currently used in the function, but remove the
arbitrary (if rarely hit) size limit.
What is the size of the internal buffer of AVBPrint?
So in av_opt_get(), I'd do something like this:
AVBPrint buf;
av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
// 1. call internal print function, with &buf
// ...
// 2. provide output to caller
if (!av_bprint_is_complete(&buf))
{
av_bprint_finalize(&buf,NULL);
return AVERROR(ENOMEM);
}
ret = av_bprint_finalize(&buf,out_val);
return ret<0 ? ret : 0;
I'll experiment with that.
>
> I have long-term projects of rewriting the options system, with each
> type (channel layouts, pixel formats, color ranges, etc.) coming with a
> descriptor for printing and parsing it, and a de-centralized way of
> adding new types. Unfortunately, first I need to convince people here
> that we need a better strings API
That sounds nice. Lets keep things here as they are then. Have you
laid out your plans somewhere i can read about them?
Cheers,
Dee
___
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] Stop using _explicit atomic operations where not necessary.
Quoting Andreas Rheinhardt (2021-06-06 19:20:38) > Anton Khirnov: > > Quoting Andreas Rheinhardt (2021-06-06 11:13:00) > >> Anton Khirnov: > >>> Memory ordering constraints other than the default (sequentially > >>> consistent) can behave in very unintuitive and unexpected ways, and so > >>> should be avoided unless there is a strong (typically performance) > >>> reason for using them. This holds especially for memory_order_relaxed, > >>> which imposes no ordering constraints. > >> > >> I disagree with this: Adding ordering constraints is wrong, as it can > >> provide synchronization between different threads and this can mask > >> races. Providing synchronization as a byproduct should be avoided: Users > >> should know what they get. And if a user wants an allocation limit in > >> thread A to be in effect for allocations in thread B, then the user > >> should provide the synchronization him/herself. > > > > I do not agree. C11 default is sequential consistency, therefore our > > public API should provide sequential consistency. > > > > 1. The atomics here are not part of the public API at all: The > documentation of none of the functions involved mentions atomics or > synchronization etc. at all (which means that if a user wants > synchronization between threads, he should take care of it himself). We > could change the implementation to not use atomics at all (e.g. by > wrapping everything in mutexes, which needn't be sequentially consistent). > (Furthermore, in case of libavformat/fifo.c this does not even come > close to affecting the public API, given that we are talking about > synchronization between the user thread and a thread owned by us; in > case of av_cpu_count() the atomics only exist because of an av_log(), > there is no reason for a user to believe that synchronization is even > involved.) > 2. I don't buy the argument: > Which memory ordering to use should be decided based upon what one wants > and needs; one should not blindly use one because it is the default or > recommended by some authority (doing so smells like cargo cult). That is an overly idealistic view of the situation. I think very few people around here have an indepth understanding of concurrency issues, e.g. of the difference between using memory_order_seq_cst and memory_order_relaxed. This stuff is complicated and unintuitive, according to pretty much everyone who ever dealt with it. And you cannot expect that everyone will study this topic in detail just because they need to touch an atomic operation. So it seems perfectly reasonable to me to defer to a suitable authority (like the people who designed c11 atomics) and use their recommended defaults unless there is a strong reason to do otherwise. > In case > of av_get_cpu_flags()+av_force_cpu_flags(), all one wants is that one > can just load and write the value; given that there is no restriction > loads a valid value (i.e. no torn reads/writes) and one wants that one > can access this function by multiple threads simultaneously without > races. Atomics provide this even with memory_order_relaxed. In case of > av_cpu_count() all that is desired is that av_log() is only called once; > an atomic_exchange provides this automatically, even with > memory_order_relaxed. The av_max_alloc()+av_malloc/... combination is > just like av_get/force_cpu_flags(). > > > > My other concern is that people are cargo culting this code around > > without understanding what it really does. Again, sequentially > > consistent atomics make that safer, since they do what you'd expect them > > to. > > > > Are you really advocating that we write our code in such a way that > makes it safe for cargo-culters to copy it? Yes I am. People copy code around, that's just a fact of life. Two separate people told me recently that their patches used memory_order_relaxed only because they based them off some other code that was already using it. So IMO we should strive to write code that is not only safe and correct, but also robust against future modifications and being copied around. That includes not using potentially-unsafe operations without a good reason. There is also the question of code readability. When I see atomic_load(&foo); then it's just an atomic load. But when I see atomic_load_explicit(&foo, memory_order_bar); I also need to think about the reason the author chose to use that specific memory order. As for your example program, you could also just remove the av_log() from av_cpu_count(). Which should be done anyway because av_log(NULL is evil. -- Anton Khirnov ___ 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".
