Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid reads in forced keyframe parsing
Quoting Zhao Zhili (2023-03-11 12:45:30)
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of Anton
> > Khirnov
> > Sent: 2023年3月11日 18:37
> > To: 'FFmpeg development discussions and patches'
> > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid
> > reads in forced keyframe parsing
> >
> > Quoting Zhao Zhili (2023-03-10 15:44:56)
> > >
> > > > From: ffmpeg-devel On Behalf Of Anton
> > > > Khirnov
> > > > Sent: 2023年3月10日 21:46
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Subject: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid
> > > > reads in forced keyframe parsing
> > > >
> > > > Fixes #10243
> > > > ---
> > > > fftools/ffmpeg_mux_init.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
> > > > index b3cc502fdd..09d24ba8e5 100644
> > > > --- a/fftools/ffmpeg_mux_init.c
> > > > +++ b/fftools/ffmpeg_mux_init.c
> > > > @@ -2063,7 +2063,7 @@ static void
> > > > parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
> > > > if (next)
> > > > *next++ = 0;
> > > >
> > > > -if (!memcmp(p, "chapters", 8)) {
> > > > +if (strstr(p, "chapters") == p) {
> > >
> > > Does strncmp() more efficient in this case?
> >
> > I don't see the point of optimizing this code for speed. A strncmp call
> > is longer and less readable IMO.
>
> This is a case for the need of a strstarts(). strncmp is more intuitive than
> strstr()
> for this job.
Yes, having a "string starts with" function in stdlib would have been
nice. But as it's not there, we have to make do with what we have.
--
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".
Re: [FFmpeg-devel] [PATCH 2/6] fftools/ffprobe: stop using AVFrame.pkt_{pos, size}
On date Friday 2023-03-10 12:56:31 +0100, Anton Khirnov wrote: > These fields are ad-hoc and will be deprecated. Use the recently-added > AV_CODEC_FLAG_COPY_OPAQUE to pass arbitrary user data from packets to > frames. > > Changes the result of the flcl1905 test, which uses ffprobe to decode > wmav2 with multiple frames per packet. Such packets are handled > internally by calling the decoder's decode callback multiple times, > offsetting the internal packet's data pointer and decreasing its size > after each call. The output pkt_size value before this commit is then > the remaining internal packet size at the time of each internal decode > call. > > After this commit, output pkt_size is simply the size of the full packet > submitted by the caller to the decoder. This is more correct, since > internal packets are never seen by the caller and should have no > observable outside effects. > --- > fftools/ffprobe.c | 26 +++- > tests/ref/fate/flcl1905 | 318 > 2 files changed, 181 insertions(+), 163 deletions(-) LGTM, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/6] fftools/ffplay: drop an unused function argument
On date Friday 2023-03-10 12:56:32 +0100, Anton Khirnov wrote:
> ---
> fftools/ffplay.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index d6479aef5f..f09ff59ccc 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1553,7 +1553,8 @@ static double vp_duration(VideoState *is, Frame *vp,
> Frame *nextvp) {
> }
> }
>
> -static void update_video_pts(VideoState *is, double pts, int64_t pos, int
> serial) {
> +static void update_video_pts(VideoState *is, double pts, int serial)
> +{
> /* update current video pts */
> set_clock(&is->vidclk, pts, serial);
> sync_clock_to_slave(&is->extclk, &is->vidclk);
> @@ -1618,7 +1619,7 @@ retry:
>
> SDL_LockMutex(is->pictq.mutex);
> if (!isnan(vp->pts))
> -update_video_pts(is, vp->pts, vp->pos, vp->serial);
> +update_video_pts(is, vp->pts, vp->serial);
> SDL_UnlockMutex(is->pictq.mutex);
>
> if (frame_queue_nb_remaining(&is->pictq) > 1) {
LGTM.
___
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 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/9/2023 11:18 AM, Raphaël Zumer wrote: Hi, While I omitted adding v2/v3 here, I believe all comments on this set of patches have been addressed so far, unless anyone strongly disagrees with the rationale for moving dynamic HDR parsing and serialization to libavutil or with the function signature. Please let me know if I missed anything. Thanks, Raphaël Zumer I'll apply this (and patch 1/1) in a few days if nobody comments. The code needs to be in lavu if we want muxers and demuxers to use this functionality, so moving the existing lavc functions is fine unless we add more avpriv_ functions that people tend to dislike. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavc/avcodec.h: extend documentation for avcodec_open2()
On date Sunday 2023-03-05 12:57:38 +0100, Stefano Sabatini wrote: > On date Wednesday 2023-03-01 15:04:16 +0800, "zhilizhao(赵志立)" wrote: [...] > Updated, thanks for the feedback. > From 672de0e5d15ab8a22f3957222680d847f60bced8 Mon Sep 17 00:00:00 2001 > From: Stefano Sabatini > Date: Tue, 28 Feb 2023 23:26:43 +0100 > Subject: [PATCH 1/2] lavc/avcodec.h: extend documentation for avcodec_open2() > > In particular, clarify how to set options in the codec context, and > mention when to use avcodec_parameters_to_context(). > > Fix trac issues: > http://trac.ffmpeg.org/ticket/5781 > http://trac.ffmpeg.org/ticket/5838 > --- > libavcodec/avcodec.h | 36 +++- > 1 file changed, 31 insertions(+), 5 deletions(-) Applied with a few minor fixes. ___ 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 2/2] avutil: add HDR10+ dynamic metadata serialization function
> From: ffmpeg-devel On Behalf Of Raphaël
> Zumer
> Sent: 2023年3月3日 5:43
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata
> serialization function
>
> Fixed brace style and moved inline buffer size calculation comments to a
> single block at the top.
>
>
> Signed-off-by: Raphaël Zumer
> ---
> libavutil/hdr_dynamic_metadata.c | 142 +++
> libavutil/hdr_dynamic_metadata.h | 11 +++
> libavutil/version.h | 2 +-
> 3 files changed, 154 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/hdr_dynamic_metadata.c
> b/libavutil/hdr_dynamic_metadata.c
> index 98f399b032..24dd3dab2d 100644
> --- a/libavutil/hdr_dynamic_metadata.c
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -225,3 +225,145 @@ int av_dynamic_hdr_plus_from_t35(AVDynamicHDRPlus *s,
> const uint8_t *data,
>
> return 0;
> }
> +
> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s)
> +{
How about add 'const' modifier to AVDynamicHDRPlus *s ?
> +AVBufferRef *buf;
> +size_t size_bits, size_bytes;
> +PutBitContext pbc, *pb = &pbc;
> +
> +if (!s)
> +return NULL;
> +
> +/**
> + * Buffer size per CTA-861-H p.253-254:
> + * 48 bits for the header (56 minus excluded 8-bit country code)
> + * 2 bits for num_windows
> + * 937 bits for window geometry, for each window above 1
> + * 27 bits for targeted_system_display_maximum_luminance
> + * 1-3855 bits for targeted system display peak luminance information
> + * 0-442 bits for intra-window pixel distribution information
> + * 1-3855 bits for mastering display peak luminance information
> + * 0-537 bits for per-window tonemapping information
> + * 0-21 bits for per-window color saturation mapping information
> + */
> +size_bits = 48 +
> +2 +
> +FFMAX((s->num_windows - 1), 0) * 937 +
> +27 +
> +1 + (s->targeted_system_display_actual_peak_luminance_flag ? 10 +
> +s->num_rows_targeted_system_display_actual_peak_luminance *
> +s->num_cols_targeted_system_display_actual_peak_luminance * 4 :
> 0) +
> +s->num_windows * 82;
> +
> +for (int w = 0; w < s->num_windows; w++)
> +size_bits += s->params[w].num_distribution_maxrgb_percentiles * 24;
> +
> +size_bits += 1 + (s->mastering_display_actual_peak_luminance_flag ? 10 +
> +s->num_rows_mastering_display_actual_peak_luminance *
> +s->num_cols_mastering_display_actual_peak_luminance * 4 : 0) +
> +s->num_windows * 1;
> +
> +for (int w = 0; w < s->num_windows; w++) {
> +if (s->params[w].tone_mapping_flag)
> +size_bits += 28 + s->params[w].num_bezier_curve_anchors * 10;
> +}
> +
> +size_bits += s->num_windows * 1;
> +for (int w = 0; w < s->num_windows; w++) {
> +if (s->params[w].color_saturation_mapping_flag)
> +size_bits += 6;
> +}
> +
> +size_bytes = (size_bits + 7) / 8;
> +
> +buf = av_buffer_alloc(size_bytes);
> +if (!buf)
> +return NULL;
> +
> +init_put_bits(pb, buf->data, size_bytes);
> +
> +// itu_t_t35_country_code shall be 0xB5 (USA) (excluded from the payload)
> +// itu_t_t35_terminal_provider_code shall be 0x003C
> +put_bits(pb, 16, 0x003C);
> +// itu_t_t35_terminal_provider_oriented_code is set to ST 2094-40
> +put_bits(pb, 16, 0x0001);
> +// application_identifier shall be set to 4
> +put_bits(pb, 8, 4);
> +// application_mode is set to Application Version 1
> +put_bits(pb, 8, 1);
> +
> +// Payload as per CTA-861-H p.253-254
> +put_bits(pb, 2, s->num_windows);
> +
> +for (int w = 1; w < s->num_windows; w++) {
> +put_bits(pb, 16, s->params[w].window_upper_left_corner_x.num /
> s->params[w].window_upper_left_corner_x.den);
> +put_bits(pb, 16, s->params[w].window_upper_left_corner_y.num /
> s->params[w].window_upper_left_corner_y.den);
> +put_bits(pb, 16, s->params[w].window_lower_right_corner_x.num /
> s->params[w].window_lower_right_corner_x.den);
> +put_bits(pb, 16, s->params[w].window_lower_right_corner_y.num /
> s->params[w].window_lower_right_corner_y.den);
> +put_bits(pb, 16, s->params[w].center_of_ellipse_x);
> +put_bits(pb, 16, s->params[w].center_of_ellipse_y);
> +put_bits(pb, 8, s->params[w].rotation_angle);
> +put_bits(pb, 16, s->params[w].semimajor_axis_internal_ellipse);
> +put_bits(pb, 16, s->params[w].semimajor_axis_external_ellipse);
> +put_bits(pb, 16, s->params[w].semiminor_axis_external_ellipse);
> +put_bits(pb, 1, s->params[w].overlap_process_option);
> +}
> +
> +put_bits(pb, 27, s->targeted_system_display_maximum_luminance.num *
> luminance_den /
> +s->targeted_system_display_maximum_luminance.den);
> +put_bits(pb, 1, s->targeted_system_display_actual_peak_luminance_flag);
> +if (s->targeted_syst
[FFmpeg-devel] [PATCH 1/3] avcodec/libxavs: Use frame_num instead of frame_number
Forgotten in 6b6f7db81932f94876ff4bcfd2da0582b8ab897e. Signed-off-by: Andreas Rheinhardt --- libavcodec/libxavs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/libxavs.c b/libavcodec/libxavs.c index 9ed73d1042..6c29539f24 100644 --- a/libavcodec/libxavs.c +++ b/libavcodec/libxavs.c @@ -141,7 +141,7 @@ static int XAVS_frame(AVCodecContext *avctx, AVPacket *pkt, x4->pic.i_pts = frame->pts; x4->pic.i_type = XAVS_TYPE_AUTO; -x4->pts_buffer[avctx->frame_number % (avctx->max_b_frames+1)] = frame->pts; +x4->pts_buffer[avctx->frame_num % (avctx->max_b_frames+1)] = frame->pts; } if (xavs_encoder_encode(x4->enc, &nal, &nnal, -- 2.34.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 2/3] avcodec/libopencore-amr: Use frame_number instead of frame_num
Forgotten in 6b6f7db81932f94876ff4bcfd2da0582b8ab897e. Signed-off-by: Andreas Rheinhardt --- libavcodec/libopencore-amr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/libopencore-amr.c b/libavcodec/libopencore-amr.c index fd9e6e6343..641a156129 100644 --- a/libavcodec/libopencore-amr.c +++ b/libavcodec/libopencore-amr.c @@ -106,8 +106,8 @@ static int amr_nb_decode_frame(AVCodecContext *avctx, AVFrame *frame, enum Mode dec_mode; int packet_size, ret; -ff_dlog(avctx, "amr_decode_frame buf=%p buf_size=%d frame_count=%d!!\n", -buf, buf_size, avctx->frame_number); +ff_dlog(avctx, "amr_decode_frame buf=%p buf_size=%d frame_count=%"PRId64"!!\n", +buf, buf_size, avctx->frame_num); /* get output buffer */ frame->nb_samples = 160; -- 2.34.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/libvpxdec: Constify VP9-decoder
Possible since 8d226fb9786f34760e80e0d6b403bd63e9ac4ddd.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/allcodecs.c | 2 +-
libavcodec/libvpxdec.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index e593ad19af..385ee34803 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -798,7 +798,7 @@ extern const FFCodec ff_libvorbis_decoder;
extern const FFCodec ff_libvpx_vp8_encoder;
extern const FFCodec ff_libvpx_vp8_decoder;
extern FFCodec ff_libvpx_vp9_encoder;
-extern FFCodec ff_libvpx_vp9_decoder;
+extern const FFCodec ff_libvpx_vp9_decoder;
/* preferred over libwebp */
extern const FFCodec ff_libwebp_anim_encoder;
extern const FFCodec ff_libwebp_encoder;
diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
index 8e6291fe20..f480545ae0 100644
--- a/libavcodec/libvpxdec.c
+++ b/libavcodec/libvpxdec.c
@@ -377,7 +377,7 @@ static av_cold int vp9_init(AVCodecContext *avctx)
return vpx_init(avctx, &ctx->decoder, vpx_codec_vp9_dx());
}
-FFCodec ff_libvpx_vp9_decoder = {
+const FFCodec ff_libvpx_vp9_decoder = {
.p.name = "libvpx-vp9",
CODEC_LONG_NAME("libvpx VP9"),
.p.type = AVMEDIA_TYPE_VIDEO,
--
2.34.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] avcodec/avutil: move dynamic HDR metadata parsing to libavutil
On 2/27/2023 2:33 PM, Raphaël Zumer wrote: Resending this patch set due to my mail client messing with the line wrapping in the messages I sent earlier today. Below is a copy of the initial explanation. This patch set implements serialization for HDR10+ dynamic metadata (AVDynamicHDRPlus), which is the inverse operation of the existing ff_parse_itu_t_t35_to_dynamic_hdr10_plus() function. It also moves both functions from libavcodec to libavutil and makes them public. For consistency, the equivalent vivid HDR parsing function is also migrated, but I did not implement serialization for it. Finally, the patch renames those functions to av_dynamic_hdr_plus_from_t35() (for parsing) and av_dynamic_hdr_plus_to_t35 (for serialization), with the equivalent change being made for vivid as well. The motivation for this change is to allow users to easily convert HDR10+ side data (which is parsed into AVDynamicHDRPlus) to a standard ITU-T T.35 payload that can be passed directly to applications that expect HDR10+ dynamic metadata in that format (e.g. x265 and rav1e encoders). The return value of the serialization function is AVBufferRef*, which I expect to be contentious. Payload size is not embedded in the T.35 data, so it must be calculated, used to allocate a buffer, and returned along with that buffer to the user. As far as I'm aware, AVBufferRef is the simplest way to do that, but I will be happy to consider alternative solutions. Please let me know if it is preferred to bump libavutil with the first commit, or with both of them, considering there are public API changes associated with each one. Raphaël Zumer Could you resend this patch and the last version of second one in a new set with proper commit messages for each of them? The text above would apply to a cover letter (patch 0/2) as it mentions the changes for the whole set plus review requests, instead of just the changes for this specific patch. The single version bump in the second patch is ok, but you should also add an APIChanges entry to it listing all three functions you added. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: Check input size
On Thu, Dec 01, 2022 at 12:08:44AM +0100, Michael Niedermayer wrote: > Fixes: Timeout > Fixes: > 53599/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IPU_fuzzer-4950102511058944 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/mpeg12dec.c | 4 > 1 file changed, 4 insertions(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. 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 1/3] avcodec/escape124: fix signdness of end of input check
On Mon, Mar 06, 2023 at 12:36:52AM +0100, Michael Niedermayer wrote: > Fixes: Timeout > Fixes: > 56561/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ESCAPE124_fuzzer-5560363635834880 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/escape124.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply patchset [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB What does censorship reveal? It reveals fear. -- Julian Assange 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] avutil/frame: move wipe_side_data counter to its utilized scope
Originally in 77b2cd7b41d7ec8008b6fac753c04f77824c514c this
counter was separate in av_frame_unref, in which the same counter
was re-utilized multiple times over multiple loops.
This code was then refactored into wipe_side_data as-is in
5d839778b9f3edb682b7f71dde4f80f07c75b098 , keeping the location of
counter initialization. This was unnecessary, as the counter was
no longer utilized outside of the for loop's scope and thus could
reside within the loop itself.
---
libavutil/frame.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 9545477acc..d06a673779 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -74,9 +74,7 @@ static void free_side_data(AVFrameSideData **ptr_sd)
static void wipe_side_data(AVFrame *frame)
{
-int i;
-
-for (i = 0; i < frame->nb_side_data; i++) {
+for (int i = 0; i < frame->nb_side_data; i++) {
free_side_data(&frame->side_data[i]);
}
frame->nb_side_data = 0;
--
2.39.2
___
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 2/2] avutil: add HDR10+ dynamic metadata serialization function
Quoting Raphaël Zumer (2023-03-02 22:43:29) > +/** > + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 > buffer, > + * excluding the country code and beginning with the terminal provider code. > + * @param s A pointer containing the decoded AVDynamicHDRPlus structure. > + * > + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 > representation > + * of the HDR10+ metadata if succeed, or NULL if buffer allocation > fails. > + */ > +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s); Why is this an AVBufferRef rather than a plain av_malloced() uint8_t array? You can very easily turn the latter into the former, but the reverse is a lot more annoying. -- 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".
Re: [FFmpeg-devel] [PATCH 2/2] avutil: add HDR10+ dynamic metadata serialization function
I expanded on this in another email in the chain, but the buffer size needs to be communicated to the user, as it is not embedded in the payload. It seems needlessly convoluted to me to create a separate function solely to calculate the size of the buffer so that it can be allocated by the user and passed to the serialization function, and I cannot think of another solution that would not be even more convoluted and awkward for the user. I don't understand how going from AVBufferRef to uint8_t* is more complicated than the reverse. The buffer in the AVBufferRef is allocated via av_malloc() and is directly accessible through the data field. Am I missing some detail? Raphaël Zumer On 3/12/23 15:48, Anton Khirnov wrote: > Quoting Raphaël Zumer (2023-03-02 22:43:29) >> +/** >> + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 >> buffer, >> + * excluding the country code and beginning with the terminal provider code. >> + * @param s A pointer containing the decoded AVDynamicHDRPlus structure. >> + * >> + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 >> representation >> + * of the HDR10+ metadata if succeed, or NULL if buffer allocation >> fails. >> + */ >> +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s); > Why is this an AVBufferRef rather than a plain av_malloced() uint8_t > array? You can very easily turn the latter into the former, but the > reverse is a lot more annoying. > ___ 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] avfilter/vf_ssim360: Use correct type in sizeof
SSIM360Context.ssim360_hist is an array of four pointers to double;
so sizeof(*ssim360_hist[0]) (=sizeof(double)) is the correct size
to use to calculate the amount of memory to allocate, not
sizeof(*ssim360_hist) (which is sizeof(double*)).
Use FF_ALLOCZ_TYPED_ARRAY to avoid this issue altogether.
Fixes Coverity issue #1520671.
Signed-off-by: Andreas Rheinhardt
---
libavfilter/vf_ssim360.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavfilter/vf_ssim360.c b/libavfilter/vf_ssim360.c
index 3eb8e43bbc..f8ce0744f2 100644
--- a/libavfilter/vf_ssim360.c
+++ b/libavfilter/vf_ssim360.c
@@ -1624,7 +1624,7 @@ static int config_output(AVFilterLink *outlink)
memset(s->ssim360_percentile_sum, 0,
sizeof(s->ssim360_percentile_sum));
for (int i = 0; i < s->nb_components; i++) {
-s->ssim360_hist[i] = av_calloc(SSIM360_HIST_SIZE,
sizeof(*s->ssim360_hist));
+FF_ALLOCZ_TYPED_ARRAY(s->ssim360_hist[i], SSIM360_HIST_SIZE);
if (!s->ssim360_hist[i])
return AVERROR(ENOMEM);
}
--
2.34.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 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/12/2023 6:50 PM, Raphaël Zumer wrote: I expanded on this in another email in the chain, but the buffer size needs to be communicated to the user, as it is not embedded in the payload. It seems needlessly convoluted to me to create a separate function solely to calculate the size of the buffer so that it can be allocated by the user and passed to the serialization function, and I cannot think of another solution that would not be even more convoluted and awkward for the user. I don't understand how going from AVBufferRef to uint8_t* is more complicated than the reverse. The buffer in the AVBufferRef is allocated via av_malloc() and is directly accessible through the data field. Am I missing some detail? Raphaël Zumer You can do it like how the AVDynamicHDRPlus struct is allocated and its size returned to the user in av_dynamic_hdr_plus_alloc(). That is uint8_t *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s, size_t *size); The function would then store the calculated size of the array on *size. On 3/12/23 15:48, Anton Khirnov wrote: Quoting Raphaël Zumer (2023-03-02 22:43:29) +/** + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer, + * excluding the country code and beginning with the terminal provider code. + * @param s A pointer containing the decoded AVDynamicHDRPlus structure. + * + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation + * of the HDR10+ metadata if succeed, or NULL if buffer allocation fails. + */ +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s); Why is this an AVBufferRef rather than a plain av_malloced() uint8_t array? You can very easily turn the latter into the former, but the reverse is a lot more annoying. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] avfilter/vf_ssim360: Remove dead code
Fixes Coverity issue #1520669.
Signed-off-by: Andreas Rheinhardt
---
libavfilter/vf_ssim360.c | 4
1 file changed, 4 deletions(-)
diff --git a/libavfilter/vf_ssim360.c b/libavfilter/vf_ssim360.c
index f8ce0744f2..5794275c2c 100644
--- a/libavfilter/vf_ssim360.c
+++ b/libavfilter/vf_ssim360.c
@@ -1274,10 +1274,6 @@ static int parse_heatmaps(void *logctx, HeatmapList
**proot,
ret = AVERROR(ENOMEM);
goto fail;
}
-if (!line) {
-av_freep(&line);
-break;
-}
// first value is frame id
av_strtok(line, ",", &saveptr);
--
2.34.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/libx264: Fix leak in case of allocation failure
Fixes Coverity issue #1518906.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/libx264.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index f65ac5dacc..e59939a8a7 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -503,6 +503,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
if (sei_data) {
pic->extra_sei.payloads =
av_mallocz(sizeof(pic->extra_sei.payloads[0]));
if (pic->extra_sei.payloads == NULL) {
+av_free(sei_data);
ret = AVERROR(ENOMEM);
goto fail;
}
--
2.34.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 2/2] avutil: add HDR10+ dynamic metadata serialization function
On 3/12/23 17:52, James Almer wrote: > On 3/12/2023 6:50 PM, Raphaël Zumer wrote: >> I expanded on this in another email in the chain, but the buffer size needs >> to be communicated to the user, as it is not embedded in the payload. It >> seems needlessly convoluted to me to create a separate function solely to >> calculate the size of the buffer so that it can be allocated by the user and >> passed to the serialization function, and I cannot think of another solution >> that would not be even more convoluted and awkward for the user. >> >> I don't understand how going from AVBufferRef to uint8_t* is more >> complicated than the reverse. The buffer in the AVBufferRef is allocated via >> av_malloc() and is directly accessible through the data field. Am I missing >> some detail? >> >> Raphaël Zumer > You can do it like how the AVDynamicHDRPlus struct is allocated and its > size returned to the user in av_dynamic_hdr_plus_alloc(). That is > > uint8_t *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s, size_t *size); > > The function would then store the calculated size of the array on *size. OK great, I will do that and address the remaining comments tomorrow. Thanks >> On 3/12/23 15:48, Anton Khirnov wrote: >>> Quoting Raphaël Zumer (2023-03-02 22:43:29) +/** + * Serialize dynamic HDR10+ metadata to a user data registered ITU-T T.35 buffer, + * excluding the country code and beginning with the terminal provider code. + * @param s A pointer containing the decoded AVDynamicHDRPlus structure. + * + * @return Pointer to an AVBufferRef containing the raw ITU-T T.35 representation + * of the HDR10+ metadata if succeed, or NULL if buffer allocation fails. + */ +AVBufferRef *av_dynamic_hdr_plus_to_t35(AVDynamicHDRPlus *s); >>> Why is this an AVBufferRef rather than a plain av_malloced() uint8_t >>> array? You can very easily turn the latter into the former, but the >>> reverse is a lot more annoying. >>> >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/frame: move wipe_side_data counter to its utilized scope
Jan Ekström:
> Originally in 77b2cd7b41d7ec8008b6fac753c04f77824c514c this
> counter was separate in av_frame_unref, in which the same counter
> was re-utilized multiple times over multiple loops.
>
> This code was then refactored into wipe_side_data as-is in
> 5d839778b9f3edb682b7f71dde4f80f07c75b098 , keeping the location of
> counter initialization. This was unnecessary, as the counter was
> no longer utilized outside of the for loop's scope and thus could
> reside within the loop itself.
> ---
> libavutil/frame.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 9545477acc..d06a673779 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -74,9 +74,7 @@ static void free_side_data(AVFrameSideData **ptr_sd)
>
> static void wipe_side_data(AVFrame *frame)
> {
> -int i;
> -
> -for (i = 0; i < frame->nb_side_data; i++) {
> +for (int i = 0; i < frame->nb_side_data; i++) {
> free_side_data(&frame->side_data[i]);
> }
> frame->nb_side_data = 0;
Don't create a patch for a single for loop; do this for all for loops in
this file where it is possible.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] avcodec/libmp3lame: Remove redundant av_packet_unref()
The AVPacket given to an encoder's encode callback
is unreferenced generically on error.
Signed-off-by: Andreas Rheinhardt
---
This stuff should probably be moved into the AudioFrameQueue
someday.
libavcodec/libmp3lame.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/libavcodec/libmp3lame.c b/libavcodec/libmp3lame.c
index 26e58baa3d..e119189f2a 100644
--- a/libavcodec/libmp3lame.c
+++ b/libavcodec/libmp3lame.c
@@ -280,17 +280,14 @@ static int mp3lame_encode_frame(AVCodecContext *avctx,
AVPacket *avpkt,
// Check if subtraction resulted in an overflow
if ((discard_padding < avctx->frame_size) != (avpkt->duration > 0)) {
av_log(avctx, AV_LOG_ERROR, "discard padding overflow\n");
-av_packet_unref(avpkt);
return AVERROR(EINVAL);
}
if ((!s->delay_sent && avctx->initial_padding > 0) || discard_padding
> 0) {
uint8_t* side_data = av_packet_new_side_data(avpkt,
AV_PKT_DATA_SKIP_SAMPLES,
10);
-if(!side_data) {
-av_packet_unref(avpkt);
+if (!side_data)
return AVERROR(ENOMEM);
-}
if (!s->delay_sent) {
AV_WL32(side_data, avctx->initial_padding);
s->delay_sent = 1;
--
2.34.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 2/3] avcodec/libfdk-aacenc: Remove redundant av_packet_unref()
The AVPacket given to an encoder's encode callback
is unreferenced generically on error.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/libfdk-aacenc.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/libavcodec/libfdk-aacenc.c b/libavcodec/libfdk-aacenc.c
index db5b0841e0..eb97e0fb41 100644
--- a/libavcodec/libfdk-aacenc.c
+++ b/libavcodec/libfdk-aacenc.c
@@ -475,16 +475,13 @@ static int aac_encode_frame(AVCodecContext *avctx,
AVPacket *avpkt,
// Check if subtraction resulted in an overflow
if ((discard_padding < avctx->frame_size) != (avpkt->duration > 0)) {
av_log(avctx, AV_LOG_ERROR, "discard padding overflow\n");
-av_packet_unref(avpkt);
return AVERROR(EINVAL);
}
if ((!s->delay_sent && avctx->initial_padding > 0) || discard_padding > 0)
{
uint8_t *side_data =
av_packet_new_side_data(avpkt, AV_PKT_DATA_SKIP_SAMPLES, 10);
-if (!side_data) {
-av_packet_unref(avpkt);
+if (!side_data)
return AVERROR(ENOMEM);
-}
if (!s->delay_sent) {
AV_WL32(side_data, avctx->initial_padding);
s->delay_sent = 1;
--
2.34.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/libopusenc: Remove redundant av_packet_unref()
The AVPacket given to an encoder's encode callback
is unreferenced generically on error.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/libopusenc.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/libavcodec/libopusenc.c b/libavcodec/libopusenc.c
index 75bc491c9e..5a0786f32f 100644
--- a/libavcodec/libopusenc.c
+++ b/libavcodec/libopusenc.c
@@ -512,18 +512,14 @@ static int libopus_encode(AVCodecContext *avctx, AVPacket
*avpkt,
discard_padding = opus->opts.packet_size - avpkt->duration;
// Check if subtraction resulted in an overflow
-if ((discard_padding < opus->opts.packet_size) != (avpkt->duration > 0)) {
-av_packet_unref(avpkt);
+if ((discard_padding < opus->opts.packet_size) != (avpkt->duration > 0))
return AVERROR(EINVAL);
-}
if (discard_padding > 0) {
uint8_t* side_data = av_packet_new_side_data(avpkt,
AV_PKT_DATA_SKIP_SAMPLES,
10);
-if(!side_data) {
-av_packet_unref(avpkt);
+if (!side_data)
return AVERROR(ENOMEM);
-}
AV_WL32(side_data + 4, discard_padding);
}
--
2.34.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] How to use threads inside custom encoder
Ronald S. Bultje: > Hi, > > On Thu, Feb 23, 2023 at 1:28 PM Alex <3.1...@ukr.net> wrote: > >> Hi! >> I write custom encoder codec and want to use threads to speed up encoding >> process. I know what ffmpeg have frame level threads and slices threads, >> but in my case best option is to use frame level threads >> with FF_CODEC_ENCODE_CB() function. >> But I have couple of questions: >> >> 1) Then I add AV_CODEC_CAP_FRAME_THREADS flag to capabilities of my >> encoder then ffmpeg call init function of my encoder for each spawned >> threads (for example 9 times because I have 8 core cpu ). How to prevent >> this and call init function only once? (I need to reuse encoder context.) >> > > In frame threading, each "frame instance" has its own context. They can be > synchronized and use dependent data. See how other codecs do this, e.g. > h264 decoder (the concept is identical between encoder & decoder). You can, > for example, use avcodec->internal->is_copy for this. See > update_thread_context() for synchronization between threads. > This is absolutely wrong: The frame-threaded-encode API is very simple and is only intended for encoders with only intra packets. There is no way to indicate progress to threads encoding later packets. It is different from the decode API. - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] doc/encoders: Add av1 to qsv encoder's summary
From: Wenbin Chen Signed-off-by: Wenbin Chen --- doc/encoders.texi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/encoders.texi b/doc/encoders.texi index b02737b9df..d6dddc2bd5 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -3188,8 +3188,8 @@ recommended value) and do not set a size constraint. @section QSV Encoders -The family of Intel QuickSync Video encoders (MPEG-2, H.264, HEVC, JPEG/MJPEG -and VP9) +The family of Intel QuickSync Video encoders (MPEG-2, H.264, HEVC, JPEG/MJPEG, +VP9, AV1) @subsection Ratecontrol Method The ratecontrol method is selected as follows: -- 2.34.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".
