[FFmpeg-devel] Possibly a little bug in af_loudnorm.c
Hi All, at af_loudnorm.c in line number 188: double this, next, max_peak; max_peak is not initialized with 0. and could be contains any value or noise. I could be wrong but I think init with 0. should be better: double this, next, max_peak=0.; Best Regards jagad. ___ 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] Possibly a little bug in af_loudnorm.c
Please check the code:line 209 if (c == 0 || fabs(buf[index + c]) > max_peak) max_peak = fabs(buf[index + c]); 'max_peak' is initialized. On Sat, Aug 20, 2022 at 5:39 PM jagad hariseno wrote: > Hi All, > > at af_loudnorm.c in line number 188: > double this, next, max_peak; > > max_peak is not initialized with 0. and could be contains any value or > noise. > I could be wrong but I think init with 0. should be better: > > double this, next, max_peak=0.; > > Best Regards > > jagad. > ___ > 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] Possibly a little bug in af_loudnorm.c
yes you're right. Thanks for your fast response Best Regards jagad On Sat, Aug 20, 2022 at 4:53 PM Jack Waller wrote: > Please check the code:line 209 > > if (c == 0 || fabs(buf[index + c]) > max_peak) > max_peak = fabs(buf[index + c]); > > 'max_peak' is initialized. > > > On Sat, Aug 20, 2022 at 5:39 PM jagad hariseno > wrote: > > > Hi All, > > > > at af_loudnorm.c in line number 188: > > double this, next, max_peak; > > > > max_peak is not initialized with 0. and could be contains any value or > > noise. > > I could be wrong but I think init with 0. should be better: > > > > double this, next, max_peak=0.; > > > > Best Regards > > > > jagad. > > ___ > > 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".
[FFmpeg-devel] framepool width and potential patch for bug 9873
Hi, I reported https://trac.ffmpeg.org/ticket/9873 last week and have since been digging into the code to see if I can get a patch together. I've found the underlying issue in ff_frame_pool_video_init at line 77 of libavfilter/framepool.c: ret = av_image_fill_linesizes(pool->linesize, pool->format, FFALIGN(pool->width, align)); When creating frames for frei0r filtering an align value of 16 gets passed in. This works fine for widths like 400 where FFALIGN just returns 400 since it's a multiple of 16. But for values like 1800 it pushes the width up to 1808 which then generates linesizes for that width which breaks the frei0r requirement that the stride matches width * 4 since width is now too large. Initially I put together a patch that adds an extra parameter to ff_frame_pool_video_init that tells it just to use the width directly and not try to align it. This seemed a bit awkward though, especially when I tried to figure our why the code is even trying to make the width a multiple of the align value (which is effectively what it's doing) to start with. I can't make sense of why it's calling FFALIGN there at all and checking the commit history didn't provide any hints. If anything I would have thought that you would want to increase the width so that the final line size would be aligned, EG: FFALIGN(pool->width*4, align)/4, rather than the width itself. In any case I have patches that work both ways, either an extra flag to not do the aligned width or the FFALIGN(pool->width*4, align)/4 fix. The former I'm confident of but am not so sure about the latter since I don't understand why it's there to start with, nor whether it may break other filters. The fate tests all pass with it though on my system. It will certainly work with frei0r though since it requires width to be a multiple of 8 which will always give a line size that's aligned to 16. Any advice on which one I should submit would be great. thanks Brendan ___ 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] framepool width and potential patch for bug 9873
On 20.08.2022 13:21, Brendan Hack wrote: Hi, I reported https://trac.ffmpeg.org/ticket/9873 last week and have since been digging into the code to see if I can get a patch together. I've found the underlying issue in ff_frame_pool_video_init at line 77 of libavfilter/framepool.c: ret = av_image_fill_linesizes(pool->linesize, pool->format, FFALIGN(pool->width, align)); When creating frames for frei0r filtering an align value of 16 gets passed in. This works fine for widths like 400 where FFALIGN just returns 400 since it's a multiple of 16. But for values like 1800 it pushes the width up to 1808 which then generates linesizes for that width which breaks the frei0r requirement that the stride matches width * 4 since width is now too large. Not sure what you mean. The code you quoted does not touch width. It just bumps up the linesize, to align the start of every line on the desired align value. Initially I put together a patch that adds an extra parameter to ff_frame_pool_video_init that tells it just to use the width directly and not try to align it. This seemed a bit awkward though, especially That parameter already exists. It's called the align. when I tried to figure our why the code is even trying to make the width a multiple of the align value (which is effectively what it's doing) to start with. I can't make sense of why it's calling FFALIGN there at all and checking the commit history didn't provide any hints. If anything I would have thought that you would want to increase the width so that the final line size would be aligned, EG: FFALIGN(pool->width*4, align)/4, rather than the width itself. In any case I have patches that work both ways, either an extra flag to not do the aligned width or the FFALIGN(pool->width*4, align)/4 fix. The former I'm confident of but am not so sure about the latter since I don't understand why it's there to start with, nor whether it may break You mean you don't understand the concept of aligning lines? The whole point is that a lot of hardware instructions require the data they work on to be aligned properly. Hence the linesize needs to be larger than width, to make sure every line starts well aligned. other filters. The fate tests all pass with it though on my system. It will certainly work with frei0r though since it requires width to be a multiple of 8 which will always give a line size that's aligned to 16. Any advice on which one I should submit would be great. If frei0r tells it to align it to 16 bytes, but then fails because it expects it to be packed without gaps, it's just passing the wrong alignment, and should pass 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] framepool width and potential patch for bug 9873
On 20.08.2022 14:27, Brendan Hack wrote: Oh, I think I've got the wrong end of the stick here in regards to what align does in ff_frame_pool_video_init. Frei0r only needs the start of the buffer to be aligned to 16 bytes. It doesn't need each _line_ to be explicitly aligned to 16 bytes since its requirement that the width be a multiple of 8 enforces that (since every 8 pixels is 32 bytes). Now that sounds like the align should be 8, not 16 or 1, in order for frames with odd sizes still work? ___ 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] framepool width and potential patch for bug 9873
On Sat, Aug 20, 2022 at 2:30 PM Timo Rothenpieler wrote: > On 20.08.2022 14:27, Brendan Hack wrote: > > Oh, I think I've got the wrong end of the stick here in regards to what > > align does in ff_frame_pool_video_init. Frei0r only needs the start of > > the buffer to be aligned to 16 bytes. It doesn't need each _line_ to be > > explicitly aligned to 16 bytes since its requirement that the width be a > > multiple of 8 enforces that (since every 8 pixels is 32 bytes). > > Now that sounds like the align should be 8, not 16 or 1, in order for > frames with odd sizes still work? > IIRC michael have wrote fix for frei0r filter, dunno if its merged. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 2/4] ffmpeg: Add display_matrix option
On 18 Aug 2022, at 12:58, Gyan Doshi wrote: On 2022-08-17 05:55 pm, Anton Khirnov wrote: Quoting Gyan Doshi (2022-08-17 12:53:11) On 2022-08-17 02:35 pm, Anton Khirnov wrote: Quoting Gyan Doshi (2022-08-17 10:50:43) On 2022-08-17 01:48 pm, Anton Khirnov wrote: Quoting Thilo Borgmann (2022-08-16 20:48:57) Am 16.08.22 um 16:10 schrieb Anton Khirnov: Quoting Thilo Borgmann (2022-08-15 22:02:09) $subject -Thilo From fe2ff114cb004f897c7774753d9cf28298eba82d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Ekstr=C3=B6m?= Date: Mon, 15 Aug 2022 21:09:27 +0200 Subject: [PATCH v2 2/4] ffmpeg: Add display_matrix option This enables overriding the rotation as well as horizontal/vertical flip state of a specific video stream on the input side. Additionally, switch the singular test that was utilizing the rotation metadata to instead override the input display rotation, thus leading to the same result. --- I still don't see how it's better to squash multiple options into a single option. It requires all this extra infrastructure and in the end it's less user-friendly, because user-understandable things like rotation or flips are now hidden under "display matrix". How many users would know what a display matrix is? FWIW I think Gyan's request to do this all in one option that effect one thing (the display matrix) is valid. I don't. It may be one thing internally, but modeling user interfaces based on internal representation is a sinful malpractice. More importantly, I see no advantage from doing it - it only makes the option parsing more complicated. It's not based on ffmpeg's 'internal representation'. All transform attributes are stored as a composite in one mathematical object. Keyword "stored". It is internal representation. Users should not care how it is stored, the entire point point of our project is to shield them from that as much as possible. Evaluating the matrix values will need to look at all sources of contribution. So gathering and presenting all these attributes in a single option (+ docs) makes it clearer to the user at the cost of an initial learning curve. Are you seriously expecting all users who want to mark a video as rotated or flipped to learn about display matrices? They don't need to know how to encode or decode the matrix if they don't want to. Only that it is the container. The difference is between -rotate:v:0 90 -hflip:v:0 1 -scale:v:0 2 and -display_matrix:v:0 rotate=90:hflip=1:scale=2 The latter syntax is all too familiar to users from AVFrame filters and BSFs. The syntax similarity is misleading - filters are applied in the order you list them, while these options are always applied in fixed order. The analogous filters are also called rotate, [vf]flip, and scale - there is no display_matrix filter. The display matrix is effected as a single matrix multiplication to obtain output pixel co-ordinates which incorporates all the encoded transforms so it is analogous to multiple options within a filter like eq or hue, not multiple filters. About SEI messaging, the h264 metadata BSF still obtains (and extracts) those attributes as a display matrix as that is the internal messaging format regardless of ultimate storage form. Thanks for all your comments! Unfortunately, I don’t see real consensus emerging here. I see a single option finds more acceptance (3:1), I see using to use AVDict is frowned upon by 1, though the alternative suggestion with a parser for SVG-style (new syntax) is not backup up by s.o. else. Therefore my interpretation would be to go with majority and stick with one option and stick with AVDict. However, I don’t want to shortcut the discussion or override s.o. opinion. Going to pick this up next week and if no more arguments emerge, I’ll continue with that. Thanks, Thilo ___ 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 2/4] ffmpeg: Add display_matrix option
Thilo Borgman (12022-08-20): > suggestion with a parser for SVG-style (new syntax) is not backup up by > s.o. else. I feel it was somewhat drowned by the rest of the discussion. Do YOU like it? Regards, -- Nicolas George 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 v2 2/4] ffmpeg: Add display_matrix option
Am 20.08.22 um 15:39 schrieb Nicolas George: Thilo Borgman (12022-08-20): suggestion with a parser for SVG-style (new syntax) is not backup up by s.o. else. I feel it was somewhat drowned by the rest of the discussion. Do YOU like it? My two cents about it that the a=b:c=d syntax from AVDict is at least known and used in filters already. The function style a(b,c,d) thing from SVG would be completely new. Instead of the AVDict overhead, it adds a simplistic parser overhead. Also, maybe I'm just unaware, these style of options appears not to be often used in the command line world. But as I said I usually don't touch ffmpeg / it's options, so I think my opinion should not be of much value here where we talk of the contextual effects of this patch. -Thilo ___ 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/1] lavf/dashdec: Multithreaded DASH initialization
This patch adds multithreading support to DASH initialization. Initializing DASH streams is currently slow, because each individual stream is opened and probed sequentially. With DASH streams often having somewhere between 10-20 substreams, this can easily take up to half a minute on slow connections. This patch adds an "init-threads" option, specifying the max number of threads to use for parallel probing and initialization of substreams. If "init-threads" is set to a value larger than 1, multiple worker threads are spun up to massively bring down init times. Here is a free DASH stream for testing: http://www.bok.net/dash/tears_of_steel/cleartext/stream.mpd It has 7 substreams. I currently get init times of 3.5 seconds without patch. The patch brings it down to about 0.5 seconds, so using 7 threads nearly cut down init times by factor 7. On a slower connection (100mbit), the same stream took 7-8 seconds to initialize, the patch brought it down to just over 1 second. In the current patch, the behavior is disabled by default (init-threads = 0). But I think it could make sense to enable it by default, maybe with a reasonable value of 8? Not sure though, open for discussion. Some notes on the actual implementation: - DASH streams sometimes share a common init section. If this is the case, a mutex and condition is used, to make sure that the first stream reads the common section before the following streams start initialization. - Only the init and probing part is done in parallel. After all threads are joined, I collect the results and add the AVStreams to the parent AVFormatContext. That is why I split open_demux_for_component() into begin_open_demux_for_component() and end_open_demux_for_component(). - I tried to do this as clean as possible and added multiple comments. - Multithreading is never simple, so a proper review is needed. If this gets merged, I might try to do the same for HLS. This is my first PR by the way, so please be nice :) Lukas 0001-lavf-dashdec-Multithreaded-DASH-initialization.patch Description: Binary data ___ 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/1] lavf/dashdec: Multithreaded DASH initialization
Trying with inline PATCH since attached file was not showing up...
---
From: Lukas Fellechner
Subject: [PATCH 1/1] lavf/dashdec: Multithreaded DASH initialization
Initializing DASH streams is currently slow, because each individual stream is
opened and probed sequentially. With DASH streams often having somewhere
between 10-20 streams, this can easily take up to half a minute. This patch
adds an "init-threads" option, specifying the max number of threads to use.
Multiple worker threads are spun up to massively bring down init times.
---
libavformat/dashdec.c | 421 +-
1 file changed, 375 insertions(+), 46 deletions(-)
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 63bf7e96a5..69a6c2ba79 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -24,6 +24,7 @@
#include "libavutil/opt.h"
#include "libavutil/time.h"
#include "libavutil/parseutils.h"
+#include "libavutil/thread.h"
#include "internal.h"
#include "avio_internal.h"
#include "dash.h"
@@ -152,6 +153,8 @@ typedef struct DASHContext {
int max_url_size;
char *cenc_decryption_key;
+int init_threads;
+
/* Flags for init section*/
int is_init_section_common_video;
int is_init_section_common_audio;
@@ -1918,22 +1921,40 @@ fail:
return ret;
}
-static int open_demux_for_component(AVFormatContext *s, struct representation
*pls)
+static int open_demux_for_component(AVFormatContext* s, struct representation*
pls)
+{
+int ret = 0;
+
+ret = begin_open_demux_for_component(s, pls);
+if (ret < 0)
+return ret;
+
+ret = end_open_demux_for_component(s, pls);
+
+return ret;
+}
+
+static int begin_open_demux_for_component(AVFormatContext* s, struct
representation* pls)
{
int ret = 0;
-int i;
pls->parent = s;
-pls->cur_seq_no = calc_cur_seg_no(s, pls);
+pls->cur_seq_no = calc_cur_seg_no(s, pls);
if (!pls->last_seq_no) {
pls->last_seq_no = calc_max_seg_no(pls, s->priv_data);
}
ret = reopen_demux_for_component(s, pls);
-if (ret < 0) {
-goto fail;
-}
+
+return ret;
+}
+
+static int end_open_demux_for_component(AVFormatContext* s, struct
representation* pls)
+{
+int ret = 0;
+int i;
+
for (i = 0; i < pls->ctx->nb_streams; i++) {
AVStream *st = avformat_new_stream(s, NULL);
AVStream *ist = pls->ctx->streams[i];
@@ -2015,6 +2036,131 @@ static void move_metadata(AVStream *st, const char
*key, char **value)
}
}
+struct work_pool_data
+{
+AVFormatContext* ctx;
+struct representation* pls;
+struct representation* common_pls;
+pthread_mutex_t* common_mutex;
+pthread_cond_t* common_condition;
+int is_common;
+int is_started;
+int result;
+};
+
+struct thread_data
+{
+pthread_t thread;
+pthread_mutex_t* mutex;
+struct work_pool_data* work_pool;
+int work_pool_size;
+int is_started;
+};
+
+static void *worker_thread(void *ptr)
+{
+int ret = 0;
+int i;
+struct thread_data* thread_data = (struct thread_data*)ptr;
+struct work_pool_data* work_pool = NULL;
+struct work_pool_data* data = NULL;
+for (;;) {
+
+// get next work item
+pthread_mutex_lock(thread_data->mutex);
+data = NULL;
+work_pool = thread_data->work_pool;
+for (i = 0; i < thread_data->work_pool_size; i++) {
+if (!work_pool->is_started) {
+data = work_pool;
+data->is_started = 1;
+break;
+}
+work_pool++;
+}
+pthread_mutex_unlock(thread_data->mutex);
+
+if (!data) {
+// no more work to do
+return NULL;
+}
+
+// if we are common section provider, init and signal
+if (data->is_common) {
+data->pls->parent = data->ctx;
+ret = update_init_section(data->pls);
+if (ret < 0) {
+pthread_cond_signal(data->common_condition);
+goto end;
+}
+else
+ret = AVERROR(pthread_cond_signal(data->common_condition));
+}
+
+// if we depend on common section provider, wait for signal and copy
+if (data->common_pls) {
+ret = AVERROR(pthread_cond_wait(data->common_condition,
data->common_mutex));
+if (ret < 0)
+goto end;
+
+if (!data->common_pls->init_sec_buf) {
+goto end;
+ret = AVERROR(EFAULT);
+}
+
+ret = copy_init_section(data->pls, data->common_pls);
+if (ret < 0)
+goto end;
+}
+
+ret = begin_open_demux_for_component(data->ctx, data->pls);
+if (ret < 0)
+goto end;
+
+end:
+data->result = ret;
+}
+
+
+return NULL;
+}
+
+static void create_work_pool_data(AVFormatContext* ctx, int stream_index,
+struct repr
[FFmpeg-devel] [PATCH 1/4] avcodec/pngdec: Fix APNG_DISPOSE_OP_BACKGROUND
APNG works with a single reference frame and an output frame.
According to the spec, decoding APNG works by decoding
the current IDAT/fdAT chunks (which decodes to a rectangular
subregion of the whole image region), followed by either
overwriting the region of the output frame with the newly
decoded data or by blending the newly decoded data with
the data from the reference frame onto the current subregion
of the output frame. The remainder of the output frame
is just copied from the reference frame.
Then the reference frame might be left untouched
(APNG_DISPOSE_OP_PREVIOUS), it might be replaced by the output
frame (APNG_DISPOSE_OP_NONE) or the rectangular subregion
corresponding to the just decoded frame has to be reset
to black (APNG_DISPOSE_OP_BACKGROUND).
The latter case is not handled correctly by our decoder:
It only performs resetting the rectangle in the reference frame
when decoding the next frame; and since commit
b593abda6c642cb0c3959752dd235c2faf66837f it does not reset
the reference frame permanently, but only temporarily (i.e.
it only affects decoding the frame after the frame with
APNG_DISPOSE_OP_BACKGROUND). This is a problem if the
frame after the APNG_DISPOSE_OP_BACKGROUND frame uses
APNG_DISPOSE_OP_PREVIOUS, because then the frame after
the APNG_DISPOSE_OP_PREVIOUS frame has an incorrect reference
frame. (If it is not followed by an APNG_DISPOSE_OP_PREVIOUS
frame, the decoder only keeps a reference to the output frame,
which is ok.)
This commit fixes this by being much closer to the spec
than the earlier code: Resetting the background is no longer
postponed until the next frame; instead it is applied to
the reference frame.
Fixes ticket #9602.
(For multithreaded decoding it was actually already broken
since commit 5663301560d77486c7f7c03c1aa5f542fab23c24.)
Signed-off-by: Andreas Rheinhardt
---
Btw: If we had a function that only references a frame's data
(and leaves the dst frame's side data completely untouched),
we could apply the side data directly to the output frame,
making 8d74baccff59192d395735036cd40a131a140391 unnecessary.
I also wonder whether the handle_p_frame stuff should be moved
out of decode_frame_common() and in the codec-specific code.
libavcodec/pngdec.c | 98 ++---
1 file changed, 48 insertions(+), 50 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 3b888199d4..8a197d038d 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -78,11 +78,8 @@ typedef struct PNGDecContext {
enum PNGImageState pic_state;
int width, height;
int cur_w, cur_h;
-int last_w, last_h;
int x_offset, y_offset;
-int last_x_offset, last_y_offset;
uint8_t dispose_op, blend_op;
-uint8_t last_dispose_op;
int bit_depth;
int color_type;
int compression_type;
@@ -94,8 +91,6 @@ typedef struct PNGDecContext {
int has_trns;
uint8_t transparent_color_be[6];
-uint8_t *background_buf;
-unsigned background_buf_allocated;
uint32_t palette[256];
uint8_t *crow_buf;
uint8_t *last_row;
@@ -725,9 +720,30 @@ static int decode_idat_chunk(AVCodecContext *avctx,
PNGDecContext *s,
}
ff_thread_release_ext_buffer(avctx, &s->picture);
-if ((ret = ff_thread_get_ext_buffer(avctx, &s->picture,
-AV_GET_BUFFER_FLAG_REF)) < 0)
-return ret;
+if (s->dispose_op == APNG_DISPOSE_OP_PREVIOUS) {
+/* We only need a buffer for the current picture. */
+ret = ff_thread_get_buffer(avctx, p, 0);
+if (ret < 0)
+return ret;
+} else if (s->dispose_op == APNG_DISPOSE_OP_BACKGROUND) {
+/* We need a buffer for the current picture as well as
+ * a buffer for the reference to retain. */
+ret = ff_thread_get_ext_buffer(avctx, &s->picture,
+ AV_GET_BUFFER_FLAG_REF);
+if (ret < 0)
+return ret;
+ret = ff_thread_get_buffer(avctx, p, 0);
+if (ret < 0)
+return ret;
+} else {
+/* The picture output this time and the reference to retain
coincide. */
+if ((ret = ff_thread_get_ext_buffer(avctx, &s->picture,
+AV_GET_BUFFER_FLAG_REF)) < 0)
+return ret;
+ret = av_frame_ref(p, s->picture.f);
+if (ret < 0)
+return ret;
+}
p->pict_type= AV_PICTURE_TYPE_I;
p->key_frame= 1;
@@ -985,12 +1001,6 @@ static int decode_fctl_chunk(AVCodecContext *avctx,
PNGDecContext *s,
return AVERROR_INVALIDDATA;
}
-s->last_w = s->cur_w;
-s->last_h = s->cur_h;
-s->last_x_offset = s->x_offset;
-s->last_y_offset = s->y_offset;
-s->last_dispose_op = s->dispose_op;
-
sequence_number = bytestream2_get_be32(gb);
c
[FFmpeg-devel] [PATCH 2/4] avcodec/pngdec: Use internal AVBPrint string when parsing chunks
One saves an allocation in case the string fits into the buffer.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/pngdec.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 8a197d038d..189c3eee89 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -543,10 +543,8 @@ static int decode_text_chunk(PNGDecContext *s,
GetByteContext *gb, int compresse
return AVERROR_INVALIDDATA;
if ((ret = decode_zbuf(&bp, data, data_end, s->avctx)) < 0)
return ret;
+text = bp.str;
text_len = bp.len;
-ret = av_bprint_finalize(&bp, (char **)&text);
-if (ret < 0)
-return ret;
} else {
text = (uint8_t *)data;
text_len = data_end - text;
@@ -554,8 +552,8 @@ static int decode_text_chunk(PNGDecContext *s,
GetByteContext *gb, int compresse
kw_utf8 = iso88591_to_utf8(keyword, keyword_end - keyword);
txt_utf8 = iso88591_to_utf8(text, text_len);
-if (text != data)
-av_free(text);
+if (compressed)
+av_bprint_finalize(&bp, NULL);
if (!(kw_utf8 && txt_utf8)) {
av_free(kw_utf8);
av_free(txt_utf8);
--
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/4] avcodec/pngdec: Use char* instead of uint8_t* for text
Also stop casting const away.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/pngdec.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 189c3eee89..132ad40dc9 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -496,20 +496,20 @@ fail:
return ret;
}
-static uint8_t *iso88591_to_utf8(const uint8_t *in, size_t size_in)
+static char *iso88591_to_utf8(const char *in, size_t size_in)
{
size_t extra = 0, i;
-uint8_t *out, *q;
+char *out, *q;
for (i = 0; i < size_in; i++)
-extra += in[i] >= 0x80;
+extra += !!(in[i] & 0x80);
if (size_in == SIZE_MAX || extra > SIZE_MAX - size_in - 1)
return NULL;
q = out = av_malloc(size_in + extra + 1);
if (!out)
return NULL;
for (i = 0; i < size_in; i++) {
-if (in[i] >= 0x80) {
+if (in[i] & 0x80) {
*(q++) = 0xC0 | (in[i] >> 6);
*(q++) = 0x80 | (in[i] & 0x3F);
} else {
@@ -525,9 +525,10 @@ static int decode_text_chunk(PNGDecContext *s,
GetByteContext *gb, int compresse
int ret, method;
const uint8_t *data= gb->buffer;
const uint8_t *data_end= gb->buffer_end;
-const uint8_t *keyword = data;
-const uint8_t *keyword_end = memchr(keyword, 0, data_end - keyword);
-uint8_t *kw_utf8 = NULL, *text, *txt_utf8 = NULL;
+const char *keyword = data;
+const char *keyword_end = memchr(keyword, 0, data_end - data);
+char *kw_utf8 = NULL, *txt_utf8 = NULL;
+const char *text;
unsigned text_len;
AVBPrint bp;
@@ -546,8 +547,8 @@ static int decode_text_chunk(PNGDecContext *s,
GetByteContext *gb, int compresse
text = bp.str;
text_len = bp.len;
} else {
-text = (uint8_t *)data;
-text_len = data_end - text;
+text = data;
+text_len = data_end - data;
}
kw_utf8 = iso88591_to_utf8(keyword, keyword_end - keyword);
--
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 4/4] avcodec/pngdec: Improve decoding text chunks
By checking immediately whether the first allocation was successfull
one can simplify the cleanup code in case of errors.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/pngdec.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 132ad40dc9..1d6ca7f4c3 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -551,12 +551,13 @@ static int decode_text_chunk(PNGDecContext *s,
GetByteContext *gb, int compresse
text_len = data_end - data;
}
-kw_utf8 = iso88591_to_utf8(keyword, keyword_end - keyword);
txt_utf8 = iso88591_to_utf8(text, text_len);
if (compressed)
av_bprint_finalize(&bp, NULL);
-if (!(kw_utf8 && txt_utf8)) {
-av_free(kw_utf8);
+if (!txt_utf8)
+return AVERROR(ENOMEM);
+kw_utf8 = iso88591_to_utf8(keyword, keyword_end - keyword);
+if (!kw_utf8) {
av_free(txt_utf8);
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 1/1] lavf/dashdec: Multithreaded DASH initialization
Lukas Fellechner 于2022年8月21日周日 05:54写道：
>
> Trying with inline PATCH since attached file was not showing up...
>
> ---
>
> From: Lukas Fellechner
> Subject: [PATCH 1/1] lavf/dashdec: Multithreaded DASH initialization
>
> Initializing DASH streams is currently slow, because each individual stream
> is opened and probed sequentially. With DASH streams often having somewhere
> between 10-20 streams, this can easily take up to half a minute. This patch
> adds an "init-threads" option, specifying the max number of threads to use.
> Multiple worker threads are spun up to massively bring down init times.
> ---
> libavformat/dashdec.c | 421 +-
> 1 file changed, 375 insertions(+), 46 deletions(-)
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 63bf7e96a5..69a6c2ba79 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -24,6 +24,7 @@
> #include "libavutil/opt.h"
> #include "libavutil/time.h"
> #include "libavutil/parseutils.h"
> +#include "libavutil/thread.h"
> #include "internal.h"
> #include "avio_internal.h"
> #include "dash.h"
> @@ -152,6 +153,8 @@ typedef struct DASHContext {
> int max_url_size;
> char *cenc_decryption_key;
>
> +int init_threads;
> +
> /* Flags for init section*/
> int is_init_section_common_video;
> int is_init_section_common_audio;
> @@ -1918,22 +1921,40 @@ fail:
> return ret;
> }
>
> -static int open_demux_for_component(AVFormatContext *s, struct
> representation *pls)
> +static int open_demux_for_component(AVFormatContext* s, struct
> representation* pls)
> +{
> +int ret = 0;
> +
> +ret = begin_open_demux_for_component(s, pls);
> +if (ret < 0)
> +return ret;
> +
> +ret = end_open_demux_for_component(s, pls);
> +
> +return ret;
> +}
> +
> +static int begin_open_demux_for_component(AVFormatContext* s, struct
> representation* pls)
> {
> int ret = 0;
> -int i;
>
> pls->parent = s;
> -pls->cur_seq_no = calc_cur_seg_no(s, pls);
> +pls->cur_seq_no = calc_cur_seg_no(s, pls);
>
> if (!pls->last_seq_no) {
> pls->last_seq_no = calc_max_seg_no(pls, s->priv_data);
> }
>
> ret = reopen_demux_for_component(s, pls);
> -if (ret < 0) {
> -goto fail;
> -}
> +
> +return ret;
> +}
> +
> +static int end_open_demux_for_component(AVFormatContext* s, struct
> representation* pls)
> +{
> +int ret = 0;
> +int i;
> +
> for (i = 0; i < pls->ctx->nb_streams; i++) {
> AVStream *st = avformat_new_stream(s, NULL);
> AVStream *ist = pls->ctx->streams[i];
> @@ -2015,6 +2036,131 @@ static void move_metadata(AVStream *st, const char
> *key, char **value)
> }
> }
>
> +struct work_pool_data
> +{
> +AVFormatContext* ctx;
> +struct representation* pls;
> +struct representation* common_pls;
> +pthread_mutex_t* common_mutex;
> +pthread_cond_t* common_condition;
Should add #if HAVE_THREADS to check if the pthread supported.
> +int is_common;
> +int is_started;
> +int result;
> +};
> +
> +struct thread_data
> +{
> +pthread_t thread;
> +pthread_mutex_t* mutex;
> +struct work_pool_data* work_pool;
> +int work_pool_size;
> +int is_started;
> +};
> +
> +static void *worker_thread(void *ptr)
> +{
> +int ret = 0;
> +int i;
> +struct thread_data* thread_data = (struct thread_data*)ptr;
> +struct work_pool_data* work_pool = NULL;
> +struct work_pool_data* data = NULL;
> +for (;;) {
> +
> +// get next work item
> +pthread_mutex_lock(thread_data->mutex);
> +data = NULL;
> +work_pool = thread_data->work_pool;
> +for (i = 0; i < thread_data->work_pool_size; i++) {
> +if (!work_pool->is_started) {
> +data = work_pool;
> +data->is_started = 1;
> +break;
> +}
> +work_pool++;
> +}
> +pthread_mutex_unlock(thread_data->mutex);
> +
> +if (!data) {
> +// no more work to do
> +return NULL;
> +}
> +
> +// if we are common section provider, init and signal
> +if (data->is_common) {
> +data->pls->parent = data->ctx;
> +ret = update_init_section(data->pls);
> +if (ret < 0) {
> +pthread_cond_signal(data->common_condition);
> +goto end;
> +}
> +else
> +ret = AVERROR(pthread_cond_signal(data->common_condition));
> +}
> +
> +// if we depend on common section provider, wait for signal and copy
> +if (data->common_pls) {
> +ret = AVERROR(pthread_cond_wait(data->common_condition,
> data->common_mutex));
> +if (ret < 0)
> +goto end;
> +
> +if (!data->common_pls->init_sec_buf) {
> +goto end;
> +ret = AVERROR(EFAULT);
> +
