January 2022 - ffmpeg-devel - lists.ffmpeg.org

The case for a good string API
by Nicolas George 15 Aug '22

15 Aug '22

Hi. I will try again proposing a good string API for the project. But this time, instead of showing the implementation code, I will ask about the principle. So, please, read the argument, and give your opinion on the principle. If we agree on the principle, we can discuss the code then. * Why we need a better string API Strings are not the bulk of what we do, but for the user interface of things we have a LOT of functions that return strings: to describe a color space or a channel layout or a set of options. When the returned string is short, we use the good old “char *buf, size_t buf_size”. When it is long, we return a dynamically-allocated buffer. When the string is almost always short but can actually be very long, and the conversion is used in speed-critical code, we are screwed. With a good string API, we could use the same API everywhere, it would automatically use the most efficient memory scheme. Furthermore, if all our object→string functions have the same prototype, it makes it easier to use these functions as standardized callbacks and opens a whole lot of new possibilities of serialization features. Moreover, a good string API would bring quite a few extra benefits, with a few examples: easier to build strings from parts, less verbose and more robust error checking, the ability to use directly the string or buffering API of another library or language. So, even though strings are not the core of our thing, I think we should have a good string API, just like we have an API for efficient FFTs (which is the core of our thing). But let us discuss it further: * Objections and counter-objections - A string API adds complexity. Indeed, that is true. But the added complexity is clearly isolated in one or a few source files. In the rest of the code, it removes complexity (see less verbose error checking, easier to build strings from parts above). String conversion functions can happen once per short frame, for example in the ashowinfo filter. We have pools for frames and for buffers: we already consider that avoid once-per-frame memory allocations is worth the added complexity. A string API is just a step in the same direction. - An unusual API is annoying for people who will use it. Unless we messed up, if they use strings more than just a little, the benefits of the API is way worth the cost of learning. But some people use strings just a little. For them, who would be happy with a plain C string, we should make sure the required learning is very very limited. And we can: “instead of ‘buf, sizeof(buf)’, write ‘string_from_buffer(buf)”. That is all, in one short line I have documented how to use the API with the old patterns. There is a little more to write for people who want a dynamic buffer for an unlimited string (error checks are always annoying), but we can keep it under three minutes. * What string API we should use We have AVBprint, but the API is ugly, and it only brings a few of the benefits I promised. I have a proposal: AVWriter. I posted it last spring, here is an introduction on how to use it (with the implementation in the same thread): https://ffmpeg.org/pipermail/ffmpeg-devel/2021-April/279383.html * Conclusion Well, I am convinced, but what about you? 1. Do you think FFmpeg needs a good string API? If not, please explain why? 2. Assuming we agree ‘yes’ on the previous question, do you think AVWriter is a good candidate? If not, what else would you propose? Regards, -- Nicolas George

3 5

[PATCH 1/5] avcodec: Require AMF SDK v1.4.23 or newer
by Michael Fabian 'Xaymar' Dirks 10 Jul '22

10 Jul '22

From: Michael Fabian 'Xaymar' Dirks <michael.dirks(a)xaymar.com> Increasing the minimum AMF SDK version allows us to support more recent hardware and drivers, which is necessary to fix some of the discrepancies between older and newer drivers on AMD hardware with FFmpeg. --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 1413122d87..515ec1a50f 100755 --- a/configure +++ b/configure @@ -6969,7 +6969,7 @@ fi enabled amf && check_cpp_condition amf "AMF/core/Version.h" \ - "(AMF_VERSION_MAJOR << 48 | AMF_VERSION_MINOR << 32 | AMF_VERSION_RELEASE << 16 | AMF_VERSION_BUILD_NUM) >= 0x0001000400090000" + "AMF_FULL_VERSION >= AMF_MAKE_FULL_VERSION(1, 4, 23, 0)" # Funny iconv installations are not unusual, so check it after all flags have been set if enabled libc_iconv; then -- 2.34.1.windows.1

4 9

[PATCH 0/1] [FFmpeg-devel, WIP] libopusdec: Enable FEC/PLC
by Philip-Dylan Gleonec 06 Jul '22

06 Jul '22

Hello, As part of GNU Jami development, I've picked up this patch and worked to complete and validate it. The few changes I've made are mostly to add checks to manage errors and different cases, notably video vs audio containers. I have tested the feature with opus stream encapsulated in mkv, opus, and RTP stream. In the last case, packets can be dropped with the tc command. Thanks a lot Steiner for your original work, it helped a lot. Thanks for your time. Philip-Dylan Gleonec (1): avcodec/libopusdec: Enable FEC/PLC libavcodec/libopusdec.c | 105 +++++++++++++++++++++++++++++++++++----- 1 file changed, 94 insertions(+), 11 deletions(-) -- 2.25.1

6 22

Patch to libavc/opus to create extradata if missing
by Jonathan Baudanza 06 Jul '22

06 Jul '22

This patch to libavcodec/opus.c will create the opus extradata if it is missing. This is required when muxing opus data from an RTP demuxer. Without this patch, the opux muxer will fail with a "No extradata present" error. This issue was first reported by Juan Navarro here: http://www.ffmpeg-archive.org/Unable-to-record-VP8-Opus-from-RTP-SDP-td4689… This patch was first created by Carl Eugen Hoyos, but didn't land because resulting opus files wouldn't play in VLC. The reason is because the opus headers in the original patch were too large. This patch update's Carl's original patch to limit the size of the extradata to 19 bytes.

5 21

[PATCH 1/2] lavf/avio: add avio_vprintf()
by Stefano Sabatini 16 Jun '22

16 Jun '22

This new function makes it possible to use avio_printf() functionality from a function taking a variable list of arguments. --- doc/APIchanges | 3 +++ libavformat/avio.h | 6 ++++++ libavformat/aviobuf.c | 17 +++++++++++++---- libavformat/version.h | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/doc/APIchanges b/doc/APIchanges index b41dadee8d..08fec7c234 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,9 @@ libavutil: 2017-10-21 API changes, most recent first: +2021-04-18 - xxxxxxxxxx - lavf 58.78.100 - avio.h + Add avio_vprintf(), similar to avio_printf(). + 2021-03-21 - xxxxxxxxxx - lavu 56.72.100 - frame.h Deprecated av_get_colorspace_name(). Use av_color_space_name() instead. diff --git a/libavformat/avio.h b/libavformat/avio.h index d022820a6e..24f6839522 100644 --- a/libavformat/avio.h +++ b/libavformat/avio.h @@ -571,6 +571,12 @@ int64_t avio_size(AVIOContext *s); */ int avio_feof(AVIOContext *s); +/** + * Writes a formatted string to the context taking a va_list. + * @return number of bytes written, < 0 on error. + */ +int avio_vprintf(AVIOContext *s, const char *fmt, va_list ap); + /** * Writes a formatted string to the context. * @return number of bytes written, < 0 on error. diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index 518cb11129..289da796c8 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -1196,15 +1196,12 @@ int avio_closep(AVIOContext **s) return ret; } -int avio_printf(AVIOContext *s, const char *fmt, ...) +int avio_vprintf(AVIOContext *s, const char *fmt, va_list ap) { - va_list ap; AVBPrint bp; av_bprint_init(&bp, 0, INT_MAX); - va_start(ap, fmt); av_vbprintf(&bp, fmt, ap); - va_end(ap); if (!av_bprint_is_complete(&bp)) { av_bprint_finalize(&bp, NULL); s->error = AVERROR(ENOMEM); @@ -1215,6 +1212,18 @@ int avio_printf(AVIOContext *s, const char *fmt, ...) return bp.len; } +int avio_printf(AVIOContext *s, const char *fmt, ...) +{ + va_list ap; + int ret; + + va_start(ap, fmt); + ret = avio_vprintf(s, fmt, ap); + va_end(ap); + + return ret; +} + void avio_print_string_array(AVIOContext *s, const char *strings[]) { for(; *strings; strings++) diff --git a/libavformat/version.h b/libavformat/version.h index ced5600034..b6023f9d2e 100644 --- a/libavformat/version.h +++ b/libavformat/version.h @@ -32,7 +32,7 @@ // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium) // Also please add any ticket numbers that you believe might be affected here #define LIBAVFORMAT_VERSION_MAJOR 58 -#define LIBAVFORMAT_VERSION_MINOR 77 +#define LIBAVFORMAT_VERSION_MINOR 78 #define LIBAVFORMAT_VERSION_MICRO 100 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \ -- 2.25.1

6 14

[WORK IN PROGRESS] Efficient string and JSON writing API
by Nicolas George 15 Jun '22

15 Jun '22

Hi. This patch series is not for review, it is for reference, because I am stashing it before working on something else. The prospect of the uphill battle that would be needed to convince some people here that we need this discouraged me a little, I must say. It is the work in progress for a set of APIs to handle string and serialization with the accent on convenience, efficiency and nimbleness. It is all that was good about BPrint, but even better. For now, it implements strings plus JSON serialization. Once it works, it can be easily extended to XML serialization based on the same principles, and that is something several part of our code base would benefit from. It is convenient because it factors error checking as much as possible. It is also convenient because it is efficient and nimble: we could use it everywhere without bothering to wonder if in a particular case something else would be better. It is efficient because it avoids unnecessary allocations. That makes it usable even when it happens once per frame or more. But it does so without adding arbitrary limits, like using a fixed-size buffer would. It is nimble because it can directly plug into other APIs and ways of handling strings. It can use Perl strings, Java strings, GUI text buffers, etc., all directly without the need for extra buffers. It can also plug directly to files or network, again without intermediate buffers. All this has some cost, of course. It costs code to implement it. Just like memory pools with efficient locking. Efficiency is worth it. It also costs about three minutes to learn how to use it. But the convenience makes it well worth it. To achieve all this, I had to use unusual programming techniques. I had to invent ways of doing things. I think this is the FFmpeg way. If you had not invented better programming techniques, we never would have got the fastest VP9 decoder and so on. I am sad that the project is progressively turning into a risk-advert collection of wrappers for existing libraries instead of continuing its tradition of inventing new and better ways of doing things. I am not good at shaving a few cycles in a tight loop, so optimizing codecs is not my scene, but infrastructure is also necessary for the project. If I have my way about this overhaul of string APIs, at some point we will be able to use FFmpeg types directly in printf-like format strings. But that is for later. For now, feel free to comment or ask for details. I would be happy to learn that you find all this interesting. But I will start working on event loop for libavformat and libavdevice. [PATCH 1/7] lavu: add macros to help making future-proof structures. [PATCH 2/7] lavu: new AVWriter API. [PATCH 3/7] lavu/writer: add test. [PATCH 4/7] lavf/dump: use a writer. [PATCH 5/7] WIP: add an intro to AVWriter [PATCH 6/7] lavu: add a JSON writer API. [PATCH 7/7] lavu: add JSON writer test. Regards, -- Nicolas George

2 8

[PATCH] libRIST: allow setting fifo size and fail on overflow.
by Gijs Peskens 09 Jun '22

09 Jun '22

Introduce fifo_size and overrun_nonfatal params to configure fifo buffer behavior. Use newly introduced RIST_DATA_FLAGS_OVERFLOW flag to check for overrun and error out in that case. --- doc/protocols.texi | 9 +++++++++ libavformat/librist.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/doc/protocols.texi b/doc/protocols.texi index d207df0b52..f1acf0cc77 100644 --- a/doc/protocols.texi +++ b/doc/protocols.texi @@ -745,6 +745,15 @@ Set internal RIST buffer size in milliseconds for retransmission of data. Default value is 0 which means the librist default (1 sec). Maximum value is 30 seconds. +@item fifo_size +Size of the librist receiver output fifo in number of packets. This must be a +power of 2. +Defaults to 8192 (vs the libRIST default of 1024). + +@item overrun_nonfatal=@var{1|0} +Survive in case of libRIST fifo buffer overrun. Default +value is 0. + @item pkt_size Set maximum packet size for sending data. 1316 by default. diff --git a/libavformat/librist.c b/libavformat/librist.c index 378b635ea7..87c14eb265 100644 --- a/libavformat/librist.c +++ b/libavformat/librist.c @@ -43,6 +43,9 @@ ((patch) + ((minor)* 0x100) + ((major) *0x10000)) #define FF_LIBRIST_VERSION FF_LIBRIST_MAKE_VERSION(LIBRIST_API_VERSION_MAJOR, LIBRIST_API_VERSION_MINOR, LIBRIST_API_VERSION_PATCH) #define FF_LIBRIST_VERSION_41 FF_LIBRIST_MAKE_VERSION(4, 1, 0) +#define FF_LIBRIST_VERSION_42 FF_LIBRIST_MAKE_VERSION(4, 2, 0) + +#define FF_LIBRIST_FIFO_SIZE_DEFAULT 8192 typedef struct RISTContext { const AVClass *class; @@ -52,6 +55,8 @@ typedef struct RISTContext { int packet_size; int log_level; int encryption; + int fifo_size; + int overrun_nonfatal; char *secret; struct rist_logging_settings logging_settings; @@ -70,6 +75,8 @@ static const AVOption librist_options[] = { { "main", NULL, 0, AV_OPT_TYPE_CONST, {.i64=RIST_PROFILE_MAIN}, 0, 0, .flags = D|E, "profile" }, { "advanced", NULL, 0, AV_OPT_TYPE_CONST, {.i64=RIST_PROFILE_ADVANCED}, 0, 0, .flags = D|E, "profile" }, { "buffer_size", "set buffer_size in ms", OFFSET(buffer_size), AV_OPT_TYPE_INT, {.i64=0}, 0, 30000, .flags = D|E }, + { "fifo_size", "Set libRIST fifo buffer. Size must be power of 2", OFFSET(fifo_size), AV_OPT_TYPE_INT, {.i64=FF_LIBRIST_FIFO_SIZE_DEFAULT}, 2 << 9, 2 << 15, .flags = D|E }, + { "overrun_nonfatal", "survive in case of libRIST receiving circular buffer overrun", OFFSET(overrun_nonfatal), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, D }, { "pkt_size", "set packet size", OFFSET(packet_size), AV_OPT_TYPE_INT, {.i64=1316}, 1, MAX_PAYLOAD_SIZE, .flags = D|E }, { "log_level", "set loglevel", OFFSET(log_level), AV_OPT_TYPE_INT, {.i64=RIST_LOG_INFO}, -1, INT_MAX, .flags = D|E }, { "secret", "set encryption secret",OFFSET(secret), AV_OPT_TYPE_STRING,{.str=NULL}, 0, 0, .flags = D|E }, @@ -161,6 +168,20 @@ static int librist_open(URLContext *h, const char *uri, int flags) if (ret < 0) goto err; + //Prior to 4.2.0 there was a bug in libRIST which made this call always fail. +#if FF_LIBRIST_VERSION >= FF_LIBRIST_VERSION_42 + if (flags & AVIO_FLAG_READ) { + ret = rist_receiver_set_output_fifo_size(s->ctx, s->fifo_size); + if (ret != 0) { + goto err; + } + } +#else + if (s->fifo_size != FF_LIBRIST_FIFO_SIZE_DEFAULT) { + av_log(h, AV_LOG_ERROR, "libRIST prior to 0.2.7 has a bug which fails setting the fifo buffer size\n"); + } +#endif + if (((s->encryption == 128 || s->encryption == 256) && !s->secret) || ((peer_config->key_size == 128 || peer_config->key_size == 256) && !peer_config->secret[0])) { av_log(h, AV_LOG_ERROR, "secret is mandatory if encryption is enabled\n"); @@ -223,8 +244,24 @@ static int librist_read(URLContext *h, uint8_t *buf, int size) return AVERROR_EXTERNAL; } +#if FF_LIBRIST_VERSION >= FF_LIBRIST_VERSION_42 + if (data_block->flags & RIST_DATA_FLAGS_OVERFLOW == RIST_DATA_FLAGS_OVERFLOW) { + if (!s->overrun_nonfatal) { + av_log(h, AV_LOG_ERROR, "Fifo buffer overrun. " + "To avoid, increase fifo_size URL option. " + "To survive in such case, use overrun_nonfatal option\n"); + size = AVERROR(EIO); + goto out_free; + } else { + av_log(h, AV_LOG_WARNING, "Fifo buffer overrun. " + "Surviving due to overrun_nonfatal option\n"); + } + } +#endif + size = data_block->payload_len; memcpy(buf, data_block->payload, size); +out_free: #if FF_LIBRIST_VERSION < FF_LIBRIST_VERSION_41 rist_receiver_data_block_free((struct rist_data_block**)&data_block); #else -- 2.32.0

2 3

[PATCH] avcodec/texturedspenc: Fix indexing in color distribution determination
by Michael Niedermayer 08 Jun '22

08 Jun '22

Fixes CID1396405 Untested except fate which does not test this, please someone test this Signed-off-by: Michael Niedermayer <michael(a)niedermayer.cc> --- libavcodec/texturedspenc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libavcodec/texturedspenc.c b/libavcodec/texturedspenc.c index 8b2863033b..5171c69d82 100644 --- a/libavcodec/texturedspenc.c +++ b/libavcodec/texturedspenc.c @@ -255,11 +255,11 @@ static void optimize_colors(const uint8_t *block, ptrdiff_t stride, muv = minv = maxv = bp[0]; for (y = 0; y < 4; y++) { - for (x = 4; x < 4; x += 4) { + for (x = 0; x < 4; x++) { muv += bp[x * 4 + y * stride]; - if (bp[x] < minv) + if (bp[x * 4 + y * stride] < minv) minv = bp[x * 4 + y * stride]; - else if (bp[x] > maxv) + else if (bp[x * 4 + y * stride] > maxv) maxv = bp[x * 4 + y * stride]; } } -- 2.11.0

4 6

[PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define
by ffmpegagent 08 Jun '22

08 Jun '22

It's annoying and unexpected, but still useful at times (as I've realized just recently). This is a follow-up to the earlier submission here: https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg128080.html There has been a comment from Anton, questioning whether the dump-feature is useful. Meanwhile I came to the conclusion that it can be useful in-fact. It just shouldn't happen automatically when DEBUG is defined. That's what these patches do. I also added fixes for the fopen() call. softworkz (4): avcodec/dvbsubdec: don't dump images to disk based on DEBUG define avcodec/dvbsubdec: fix writing ppm avcodec/dvdsubdec: don't dump images to disk based on DEBUG define avcodec/dvdsubdec: fix writing ppm libavcodec/dvbsubdec.c | 20 +++++++++++++------- libavcodec/dvdsubdec.c | 11 ++++++++--- 2 files changed, 21 insertions(+), 10 deletions(-) base-commit: 242ed971cb005157488b9a21942d9fb4be4d0347 Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-17%2Fsoftwork… Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-17/softworkz/submit_dvb_subs-v1 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/17 -- ffmpeg-codebot

5 19

[PATCH] libavutil/tests/md5: Avoid warnings
by softworkz 08 Jun '22

08 Jun '22

From: softworkz <softworkz(a)hotmail.com> Those are always showing up on Patchwork when FATE tests are failing, covering some possibly more useful information. Signed-off-by: softworkz <softworkz(a)hotmail.com> --- libavutil/tests/md5: Avoid warnings Those are always showing up on Patchwork when FATE tests are failing, covering some possibly more useful information. Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-20%2Fsoftwork… Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-20/softworkz/submit_md5-v1 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/20 libavutil/tests/md5.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libavutil/tests/md5.c b/libavutil/tests/md5.c index 42e4538e0a..1f27c40987 100644 --- a/libavutil/tests/md5.c +++ b/libavutil/tests/md5.c @@ -33,8 +33,12 @@ int main(void) { uint8_t md5val[16]; int i; + +#if defined(__clang__) && defined(__clang_major__) && __clang_major__ < 4 volatile uint8_t in[1000]; // volatile to workaround http://llvm.org/bugs/show_bug.cgi?id=20849 - // FIXME remove volatile once it has been fixed and all fate clients are updated +#else + uint8_t in[1000]; +#endif for (i = 0; i < 1000; i++) in[i] = i * i; base-commit: 7512293cf912c65fa657eab680afd5e18fd47603 -- ffmpeg-codebot

5 8