[FFmpeg-devel] [PATCH v4] avfilter: fix data type for {Tile, Untile}Context's image size
These are accessed as AV_OPT_TYPE_IMAGE_SIZE AVOptions,
so must be implemented as (signed) int's
---
libavfilter/vf_tile.c | 2 +-
libavfilter/vf_untile.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavfilter/vf_tile.c b/libavfilter/vf_tile.c
index b45e739bb6..95ac3a1992 100644
--- a/libavfilter/vf_tile.c
+++ b/libavfilter/vf_tile.c
@@ -34,7 +34,7 @@
typedef struct TileContext {
const AVClass *class;
-unsigned w, h;
+int w, h;
unsigned margin;
unsigned padding;
unsigned overlap;
diff --git a/libavfilter/vf_untile.c b/libavfilter/vf_untile.c
index f32f3e186b..bfc0998b46 100644
--- a/libavfilter/vf_untile.c
+++ b/libavfilter/vf_untile.c
@@ -28,7 +28,7 @@
typedef struct UntileContext {
const AVClass *class;
-unsigned w, h;
+int w, h;
unsigned current;
unsigned nb_frames;
AVFrame *frame;
--
2.45.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 v2] avfilter: fix data type for {Tile, Untile}Context's image size
On Fri, Jul 19, 2024 at 05:31:53PM +0200, Paul B Mahol wrote: > Internal/private filter structures/API changes does not need be mentioned > in that file, isn't that fact obvious even for average Joe? The documentation[1] says "FFmpeg guarantees backward API and ABI compatibility for each library", followed by "Behavior in undocumented situations ... are accompanied by an entry in doc/APIchanges". It is not obvious to this average Joe why the AVOptions API would be an exception to that rule. Please submit a patch to libavutil/avutil.h that fixes the bug in the documentation. [1] https://ffmpeg.org/doxygen/trunk/ ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] avfilter: fix data type for {Tile, Untile}Context's image size
These are accessed as AV_OPT_TYPE_IMAGE_SIZE AVOptions,
so must be implemented as (signed) int's
---
doc/APIchanges | 6 ++
libavfilter/version_major.h | 1 +
libavfilter/vf_tile.c | 4
libavfilter/vf_untile.c | 4
4 files changed, 15 insertions(+)
diff --git a/doc/APIchanges b/doc/APIchanges
index 5751216b24..e839d1b674 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,12 @@ The last version increases of all libraries were on 2024-03-07
API changes, most recent first:
+2024-07-19 - xx - lavfi 10 - vf_tile.c
+ Convert TileContext::w and TileContext::h from unsigned to int
+
+2024-07-19 - xx - lavfi 10 - vf_untile.c
+ Convert UntileContext::w and UntileContext::h from unsigned to int
+
2024-07-xx - xx - lavf 61 - avformat.h
Deprecate avformat_transfer_internal_stream_timing_info()
and av_stream_get_codec_timebase() without replacement.
diff --git a/libavfilter/version_major.h b/libavfilter/version_major.h
index c5e660eeda..a8dc790c0a 100644
--- a/libavfilter/version_major.h
+++ b/libavfilter/version_major.h
@@ -36,5 +36,6 @@
*/
#define FF_API_LINK_PUBLIC (LIBAVFILTER_VERSION_MAJOR < 11)
+#define FF_API_TILE_SIZE_TYPE (LIBAVFILTER_VERSION_MAJOR < 11)
#endif /* AVFILTER_VERSION_MAJOR_H */
diff --git a/libavfilter/vf_tile.c b/libavfilter/vf_tile.c
index b45e739bb6..0076657c92 100644
--- a/libavfilter/vf_tile.c
+++ b/libavfilter/vf_tile.c
@@ -34,7 +34,11 @@
typedef struct TileContext {
const AVClass *class;
+#if FF_API_TILE_SIZE_TYPE
unsigned w, h;
+#else
+int w, h;
+#endif
unsigned margin;
unsigned padding;
unsigned overlap;
diff --git a/libavfilter/vf_untile.c b/libavfilter/vf_untile.c
index f32f3e186b..5164e2efb0 100644
--- a/libavfilter/vf_untile.c
+++ b/libavfilter/vf_untile.c
@@ -28,7 +28,11 @@
typedef struct UntileContext {
const AVClass *class;
+#if FF_API_TILE_SIZE_TYPE
unsigned w, h;
+#else
+int w, h;
+#endif
unsigned current;
unsigned nb_frames;
AVFrame *frame;
--
2.45.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 v2] lavu/opt: Mention that AV_OPT_TYPE_IMAGE_SIZE can be unsigned
Ping ___ 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 v3 3/3] tests/fate/source-check: Check for AVERROR codes without error strings
--- tests/fate/source-check.sh | 8 tests/ref/fate/source | 1 + 2 files changed, 9 insertions(+) diff --git a/tests/fate/source-check.sh b/tests/fate/source-check.sh index 4d7e175784..71f01cbdec 100755 --- a/tests/fate/source-check.sh +++ b/tests/fate/source-check.sh @@ -45,4 +45,12 @@ git grep -E 'av_clip *\(.*, *(-2 *, *1|-4 *, *3|-8 *, *7|-16 *, *15|-32 *, *31|- ' *, *33554431|-67108864 *, *67108863|-134217728 *, *134217727|-268435456 *, *'\ '268435455|-536870912 *, *536870911|-1073741824 *, *1073741823) *\)'| grep -v fate/source +echo "AVERROR_xxx constants with no associated error string:" +git show HEAD:libavutil/error.c \ +| sed -ne 's/.*AVERROR_\([^ ]*\).*/\1/p' libavutil/error.h \ +| while read ERROR + do git grep -q "$ERROR" libavutil/error.c \ + || echo "Please add ERROR_TAG($ERROR)" + done + exit 0 diff --git a/tests/ref/fate/source b/tests/ref/fate/source index 1703b36c02..b84cc88a6e 100644 --- a/tests/ref/fate/source +++ b/tests/ref/fate/source @@ -28,3 +28,4 @@ libavcodec/bitstream_template.h tools/decode_simple.h Use of av_clip() where av_clip_uintp2() could be used: Use of av_clip() where av_clip_intp2() could be used: +AVERROR_xxx constants with no associated error string: -- 2.45.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".
[FFmpeg-devel] [PATCH v3 2/3] avutil/error: Provide better feedback about unknown error codes
AVERROR messages should always be less than zero, and are often FourCCs.
For error codes that aren't explicitly handled by error.c (e.g. undocumented
system error codes, or internal error codes that leaked to the public API),
print the FourCC code so the user has a little more information to work with.
If a non-negative number somehow gets passed to this function,
print a message saying this shouldn't happen.
---
libavutil/error.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/libavutil/error.c b/libavutil/error.c
index 90bab7b9d3..0f748bd9e5 100644
--- a/libavutil/error.c
+++ b/libavutil/error.c
@@ -20,6 +20,7 @@
#define _XOPEN_SOURCE 600 /* XSI-compliant version of strerror_r */
#include
#include
+#include "avutil.h"
#include "config.h"
#include "avstring.h"
#include "error.h"
@@ -119,6 +120,8 @@ int av_strerror(int errnum, char *errbuf, size_t
errbuf_size)
}
if (entry) {
av_strlcpy(errbuf, entry->str, errbuf_size);
+} else if (errnum >= 0) {
+snprintf(errbuf, errbuf_size, "Impossible: non-negative error number
%d occurred, please report this bug", errnum);
} else {
#if HAVE_STRERROR_R
ret = AVERROR(strerror_r(AVUNERROR(errnum), errbuf, errbuf_size));
@@ -126,7 +129,7 @@ int av_strerror(int errnum, char *errbuf, size_t
errbuf_size)
ret = -1;
#endif
if (ret < 0)
-snprintf(errbuf, errbuf_size, "Error number %d occurred", errnum);
+snprintf(errbuf, errbuf_size, "Error number -0x%X (%s) occurred,
please report this bug", -errnum, av_fourcc2str(-errnum));
}
return ret;
--
2.45.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".
[FFmpeg-devel] [PATCH v3 1/3] avutil/utils: Allow "!" in FourCCs
For example, AVERROR_BUG is "BUG!"
---
libavutil/utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavutil/utils.c b/libavutil/utils.c
index 94d247bbee..a94589e873 100644
--- a/libavutil/utils.c
+++ b/libavutil/utils.c
@@ -81,7 +81,7 @@ char *av_fourcc_make_string(char *buf, uint32_t fourcc)
const int print_chr = (c >= '0' && c <= '9') ||
(c >= 'a' && c <= 'z') ||
(c >= 'A' && c <= 'Z') ||
- (c && strchr(". -_", c));
+ (c && strchr(". -_!", c));
const int len = snprintf(buf, buf_size, print_chr ? "%c" : "[%d]", c);
if (len < 0)
break;
--
2.45.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".
[FFmpeg-devel] [PATCH v3 0/3] Protect against undocumented error codes
I hadn't noticed av_fourcc2str() before - so long as it's patched to handle "!", it would be a clear improvement over the earlier patches. The example in the "!" patch is "BUG!", which is handled in error.c, but shows that "!" is considered a valid FourCC character. I think I understand the fate argument now, but this is my first fate patch - please review it as such! I still don't see the problem with asking for a bug report. If this patch is unacceptable, could you give an example of a code where we would reject a request to add an error string? ___ 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] [OSS-Fuzz] Have you considered enabling memory sanitizer?
On Tue, Jul 16, 2024 at 02:25:04PM +0200, Michael Niedermayer wrote: > On Mon, Jul 15, 2024 at 02:36:15PM +0200, Vittorio Giovara wrote: > > Disagree - this is not the right way to attract new contributors. > > no ? > did you do a study ? > > try this: > A. "please we need more maintainers" (we tried this i think) > B. gently push someone away > (now people are angry, they want their rights, their access, they want to > contribute ...) > ;) A little off-topic, but I suspect this points to a deeper problem: When a new person comes along and says "can I contribute my way?", replying "no, here's the correct way to do it" leaves them with two options: accept things are done that way for reasons they don't understand, or go find another project that does things the way they already understand. If you want more maintainers - that is, people with a deep understanding of the project - you need to engage new contributors in a discussion about why things are done that way. Then over the course of years, they'll learn enough to become maintainers. I realise this can be daunting, but think about it this way: if you're right about the best solution to some problem, arguing your case is a great way to weed out illogical people who shouldn't become maintainers. And if you're wrong, you can hand over a part of the project to a new maintainer, safe in the knowledge they understand that particular issue better than you. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [RFC PATCH v2] avutil/error: Provide better feedback about unknown error codes
AVERROR messages should always be less than zero,
and are usually based on three or four ASCII characters.
For error codes that aren't explicitly handled by error.c (e.g. FFERROR_REDO),
print the ASCII code so the user has a little more information.
If a non-negative number somehow gets passed to this function,
print a message saying this shouldn't happen.
---
libavutil/error.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/libavutil/error.c b/libavutil/error.c
index 90bab7b9d3..9706578be6 100644
--- a/libavutil/error.c
+++ b/libavutil/error.c
@@ -119,6 +119,40 @@ int av_strerror(int errnum, char *errbuf, size_t
errbuf_size)
}
if (entry) {
av_strlcpy(errbuf, entry->str, errbuf_size);
+} else if (
+-errnum <= 0x
+&& ((-errnum >> 0) & 0xFF) >= 0x20 && ((-errnum >> 0) & 0xFF) <= 0x7F
+&& ((-errnum >> 8) & 0xFF) >= 0x20 && ((-errnum >> 8) & 0xFF) <= 0x7F
+&& ((-errnum >> 16) & 0xFF) >= 0x20 && ((-errnum >> 16) & 0xFF) <= 0x7F
+&& (
+ (((-errnum >> 24) & 0xFF) >= 0x20 && ((-errnum >> 24) & 0xFF) <=
0x7F)
+ || !((-errnum >> 24) & 0xFF)
+)
+) {
+if ((-errnum >> 24) & 0xFF) {
+snprintf(
+errbuf,
+errbuf_size,
+"Error number -0x%x (\"%c%c%c%c\") occurred, please report
this bug",
+-errnum,
+(-errnum >> 0) & 0xFF,
+(-errnum >> 8) & 0xFF,
+(-errnum >> 16) & 0xFF,
+(-errnum >> 24) & 0xFF
+);
+} else {
+snprintf(
+errbuf,
+errbuf_size,
+"Error number -0x%x (\"%c%c%c\") occurred, please report this
bug",
+-errnum,
+(-errnum >> 0) & 0xFF,
+(-errnum >> 8) & 0xFF,
+(-errnum >> 16) & 0xFF
+);
+}
+} else if (errnum >= 0) {
+snprintf(errbuf, errbuf_size, "Impossible: non-negative error number
%d occurred, please report this bug", errnum);
} else {
#if HAVE_STRERROR_R
ret = AVERROR(strerror_r(AVUNERROR(errnum), errbuf, errbuf_size));
@@ -126,7 +160,7 @@ int av_strerror(int errnum, char *errbuf, size_t
errbuf_size)
ret = -1;
#endif
if (ret < 0)
-snprintf(errbuf, errbuf_size, "Error number %d occurred", errnum);
+snprintf(errbuf, errbuf_size, "Error number -0x%X occurred, please
report this bug", -errnum);
}
return ret;
--
2.45.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".
[FFmpeg-devel] [RFC PATCH v2 0/1] avutil/error: Provide better feedback about unknown error codes
I'm having trouble managing this conversation. On one hand, you've brought up several important details that would need to be included in a new patch. On the other hand, I'm pretty sure we're talking past each other on the big problems, and need to start over. So let's fork the discussion. # First, let's haggle over some details The patch below fixes a number of small issues brought up by your comments... Error numbers are always expressed in the code as either uppercase hex numbers or FourCCs (or ThreeCCs, but you get the point). This patch prints error codes as hex, which is no less unintelligible for ordinary users, might make problems easier to find on Google, and will sometimes make them easier to grep for. Having said that, this patch prints non-negative numbers in decimal, because all bets are off if that manages to happen. A developer could create an error code that just happens to be valid ASCII. In that situation, the previous patch would have printed something like "Unrecognised error code \"~!X\"" occurred", which is worse than the current behaviour. This patch includes both (hex) number and name in those messages. This patch adds "please report this bug" for all unknown error messages. I'll cover the reasoning below, but the relevant detail is that the previous patch just gave users a different heiroglyphic before abandoning them. # Second, let's talk about the big picture Consider the following scenario: 1. a new developer adds some code to FFmpeg that calls an existing function 2. it turns out that function sometimes calls another function that returns a variety of internal error codes (FFERROR_REDO among others) 3. their testing uncovers a situation that intermittently returns an unknown error number, but they don't notice there are two different numbers 4. they spend a lot of time tracking down an error message based on a random number, and eventually fix "the" bug (actually one of two intermittent bugs) 5. the review doesn't catch the other bug, and the new code goes live 6. a user trips over the other bug and sees "Error number occurred" 7. the user wastes a lot of time trying to work out what they did wrong, badmouthing FFmpeg to anyone who will listen as they do 8. they eventually catch the attention of a developer 9. that developer spends a lot of time bisecting the bug 10. the new developer is expected to fix this patch, and feels like they're to blame for the whole situation An error message like "Unrecognised error code \"REDO\" occurred, please report this bug" would give the newbie a fighting chance to catch both bugs at step 3, would make step 4 much shorter, and would optimise steps 7-10 to almost nothing. Catching this in a fate test would involve checking for an unknown function returning an unknown number that gets reused in a context it's subtly inappropriate for. I have no idea where to begin with that, and anyway it wouldn't help a developer in the process of tracking down an intermittent bug. As mentioned above, the v2 patch adds "please report this bug" in a few places. Any negative error code can be valid, but returning a raw error number is always a bug, so it's all the same to users - if they see this message, they're not expected to fix it themselves, they're expected to let us know. ___ 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/error: Provide better feedback about unknown error codes
On Mon, Jul 15, 2024 at 05:45:24PM +0200, Marton Balint wrote: > > > On Mon, 15 Jul 2024, Andrew Sayers wrote: > > > AVERROR messages should always be less than zero, > > and are usually based on three or four ASCII characters. > > > > For error codes that aren't explicitly handled by error.c (e.g. > > FFERROR_REDO), > > print the ASCII code so the user has a little more information. > > All ffmpeg internal error codes (including the ones having some special tag > representation) should be handled by error.c. The user should never receive > FFERROR_REDO, that is an internal error code, it should never reach the > user. Therefore I see no benefit in disclosing the error bytes, because that > is not the proper fix. > > Regards, > Marton So it sounds like this patch is addressing two separate issues: 1. any messages caught by the test in the patch represent a bug in FFmpeg * how about I modify this patch to ask the user to report the bug? * would the ASCII error code help with triage? 2. FFERROR_REDO should be added to error.c * let me know if I should submit a separate patch for this ___ 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/error: Provide better feedback about unknown error codes
AVERROR messages should always be less than zero,
and are usually based on three or four ASCII characters.
For error codes that aren't explicitly handled by error.c (e.g. FFERROR_REDO),
print the ASCII code so the user has a little more information.
If a non-negative number somehow gets passed to this function,
print a message saying this shouldn't happen.
---
libavutil/error.c | 32
1 file changed, 32 insertions(+)
diff --git a/libavutil/error.c b/libavutil/error.c
index 90bab7b9d3..c40b54b1f9 100644
--- a/libavutil/error.c
+++ b/libavutil/error.c
@@ -119,6 +119,38 @@ int av_strerror(int errnum, char *errbuf, size_t
errbuf_size)
}
if (entry) {
av_strlcpy(errbuf, entry->str, errbuf_size);
+} else if (
+-errnum <= 0x
+&& ((-errnum >> 0) & 0xFF) >= 0x20 && ((-errnum >> 0) & 0xFF) <= 0x7F
+&& ((-errnum >> 8) & 0xFF) >= 0x20 && ((-errnum >> 8) & 0xFF) <= 0x7F
+&& ((-errnum >> 16) & 0xFF) >= 0x20 && ((-errnum >> 16) & 0xFF) <= 0x7F
+&& (
+ (((-errnum >> 24) & 0xFF) >= 0x20 && ((-errnum >> 24) & 0xFF) <=
0x7F)
+ || !((-errnum >> 24) & 0xFF)
+)
+) {
+if ((-errnum >> 24) & 0xFF) {
+snprintf(
+errbuf,
+errbuf_size,
+"Unrecognised error code \"%c%c%c%c\" occurred",
+(-errnum >> 0) & 0xFF,
+(-errnum >> 8) & 0xFF,
+(-errnum >> 16) & 0xFF,
+(-errnum >> 24) & 0xFF
+);
+} else {
+snprintf(
+errbuf,
+errbuf_size,
+"Unrecognised error code \"%c%c%c\" occurred",
+(-errnum >> 0) & 0xFF,
+(-errnum >> 8) & 0xFF,
+(-errnum >> 16) & 0xFF
+);
+}
+} else if (errnum >= 0) {
+snprintf(errbuf, errbuf_size, "Impossible: non-negative error number
%d occurred", errnum);
} else {
#if HAVE_STRERROR_R
ret = AVERROR(strerror_r(AVUNERROR(errnum), errbuf, errbuf_size));
--
2.45.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 v2] lavu/opt: Mention that AV_OPT_TYPE_IMAGE_SIZE can be unsigned
On Wed, Jul 10, 2024 at 04:01:44PM +0200, Paul B Mahol wrote: > tile and untile are wrong How so? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] lavu/opt: Mention that AV_OPT_TYPE_IMAGE_SIZE can be unsigned
TileContext in libavfilter/vf_tile.c and
UntileContext in libavfilter/vf_untile.c
point to unsigned ints - confirm this is OK.
---
Thanks Marcus - have updated my e-mail script to remind myself in future.
libavutil/opt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..9339b1a6ac 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -252,7 +252,7 @@ enum AVOptionType{
AV_OPT_TYPE_DICT,
AV_OPT_TYPE_UINT64,
AV_OPT_TYPE_CONST,
-AV_OPT_TYPE_IMAGE_SIZE, ///< offset must point to two consecutive ints
+AV_OPT_TYPE_IMAGE_SIZE, ///< offset must point to two consecutive ints (or
unsigned ints)
AV_OPT_TYPE_PIXEL_FMT,
AV_OPT_TYPE_SAMPLE_FMT,
AV_OPT_TYPE_VIDEO_RATE, ///< offset must point to AVRational
--
2.45.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".
[FFmpeg-devel] [PATCH] Mention that AV_OPT_TYPE_IMAGE_SIZE can be unsigned
TileContext in libavfilter/vf_tile.c and
UntileContext in libavfilter/vf_untile.c
point to unsigned ints - confirm this is OK.
---
libavutil/opt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..9339b1a6ac 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -252,7 +252,7 @@ enum AVOptionType{
AV_OPT_TYPE_DICT,
AV_OPT_TYPE_UINT64,
AV_OPT_TYPE_CONST,
-AV_OPT_TYPE_IMAGE_SIZE, ///< offset must point to two consecutive ints
+AV_OPT_TYPE_IMAGE_SIZE, ///< offset must point to two consecutive ints (or
unsigned ints)
AV_OPT_TYPE_PIXEL_FMT,
AV_OPT_TYPE_SAMPLE_FMT,
AV_OPT_TYPE_VIDEO_RATE, ///< offset must point to AVRational
--
2.45.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] [RFC]] swscale modernization proposal
On Mon, Jul 08, 2024 at 07:58:44AM -0400, Ronald S. Bultje wrote: > On Sat, Jul 6, 2024 at 1:29 PM Hendrik Leppkes wrote: > > On Sat, Jul 6, 2024 at 6:42 PM Michael Niedermayer [...] > > > > The entire point of presets is to have them provide a predefined set > > > > of parameters, easy for users to pick one value, rather than a bunch. > > > > And what a preset actually means should be documented. > > > > How do you define "presets" if they don't hardcode a list of choices > > > > for all the relevant options? > > > > > > > > Advanced settings exist for a user to select any particular detail, if > > > > they so desire. > > > > > > The problem is if new features are added and you have a hardcoded list in > > > the API what each quality corresponds to change it you have to bump major > > > > > > also, do we really have or want to have optimized nearest neighbor scaler > > > code ? > > > If not the AV_SCALE_ULTRAFAST could be slower than AV_SCALE_VERYFAST > > > simply because it now "has to" do something we actually have not > > optimized > > > > > > > So.. you object to the comments that explain what it does? > > Someone that uses presets will never have a guarantee to the selected > > algorithms and options > > > > But then why did we provide this information? It's one thing to have it in > a stackoverflow answer re: a specific FFmpeg version, but in a header, it > feels much more ... burdening. Even if no actual API linkage occurred. That burden exists no matter what, documentation just puts it on the shoulders of a small number of developers who understand the problem; instead of a large number of users who have to hope an SO answer isn't out-of-date yet. We often say e.g. "this struct currently has such-and-such members, but the size is not part of the public API". So it's not much of a stretch to say "this preset enables such-and-such features, but the value is not part of the public API". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 0/3] s/RUNTIME/POST_INIT_SETTABLE/
On Sat, Jul 06, 2024 at 06:49:51PM +0200, Michael Niedermayer wrote: > On Sat, Jul 06, 2024 at 12:40:07PM +0200, Paul B Mahol wrote: > [...] > > > its much more text to type too. > > shorter options: > > AV_OPT_FLAG_ALTERABLE_PARAM > > AV_OPT_FLAG_MUTABLE_PARAM > > (just in case consensus ends on a rename, i am not sure a rename is a good > idea here) I'm fine with someone doing s/POST_INIT_SETTABLE/MUTABLE/g on the patch before committing it if they feel that's better, but ALTERABLE seems like a bad idea. I guess you could argue a sufficiently inattentive reader might read "POST_INIT_SETTABLE" to mean "*only* settable after init", even though the documentation says otherwise. But such a reader might also read "ALTERABLE" to somehow mean "data type can be altered" (like SQL's ALTER TABLE statement), whereas "mutable" conventionally means "contents can be altered". But this is just a nitpick - POST_INIT_SETTABLE is still fine by me. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC]] swscale modernization proposal
On Fri, Jul 05, 2024 at 11:34:06PM +0200, Michael Niedermayer wrote: > On Fri, Jul 05, 2024 at 08:31:17PM +0200, Niklas Haas wrote: [...] > > > Attached is my revised working draft of . > > I dont agree to the renaming of swscale, that is heading toward Would you mind talking a bit more about this distinction? I feel like I'm missing something important here. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 0/3] s/RUNTIME/POST_INIT_SETTABLE/
On Sat, Jul 06, 2024 at 11:37:19AM +0200, Stefano Sabatini wrote:
> On date Tuesday 2024-07-02 10:08:37 +0100, Andrew Sayers wrote:
[...]
> While I agree with Anton that we should avoid duplication, for the
> usual arguments that a reference should avoid duplication of content
> as much as possible, which inevitably leads to inconsistent content
> when it is updated partially, leading to inconsistent information.
I think we're mostly just bikeshedding about how to balance discoverability
with duplication, but feel obliged to point out a third option. Our Doxygen
config has MACRO_EXPANSION enabled both in the repo and on the site,
so we can use macros to avoid duplicating documentation:
#define BOILERPLATE \
/** Lorem ipsum dolor sit amet, consectetur adipiscing elit */
/**
* @brief Main function
*
*/
BOILERPLATE
int main() {}
I think that's too weird a hack, but happy to propose a patch if you disagree.
___
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 1/8] swscale: document SWS_FULL_CHR_H_* flags
On Thu, Jul 04, 2024 at 07:59:10PM +0200, Niklas Haas wrote: > On Thu, 04 Jul 2024 19:56:56 +0200 Niklas Haas wrote: > > On Thu, 04 Jul 2024 16:24:24 +0100 Andrew Sayers > > wrote: > > > On Thu, Jul 04, 2024 at 04:30:57PM +0200, Niklas Haas wrote: > > > > From: Niklas Haas > > > > > > > > Based on my best understanding of what they do, given the source code. > > > > --- > > > > libswscale/swscale.h | 28 ++-- > > > > 1 file changed, 26 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/libswscale/swscale.h b/libswscale/swscale.h > > > > index 9d4612aaf3..e22931cab4 100644 > > > > --- a/libswscale/swscale.h > > > > +++ b/libswscale/swscale.h > > > > @@ -82,11 +82,35 @@ const char *swscale_license(void); > > > > #define SWS_PRINT_INFO 0x1000 > > > > > > > > //the following 3 flags are not completely implemented > > > > -//internal chrominance subsampling info > > > > + > > > > +/** > > > > + * Perform full chroma upsampling when converting to RGB as part of > > > > scaling. > > > > > > Nitpick: "as part of scaling" seems redundant - can it be removed? > > > > I wrote it this way because, afaict, this flag does not affect unscaled > > special converters (yuv->rgba). But I can remove it if you still think > > it's unnecessary. > > How about: "Perform full chroma upsampling when upscaling to RGB"? Ah, I hadn't understood that distinction at all. I'd recommend... 1. keep the original if this applies to both up- and down-scaling 2. use the second if it's just for upscaling 3. either way, add a line like this at the end of the section: Note: this flag is ignored by unscaled special converters. I realise this patch is just documenting current behaviour, and I'm not saying that behaviour is correct or incorrect, but it seems important and certainly wasn't intuitive to me. So it's worth mentioning a bit louder :) ___ 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 1/8] swscale: document SWS_FULL_CHR_H_* flags
On Thu, Jul 04, 2024 at 04:30:57PM +0200, Niklas Haas wrote: > From: Niklas Haas > > Based on my best understanding of what they do, given the source code. > --- > libswscale/swscale.h | 28 ++-- > 1 file changed, 26 insertions(+), 2 deletions(-) > > diff --git a/libswscale/swscale.h b/libswscale/swscale.h > index 9d4612aaf3..e22931cab4 100644 > --- a/libswscale/swscale.h > +++ b/libswscale/swscale.h > @@ -82,11 +82,35 @@ const char *swscale_license(void); > #define SWS_PRINT_INFO 0x1000 > > //the following 3 flags are not completely implemented > -//internal chrominance subsampling info > + > +/** > + * Perform full chroma upsampling when converting to RGB as part of scaling. Nitpick: "as part of scaling" seems redundant - can it be removed? > + * > + * For example, when converting 50x50 yuv420p to 100x100 rgba, setting this > flag > + * will scale the chroma plane from 25x25 to 100x100 (4:4:4), and then > convert > + * the 100x100 yuv444p image to rgba in the final output step. > + * > + * Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2), > + * with a single chroma sample being re-used for both horizontally adjacent > RGBA > + * output pixels. Nitpick: this would be more readable as "for both of the...". Consider the following sentence: Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2), with a single chroma sample being re-used for both horizontally and vertically adjacent RGBA output pixels. Using "both of the" would make it clear what "both" refers to before the reader starts doing branch-prediction in their head. Otherwise, LGTM (by which I mean it's clear, not that I know whether it's correct). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 2/3] lavu/opt: Mention AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM in more places
On Tue, Jul 02, 2024 at 12:16:24PM +0200, Anton Khirnov wrote: > Quoting Andrew Sayers (2024-07-02 12:13:16) > > On Tue, Jul 02, 2024 at 11:52:29AM +0200, Anton Khirnov wrote: > > > Quoting Andrew Sayers (2024-07-02 11:08:39) > > > > An inattentive user might not see the explanation at the top of this > > > > file. > > > > Paste the explanation to all the places they might see it. > > > > > > Duplication is bad, and the premise doesn't work anyway. Inattentive > > > users will happily ignore arbitrary amounts of text. In fact the more > > > text there is, the more of it they will ignore. > > > Meanwhile you make actual information harder to find for people who > > > actually bother to read. > > > > That's a reasonable argument, but incompatible with your other one[1]. > > If users are inattentive and will ignore arbitrary amounts of text, > > the explanation needs to go in the one place they have to look (the > > macro name). > > I don't understand your point. In my other email I'm objecting to > breaking API for flimsy reasons, how is that related to documentation? I could be wrong, but I think this points to a fundamental issue... We normally talk about ABI (binary interface) and API (programming interface), then say "documentation" as if that's some separate thing. It would have been better if programmers had used a term like "ADI" (developer interface) instead, but the world didn't go that way. API is as intermingled with documentation as it is with ABI, and drawing a bright line between the two just causes problems. In this case, spelling it "AV_OPT_FLAG_RUNTIME_PARAM" isn't API, it's documentation. The compiler would work just the same if it had been called e.g. "AV_OPT_FLAG_15", the name is just there for us humans. This patch is about fixing the documentation, so the primary justification is that the old spelling mislead humans. Breaking the API is a side-effect, and I'd argue it's a net benefit, because it nudges external devs to fix their code. You can make the opposite argument, but either way it's incidental to the main goal of picking a spelling that unambiguously documents what the macro does. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 2/3] lavu/opt: Mention AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM in more places
On Tue, Jul 02, 2024 at 11:52:29AM +0200, Anton Khirnov wrote: > Quoting Andrew Sayers (2024-07-02 11:08:39) > > An inattentive user might not see the explanation at the top of this file. > > Paste the explanation to all the places they might see it. > > Duplication is bad, and the premise doesn't work anyway. Inattentive > users will happily ignore arbitrary amounts of text. In fact the more > text there is, the more of it they will ignore. > Meanwhile you make actual information harder to find for people who > actually bother to read. That's a reasonable argument, but incompatible with your other one[1]. If users are inattentive and will ignore arbitrary amounts of text, the explanation needs to go in the one place they have to look (the macro name). [1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-July/330559.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Development process for explaining contexts (was Re: [PATCH v6 1/4] doc: Explain what "context" means)
On Tue, Jul 02, 2024 at 12:16:21AM +0200, Stefano Sabatini wrote: > On date Sunday 2024-06-16 19:02:51 +0100, Andrew Sayers wrote: [...] > > Andrew, sorry again for the slow reply. Thinking about the whole > discussion, I reckon I probably gave some bad advice, and I totally > understand how this is feeling dragging and burning out, and I'm sorry > for that. > > I'm still on the idea of erring on the side of under-communicating for > the reference documentation (with the idea that too much information > is just too much, and would scare people away and make it harder to > maintain the documentation, as now you have to check in many places > when changing/updating it, resulting in contradicting content). > > So at the moment I'd be willing to publish an abridged version of your > latest patch, with the suggested cuts - I can make the edit myself if > you prefer like that. This way we can get the non controversial parts > committed, and we can work on the other parts where there is no still > agreement. > > Also, I'd like to hear opinions from other developers, although my > impression - from the scattered feedback I read - is that other > developers have the same feeling as me. > > In general, having different channels for different targets would be > ideal, e.g. for articles and tutorials. For this it would be ideal to > have a blog entry for the project org, to simplify contributions from > contributors who don't want to setup a blog just for that and to > collect resources in a single place. In practice we lack this so this > is not an option at the moment (and the wiki is not the ideal place > too). No problem about the delay, although my thinking has moved on a little (e.g. it turns out GIMP uses the word "context" in a completely different way than we do[1]). But rather than argue over today's minutia, here's a big picture idea... It sounds like your vision is for smaller, more disparate documentation; and you're willing to spend some time writing that up. How would you feel about taking the AVClass/AVOptions bits from this document, and working them in to the existing AVClass/AVOptions documentation? That would require a level of experience (and commit access) beyond what I can offer, after which we could come back here and uncontroversially trim that stuff out of this document. For inspiration, here are some uninformed questions a newbie might ask: * (reading AVClass) does the struct name mean I have to learn OOP before I can use FFmpeg? * (reading AVOptions) if the options API only works post-init for a subset of options, should I just ignore this API and set the variables directly whenever I like? [1] https://developer.gimp.org/api/2.0/libgimp/libgimp-gimpcontext.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v4 3/3] all: s/AV_OPT_FLAG_RUNTIME_PARAM/AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM/g
Use the new name for the macro throughout the codebase. Patch generated with the following command: sed -i -e 's/AV_OPT_FLAG_RUNTIME_PARAM/AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM/g' \ $( git grep -l AV_OPT_FLAG_RUNTIME_PARAM | grep -v '^libavutil/opt.h' | grep -v '^doc/APIchanges$' ) --- libavfilter/af_aap.c | 2 +- libavfilter/af_acrusher.c | 2 +- libavfilter/af_adelay.c| 2 +- libavfilter/af_adenorm.c | 2 +- libavfilter/af_adrc.c | 2 +- libavfilter/af_adynamicequalizer.c | 2 +- libavfilter/af_adynamicsmooth.c| 2 +- libavfilter/af_aemphasis.c | 2 +- libavfilter/af_aexciter.c | 2 +- libavfilter/af_afade.c | 2 +- libavfilter/af_afftdn.c| 2 +- libavfilter/af_afir.c | 2 +- libavfilter/af_afreqshift.c| 2 +- libavfilter/af_afwtdn.c| 2 +- libavfilter/af_agate.c | 2 +- libavfilter/af_alimiter.c | 2 +- libavfilter/af_amix.c | 2 +- libavfilter/af_anlmdn.c| 2 +- libavfilter/af_anlms.c | 2 +- libavfilter/af_apsyclip.c | 2 +- libavfilter/af_arls.c | 2 +- libavfilter/af_arnndn.c| 2 +- libavfilter/af_asetnsamples.c | 2 +- libavfilter/af_asoftclip.c | 2 +- libavfilter/af_asubboost.c | 2 +- libavfilter/af_asupercut.c | 2 +- libavfilter/af_atempo.c| 2 +- libavfilter/af_atilt.c | 2 +- libavfilter/af_biquads.c | 2 +- libavfilter/af_compensationdelay.c | 2 +- libavfilter/af_crossfeed.c | 2 +- libavfilter/af_crystalizer.c | 2 +- libavfilter/af_dialoguenhance.c| 2 +- libavfilter/af_dynaudnorm.c| 2 +- libavfilter/af_extrastereo.c | 2 +- libavfilter/af_firequalizer.c | 2 +- libavfilter/af_rubberband.c| 2 +- libavfilter/af_sidechaincompress.c | 2 +- libavfilter/af_silenceremove.c | 2 +- libavfilter/af_speechnorm.c| 2 +- libavfilter/af_stereotools.c | 2 +- libavfilter/af_stereowiden.c | 2 +- libavfilter/af_surround.c | 2 +- libavfilter/af_virtualbass.c | 2 +- libavfilter/af_volume.c| 2 +- libavfilter/avf_a3dscope.c | 2 +- libavfilter/avf_avectorscope.c | 2 +- libavfilter/avfilter.c | 4 ++-- libavfilter/f_graphmonitor.c | 2 +- libavfilter/f_perms.c | 2 +- libavfilter/f_realtime.c | 2 +- libavfilter/f_streamselect.c | 2 +- libavfilter/qrencode.c | 2 +- libavfilter/setpts.c | 2 +- libavfilter/src_avsynctest.c | 2 +- libavfilter/vf_amplify.c | 2 +- libavfilter/vf_atadenoise.c| 2 +- libavfilter/vf_avgblur.c | 2 +- libavfilter/vf_backgroundkey.c | 2 +- libavfilter/vf_bbox.c | 2 +- libavfilter/vf_bilateral.c | 2 +- libavfilter/vf_blend.c | 2 +- libavfilter/vf_cas.c | 2 +- libavfilter/vf_chromakey.c | 2 +- libavfilter/vf_chromanr.c | 2 +- libavfilter/vf_chromashift.c | 2 +- libavfilter/vf_colorbalance.c | 2 +- libavfilter/vf_colorchannelmixer.c | 2 +- libavfilter/vf_colorcontrast.c | 2 +- libavfilter/vf_colorcorrect.c | 2 +- libavfilter/vf_colorize.c | 2 +- libavfilter/vf_colorkey.c | 2 +- libavfilter/vf_colorlevels.c | 2 +- libavfilter/vf_colormap.c | 2 +- libavfilter/vf_colortemperature.c | 2 +- libavfilter/vf_convolution.c | 2 +- libavfilter/vf_crop.c | 2 +- libavfilter/vf_cropdetect.c| 2 +- libavfilter/vf_curves.c| 2 +- libavfilter/vf_datascope.c | 2 +- libavfilter/vf_dblur.c | 2 +- libavfilter/vf_deband.c| 2 +- libavfilter/vf_deblock.c | 2 +- libavfilter/vf_despill.c | 2 +- libavfilter/vf_displace.c | 2 +- libavfilter/vf_drawbox.c | 2 +- libavfilter/vf_drawtext.c | 2 +- libavfilter/vf_eq.c| 2 +- libavfilter/vf_estdif.c| 2 +- libavfilter/vf_exposure.c | 2 +- libavfilter/vf_feedback.c | 2 +- libavfilter/vf_fftdnoiz.c | 2 +- libavfilter/vf_fillborders.c | 2 +- libavfilter/vf_frei0r.c| 2 +- libavfilter/vf_gblur.c | 2 +- libavfilter/vf_guided.c| 2 +- libavfilter/vf_hqdn3d.c| 2 +- libavfilter/vf_hsvkey.c| 2 +- libavfilter/vf_hue.c | 2 +- libavfilter/vf_huesaturation.c | 2 +- libavfilter/vf_il.c| 2 +- libavfilter/vf_lagfun.c| 2 +- libavfilter/vf_lenscorrection.c| 2 +- libavfilter/vf_libplacebo.c| 2 +- libavfilter/vf_limitdiff.c | 2 +- libavfilter/vf_limiter.c | 2 +- libavfilter/vf_lumakey.c | 2 +- libavfilter/vf_lut.c | 2 +- libavfilter/vf_lut2.c | 2 +- libavfilter/vf_l
[FFmpeg-devel] [PATCH v4 2/3] lavu/opt: Mention AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM in more places
An inattentive user might not see the explanation at the top of this file.
Paste the explanation to all the places they might see it.
---
libavutil/opt.h | 21 +
1 file changed, 21 insertions(+)
diff --git a/libavutil/opt.h b/libavutil/opt.h
index b78c3406fa..289ae9f410 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -496,6 +496,9 @@ typedef struct AVOptionRanges {
/**
* Set the values of all AVOption fields to their default values.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param s an AVOption-enabled struct (its first member must be a pointer to
AVClass)
*/
void av_opt_set_defaults(void *s);
@@ -505,6 +508,9 @@ void av_opt_set_defaults(void *s);
* AVOption fields for which (opt->flags & mask) == flags will have their
* default applied to s.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param s an AVOption-enabled struct (its first member must be a pointer to
AVClass)
* @param mask combination of AV_OPT_FLAG_*
* @param flags combination of AV_OPT_FLAG_*
@@ -674,6 +680,9 @@ enum {
* key. ctx must be an AVClass context, storing is done using
* AVOptions.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param opts options string to parse, may be NULL
* @param key_val_sep a 0-terminated list of characters used to
* separate key from value
@@ -692,6 +701,9 @@ int av_set_options_string(void *ctx, const char *opts,
* Parse the key-value pairs list in opts. For each key=value pair found,
* set the value of the corresponding option in ctx.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param ctx the AVClass object to set options on
* @param opts the options string, key-value pairs separated by a
* delimiter
@@ -722,6 +734,9 @@ int av_opt_set_from_string(void *ctx, const char *opts,
/**
* Set all the options from a given dictionary on an object.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param obj a struct whose first element is a pointer to AVClass
* @param options options to process. This dictionary will be freed and
replaced
*by a new one containing all options not found in obj.
@@ -739,6 +754,9 @@ int av_opt_set_dict(void *obj, struct AVDictionary
**options);
/**
* Set all the options from a given dictionary on an object.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param obj a struct whose first element is a pointer to AVClass
* @param options options to process. This dictionary will be freed and
replaced
*by a new one containing all options not found in obj.
@@ -777,6 +795,9 @@ int av_opt_copy(void *dest, const void *src);
* @{
* Those functions set the field of obj with the given name to value.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param[in] obj A struct whose first element is a pointer to an AVClass.
* @param[in] name the name of the field to set
* @param[in] val The value to set. In case of av_opt_set() if the field is not
--
2.45.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".
[FFmpeg-devel] [PATCH v4 1/3] lavu/opt: Rename AV_OPT_FLAG_RUNTIME_PARAM to ...POST_INIT_SETTABLE_PARAM
The old name could be misread as the opposite of "AV_OPT_FLAG_READONLY" -
some things can be set at runtime, others are read-only. Clarify that
this refers to options that can be set after the struct is initialized.
---
doc/APIchanges | 4
libavutil/opt.h | 15 ++-
libavutil/version.h | 3 ++-
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index f1828436e5..8217c391cb 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,10 @@ The last version increases of all libraries were on 2024-03-07
API changes, most recent first:
+2024-07-02 - xx - lavu 59.28.100 - opt.h
+ Deprecate AV_OPT_FLAG_RUNTIME_PARAM and replace it with
+ AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM.
+
2024-06-28 - xx - lavu 59.27.100 - stereo3d.h
Add AV_STEREO3D_UNSPEC and AV_STEREO3D_VIEW_UNSPEC.
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..b78c3406fa 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -53,6 +53,9 @@
* question is allowed to access the field. This allows us to extend the
* semantics of those fields without breaking API compatibility.
*
+ * Note: only options with the AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be
+ * modified after the struct is initialized.
+ *
* @section avoptions_scope Scope of AVOptions
*
* AVOptions is designed to support any set of multimedia configuration options
@@ -300,9 +303,19 @@ enum AVOptionType{
#define AV_OPT_FLAG_BSF_PARAM (1 << 8)
/**
- * A generic parameter which can be set by the user at runtime.
+ * A generic parameter which can be set by the user after the struct is
initialized.
+ */
+#define AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM (1 << 15)
+#if FF_API_OPT_FLAG_RUNTIME_PARAM
+/**
+ * A generic parameter which can be set by the user after the struct is
initialized.
+ *
+ * @deprecated Renamed for clarity - to continue using this feature,
+ * please do s/AV_OPT_FLAG_RUNTIME_PARAM/AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM/g
+ * throughout your codebase
*/
#define AV_OPT_FLAG_RUNTIME_PARAM (1 << 15)
+#endif
/**
* A generic parameter which can be set by the user for filtering.
*/
diff --git a/libavutil/version.h b/libavutil/version.h
index a8962734e7..c03681f802 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 59
-#define LIBAVUTIL_VERSION_MINOR 27
+#define LIBAVUTIL_VERSION_MINOR 28
#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
@@ -113,6 +113,7 @@
#define FF_API_VULKAN_CONTIGUOUS_MEMORY (LIBAVUTIL_VERSION_MAJOR < 60)
#define FF_API_H274_FILM_GRAIN_VCS (LIBAVUTIL_VERSION_MAJOR < 60)
#define FF_API_MOD_UINTP2 (LIBAVUTIL_VERSION_MAJOR < 60)
+#define FF_API_OPT_FLAG_RUNTIME_PARAM (LIBAVUTIL_VERSION_MAJOR < 60)
/**
* @}
--
2.45.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".
[FFmpeg-devel] [PATCH v4 0/3] s/RUNTIME/POST_INIT_SETTABLE/
Some notes about this version: As previously mentioned, I think this is better with all three patches, but can live without #2. I think I've followed the deprecation instructions, but editing APIchanges seems to imply needing a minor version bump? This patch includes said bump. Zhao Zhili argued this bothers users for too little benefit - I'd argue it's beneficial to confirm they haven't misused this feature in a way that causes bugs in their code, but the old deprecation message didn't make that clear enough. This version rephrases the @deprecated message to speak directly to the maintainer. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC]] swscale modernization proposal
On Sun, Jun 23, 2024 at 02:57:31PM -0300, James Almer wrote: > On 6/22/2024 7:19 PM, Vittorio Giovara wrote: > > Needless to say I support the plan of renaming the library so that it can > > be inline with the other libraries names, and the use of a separate header > > since downstream applications will need to update a lot to use the new > > library (or the new apis in the existing library) and/or we could provide a > > thin conversion layer when the new lib is finalized. > > I don't quite agree with renaming it. As Michael already pointed out, the av > prefix wouldn't fit a scaling library nor a resampling one, as they only > handle one or the other. > There's also the precedent of avresample, which was ultimately dropped in > favor of swresample, so trying to replace swscale with a new avscale library > will be both confusing and going against what was already established. It wouldn't confuse users, because the meaning isn't documented. Can you summarise what information "av" vs. "sw" prefixes conveys to API users? Preferably in the form of a patch to @mainpage section of libavutil/avutil.h :) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC]] swscale modernization proposal
On Sat, Jun 22, 2024 at 03:13:34PM +0200, Niklas Haas wrote: [...] > > ## Comments / feedback? > > Does the above approach seem reasonable? How do people feel about introducing > a new API vs. trying to hammer the existing API into the shape I want it to > be? > > I've attached an example of what could end up looking like. If > there is broad agreement on this design, I will move on to an implementation. API users seem to have difficulty with this type of big change[[1], and doing the interface before the implementation means there's less reason for developers to switch while you're still looking for feedback. What's the plan to bring them along? [1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-June/328852.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/af_afade: fix opt_type for nb_samples/ns
On Sat, Jun 22, 2024 at 03:20:52PM +0200, Andreas Rheinhardt wrote:
> Andrew Sayers:
> > The actual value is an int64_t, and is accessed elsewhere as
> > AV_OPT_TYPE_INT64.
> >
> > Accessing it as INT will likely cause bugs on some 32-bit architectures.
>
> Whether this works or not will depend upon endianness, not on whether
> the architecture is 32-bit (as long as int is 32bits, which is mostly
> true for 64-bit architectures).
>
> > ---
> > libavfilter/af_afade.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavfilter/af_afade.c b/libavfilter/af_afade.c
> > index 3a45873460..c79271ec92 100644
> > --- a/libavfilter/af_afade.c
> > +++ b/libavfilter/af_afade.c
> > @@ -452,8 +452,8 @@ const AVFilter ff_af_afade = {
> > #if CONFIG_ACROSSFADE_FILTER
> >
> > static const AVOption acrossfade_options[] = {
> > -{ "nb_samples", "set number of samples for cross fade duration",
> > OFFSET(nb_samples), AV_OPT_TYPE_INT,{.i64 = 44100}, 1, INT32_MAX/10,
> > FLAGS },
> > -{ "ns", "set number of samples for cross fade duration",
> > OFFSET(nb_samples), AV_OPT_TYPE_INT,{.i64 = 44100}, 1, INT32_MAX/10,
> > FLAGS },
> > +{ "nb_samples", "set number of samples for cross fade duration",
> > OFFSET(nb_samples), AV_OPT_TYPE_INT64, {.i64 = 44100}, 1, INT32_MAX/10,
> > FLAGS },
> > +{ "ns", "set number of samples for cross fade duration",
> > OFFSET(nb_samples), AV_OPT_TYPE_INT64, {.i64 = 44100}, 1, INT32_MAX/10,
> > FLAGS },
> > { "duration", "set cross fade duration",
> > OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0 }, 0, 6000,
> > FLAGS },
> > { "d","set cross fade duration",
> > OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0 }, 0, 6000,
> > FLAGS },
> > { "overlap", "overlap 1st stream end with 2nd stream start",
> > OFFSET(overlap), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FLAGS },
>
> LGTM. How did you find this?
It was a side-effect of yet another attempt to understand the codebase -
looking for struct members with more than one associated AVOptionType.
FWIW, the other oddities uncovered were:
* libavformat/rtsp.c treats stimeout as both INT64 and DURATION
(should probably have been caught at the time, but fixing it would likely
break scripts that expect the current CLI behaviour)
* libavdevice/v4l2.c treats list_format as both INT and CONST
(not a bug, but a comment explaining this clever trick would have been nice)
* libavfilter/buffersrc.c lets you set w/h separately or as video_size
(not a bug, and I assume there's a reason for doing it this way)
___
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] avfilter/af_afade: fix opt_type for nb_samples/ns
The actual value is an int64_t, and is accessed elsewhere as AV_OPT_TYPE_INT64.
Accessing it as INT will likely cause bugs on some 32-bit architectures.
---
libavfilter/af_afade.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavfilter/af_afade.c b/libavfilter/af_afade.c
index 3a45873460..c79271ec92 100644
--- a/libavfilter/af_afade.c
+++ b/libavfilter/af_afade.c
@@ -452,8 +452,8 @@ const AVFilter ff_af_afade = {
#if CONFIG_ACROSSFADE_FILTER
static const AVOption acrossfade_options[] = {
-{ "nb_samples", "set number of samples for cross fade duration",
OFFSET(nb_samples), AV_OPT_TYPE_INT,{.i64 = 44100}, 1, INT32_MAX/10,
FLAGS },
-{ "ns", "set number of samples for cross fade duration",
OFFSET(nb_samples), AV_OPT_TYPE_INT,{.i64 = 44100}, 1, INT32_MAX/10,
FLAGS },
+{ "nb_samples", "set number of samples for cross fade duration",
OFFSET(nb_samples), AV_OPT_TYPE_INT64, {.i64 = 44100}, 1, INT32_MAX/10,
FLAGS },
+{ "ns", "set number of samples for cross fade duration",
OFFSET(nb_samples), AV_OPT_TYPE_INT64, {.i64 = 44100}, 1, INT32_MAX/10,
FLAGS },
{ "duration", "set cross fade duration",
OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0 }, 0, 6000, FLAGS },
{ "d","set cross fade duration",
OFFSET(duration), AV_OPT_TYPE_DURATION, {.i64 = 0 }, 0, 6000, FLAGS },
{ "overlap", "overlap 1st stream end with 2nd stream start",
OFFSET(overlap), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FLAGS },
--
2.45.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
On Mon, Jun 17, 2024 at 09:29:57PM +0200, Vittorio Giovara wrote: > On Mon, Jun 17, 2024 at 9:01 PM Nicolas George wrote: > > > Michael Niedermayer (12024-06-17): > > > also if you look at google trends, even today more people search for > > ffserver > > > than txproto. In fact at every point in time more people searched for > > ffserver > > > than txproto. > > > > > > https://trends.google.com/trends/explore?date=all&q=txproto,ffserver > > > > > > So even though ffserver is dead, removed and unmaintained, it has more > > > users > > > > > > And this comes back to what i said many times. We should use the name > > > FFmpeg, our domain and NOT push every bit of new inovation out into > > > sub projects. > > > > > > We should put a newly developed ffserver into the main ffmpeg git. > > > We should put wasm build support into the main ffmpeg git. > > > We should turn ffplay into a fully competetive player. > > > ... > > > > Hear! Hear! > > > > I would add, as general guiding principles: > > > > We should provide both low- and high-level APIs. Ideally, the fftools > > should be just user interface around the high-level APIs provided by the > > libraries. > > > Patches welcome? Not that it means anything, but you had exactly 2 lines on > ffserver before it got removed, so I wonder who exactly you think should be > maintaining all that cruft code (honest question, if you have a real plan > for solving this ageless problem I think many people on the ML would be > interested) This isn't a hard problem to solve, just a boring one: If you want more contributions, you need more contributors. If you want more contributors, you need to make it easy to get started. If you want to make it easy to get started, focus on the tedious things that trip newbies up, not the interesting problems you'd like them to have. You talked elsewhere about moving to a modern UI. That's a fine long-term goal, but how about starting simple - instead of waiting for patches to go stale and making people beg, apply patches 48 hours after the thread goes quiet, then revert them if someone asks for more time to review. ___ 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] Development process for explaining contexts (was Re: [PATCH v6 1/4] doc: Explain what "context" means)
Meta note #1: I've replied in this thread but changed the subject line. That's because it needs to stay focussed on solving this thread's problem, but may be of more general interest. Meta note #2: Stefano, I appreciate your feedback, but would rather wait for [1] to get sorted out, then formulate my thoughts while writing a new version. That way I'll be more focussed on ways to improve things for readers. This thread started with what I thought was a trivia question[1] - what is a context? It's short for "AVClass context structure", which is synonymous with "AVOptions-enabled struct". It turned out to be more complex than that, so I wrote a little patch[3] explaining this piece of jargon. But it turned out to be more complex again, and so on until we got a 430-line document explaining things in voluminous detail. Everyone agrees this isn't ideal, so here are some alternatives. This may also inspire thoughts about FFmpeg development in general. # Alternative: Just give up The argument: We tried something, learnt a lot, but couldn't find a solution we agreed on, so let's come back another day. Obviously this is the easy way out, but essentially means leaving a critical bug in the documentation (misleads the reader about a fundamental concept). Even the most negative take on this document is that it's better than nothing, so I think we can rule this one out. # Err on the side of under-communicating The argument: this document is on the right tracks, but explains too many things the reader can already be assumed to know. This argument is more complex than it appears. To take some silly examples, I'm not going to learn Mandarin just because FFmpeg users can't be assumed to speak English. But I am willing to use American spelling because it's what more readers are used to. This e-mail is plenty long enough already, so I'll stick to some high-level points about this argument. The main risk of cutting documentation is that if someone can't follow a single step, they're lost and don't even know how to express their problem. Imagine teaching maths to children - you need to teach them what numbers are, then how to add them together, then multiplication, then finally exponents. But if you say "we don't need to teach numbers because kids all watch Numberblocks now", you'll cover the majority of kids who could have worked it out anyway, and leave a minority who just give up and say "I guess I must be bad at maths". I'd argue it's better to write more, then get feedback from actual newbies and cut based on the evidence - we'll get it wrong either way, but at least this way the newbies will know what they want us to cut. Incidentally, there's a much stronger argument for *drafting* a long document, even if it gets cut down before it's committed. FFmpeg has lots of undocumented nuances that experts just know and newbies don't know to ask, and this thread is full of instances where writing more detail helped tease out a misunderstanding. [1] is a great example - I had finally written enough detail to uncover my assumption that all AVOptions could be set at any time, then that thread taught me to look for a flag that tells you the options for which that's true. If you assume I'm not the only person who has been subtly misled that way, you could argue it's better to commit the long version. That would give readers more opportunities to confront their own wrong assumptions, instead of reading something that assumed they knew one thing, but let them keep believing another. The obvious counterargument is that we should... # Spread the information across multiple documents The argument: this document puts too much information in one place. We should instead focus on making small patches that put information people need to know where they need to know it. This is where things get more interesting to a general audience. If you have repo commit access, you're probably imagining a workflow like: write a bunch of little commits, send them out for review, then commit them when people stop replying. Your access is evidence that you basically know how things work, and also lets you make plans confident in the knowledge that anything you need committed will make it there in the end. My workflow is nothing like that. This thread has constantly reinforced that I don't understand FFmpeg, so it's better for me not to have commit access. But that means I can only work on one patch at once, because I will probably learn something that invalidates any other work I would have done. It also means a single patch not getting interest is enough to sink the project altogether. I can put up with that when it's one big multi-faceted patch, because I can work on one part while waiting for feedback on another part. But my small patches generally involve a few hours of work, a week of waiting, a ping begging for attention, then often being rejected or ignored. In the real world, the only thing this approach will achieve i
[FFmpeg-devel] [PATCH v3 3/3] all: s/AV_OPT_FLAG_RUNTIME_PARAM/AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM/g
Use the new name for the macro throughout the codebase. Patch generated with the following command: sed -i -e 's/AV_OPT_FLAG_RUNTIME_PARAM/AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM/g' \ $( git grep -l AV_OPT_FLAG_RUNTIME_PARAM | grep -v '^libavutil/opt.h' | grep -v '^doc/APIchanges$' ) --- libavfilter/af_aap.c | 2 +- libavfilter/af_acrusher.c | 2 +- libavfilter/af_adelay.c| 2 +- libavfilter/af_adenorm.c | 2 +- libavfilter/af_adrc.c | 2 +- libavfilter/af_adynamicequalizer.c | 2 +- libavfilter/af_adynamicsmooth.c| 2 +- libavfilter/af_aemphasis.c | 2 +- libavfilter/af_aexciter.c | 2 +- libavfilter/af_afade.c | 2 +- libavfilter/af_afftdn.c| 2 +- libavfilter/af_afir.c | 2 +- libavfilter/af_afreqshift.c| 2 +- libavfilter/af_afwtdn.c| 2 +- libavfilter/af_agate.c | 2 +- libavfilter/af_alimiter.c | 2 +- libavfilter/af_amix.c | 2 +- libavfilter/af_anlmdn.c| 2 +- libavfilter/af_anlms.c | 2 +- libavfilter/af_apsyclip.c | 2 +- libavfilter/af_arls.c | 2 +- libavfilter/af_arnndn.c| 2 +- libavfilter/af_asetnsamples.c | 2 +- libavfilter/af_asoftclip.c | 2 +- libavfilter/af_asubboost.c | 2 +- libavfilter/af_asupercut.c | 2 +- libavfilter/af_atempo.c| 2 +- libavfilter/af_atilt.c | 2 +- libavfilter/af_biquads.c | 2 +- libavfilter/af_compensationdelay.c | 2 +- libavfilter/af_crossfeed.c | 2 +- libavfilter/af_crystalizer.c | 2 +- libavfilter/af_dialoguenhance.c| 2 +- libavfilter/af_dynaudnorm.c| 2 +- libavfilter/af_extrastereo.c | 2 +- libavfilter/af_firequalizer.c | 2 +- libavfilter/af_rubberband.c| 2 +- libavfilter/af_sidechaincompress.c | 2 +- libavfilter/af_silenceremove.c | 2 +- libavfilter/af_speechnorm.c| 2 +- libavfilter/af_stereotools.c | 2 +- libavfilter/af_stereowiden.c | 2 +- libavfilter/af_surround.c | 2 +- libavfilter/af_virtualbass.c | 2 +- libavfilter/af_volume.c| 2 +- libavfilter/avf_a3dscope.c | 2 +- libavfilter/avf_avectorscope.c | 2 +- libavfilter/avfilter.c | 4 ++-- libavfilter/f_graphmonitor.c | 2 +- libavfilter/f_perms.c | 2 +- libavfilter/f_realtime.c | 2 +- libavfilter/f_streamselect.c | 2 +- libavfilter/qrencode.c | 2 +- libavfilter/setpts.c | 2 +- libavfilter/src_avsynctest.c | 2 +- libavfilter/vf_amplify.c | 2 +- libavfilter/vf_atadenoise.c| 2 +- libavfilter/vf_avgblur.c | 2 +- libavfilter/vf_backgroundkey.c | 2 +- libavfilter/vf_bbox.c | 2 +- libavfilter/vf_bilateral.c | 2 +- libavfilter/vf_blend.c | 2 +- libavfilter/vf_cas.c | 2 +- libavfilter/vf_chromakey.c | 2 +- libavfilter/vf_chromanr.c | 2 +- libavfilter/vf_chromashift.c | 2 +- libavfilter/vf_colorbalance.c | 2 +- libavfilter/vf_colorchannelmixer.c | 2 +- libavfilter/vf_colorcontrast.c | 2 +- libavfilter/vf_colorcorrect.c | 2 +- libavfilter/vf_colorize.c | 2 +- libavfilter/vf_colorkey.c | 2 +- libavfilter/vf_colorlevels.c | 2 +- libavfilter/vf_colormap.c | 2 +- libavfilter/vf_colortemperature.c | 2 +- libavfilter/vf_convolution.c | 2 +- libavfilter/vf_crop.c | 2 +- libavfilter/vf_cropdetect.c| 2 +- libavfilter/vf_curves.c| 2 +- libavfilter/vf_datascope.c | 2 +- libavfilter/vf_dblur.c | 2 +- libavfilter/vf_deband.c| 2 +- libavfilter/vf_deblock.c | 2 +- libavfilter/vf_despill.c | 2 +- libavfilter/vf_displace.c | 2 +- libavfilter/vf_drawbox.c | 2 +- libavfilter/vf_drawtext.c | 2 +- libavfilter/vf_eq.c| 2 +- libavfilter/vf_estdif.c| 2 +- libavfilter/vf_exposure.c | 2 +- libavfilter/vf_feedback.c | 2 +- libavfilter/vf_fftdnoiz.c | 2 +- libavfilter/vf_fillborders.c | 2 +- libavfilter/vf_frei0r.c| 2 +- libavfilter/vf_gblur.c | 2 +- libavfilter/vf_guided.c| 2 +- libavfilter/vf_hqdn3d.c| 2 +- libavfilter/vf_hsvkey.c| 2 +- libavfilter/vf_hue.c | 2 +- libavfilter/vf_huesaturation.c | 2 +- libavfilter/vf_il.c| 2 +- libavfilter/vf_lagfun.c| 2 +- libavfilter/vf_lenscorrection.c| 2 +- libavfilter/vf_libplacebo.c| 2 +- libavfilter/vf_limitdiff.c | 2 +- libavfilter/vf_limiter.c | 2 +- libavfilter/vf_lumakey.c | 2 +- libavfilter/vf_lut.c | 2 +- libavfilter/vf_lut2.c | 2 +- libavfilter/vf_l
[FFmpeg-devel] [PATCH v3 2/3] lavu/opt: Mention AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM in more places
An inattentive user might not see the explanation at the top of this file.
Paste the explanation to all the places they might see it.
---
libavutil/opt.h | 21 +
1 file changed, 21 insertions(+)
diff --git a/libavutil/opt.h b/libavutil/opt.h
index e050d126ed..06cbe3c336 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -491,6 +491,9 @@ typedef struct AVOptionRanges {
/**
* Set the values of all AVOption fields to their default values.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param s an AVOption-enabled struct (its first member must be a pointer to
AVClass)
*/
void av_opt_set_defaults(void *s);
@@ -500,6 +503,9 @@ void av_opt_set_defaults(void *s);
* AVOption fields for which (opt->flags & mask) == flags will have their
* default applied to s.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param s an AVOption-enabled struct (its first member must be a pointer to
AVClass)
* @param mask combination of AV_OPT_FLAG_*
* @param flags combination of AV_OPT_FLAG_*
@@ -669,6 +675,9 @@ enum {
* key. ctx must be an AVClass context, storing is done using
* AVOptions.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param opts options string to parse, may be NULL
* @param key_val_sep a 0-terminated list of characters used to
* separate key from value
@@ -687,6 +696,9 @@ int av_set_options_string(void *ctx, const char *opts,
* Parse the key-value pairs list in opts. For each key=value pair found,
* set the value of the corresponding option in ctx.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param ctx the AVClass object to set options on
* @param opts the options string, key-value pairs separated by a
* delimiter
@@ -717,6 +729,9 @@ int av_opt_set_from_string(void *ctx, const char *opts,
/**
* Set all the options from a given dictionary on an object.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param obj a struct whose first element is a pointer to AVClass
* @param options options to process. This dictionary will be freed and
replaced
*by a new one containing all options not found in obj.
@@ -734,6 +749,9 @@ int av_opt_set_dict(void *obj, struct AVDictionary
**options);
/**
* Set all the options from a given dictionary on an object.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param obj a struct whose first element is a pointer to AVClass
* @param options options to process. This dictionary will be freed and
replaced
*by a new one containing all options not found in obj.
@@ -772,6 +790,9 @@ int av_opt_copy(void *dest, const void *src);
* @{
* Those functions set the field of obj with the given name to value.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be modified.
+ *
* @param[in] obj A struct whose first element is a pointer to an AVClass.
* @param[in] name the name of the field to set
* @param[in] val The value to set. In case of av_opt_set() if the field is not
--
2.45.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 v3 1/3] lavu/opt: Rename AV_OPT_FLAG_RUNTIME_PARAM to ...POST_INIT_SETTABLE_PARAM
The old name could be misread as the opposite of "AV_OPT_FLAG_READONLY" -
some things can be set at runtime, others are read-only. Clarify that
this refers to options that can be set after the struct is initialized.
---
libavutil/opt.h | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..e050d126ed 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -53,6 +53,9 @@
* question is allowed to access the field. This allows us to extend the
* semantics of those fields without breaking API compatibility.
*
+ * Note: only options with the AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM flag can be
+ * modified after the struct is initialized.
+ *
* @section avoptions_scope Scope of AVOptions
*
* AVOptions is designed to support any set of multimedia configuration options
@@ -300,7 +303,12 @@ enum AVOptionType{
#define AV_OPT_FLAG_BSF_PARAM (1 << 8)
/**
- * A generic parameter which can be set by the user at runtime.
+ * A generic parameter which can be set by the user after the struct is
initialized.
+ */
+#define AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM (1 << 15)
+/**
+ * A generic parameter which can be set by the user after the struct is
initialized.
+ * @deprecated Renamed to AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM for clarity
*/
#define AV_OPT_FLAG_RUNTIME_PARAM (1 << 15)
/**
--
2.45.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 v3 0/3] s/RUNTIME/POST_INIT_SETTABLE/
AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM is fine by me, here's a patch. I've added a "@deprecated" comment for the old name, but would this need to be queued up for 8.0? Technically this is a backwards-incompatible change to the existing API, even though it doesn't change the ABI or generate warnings when compiling code. My vote is always going to be for putting documentation in the first place people look, even at the expense of redundancy. But I can live without the extra comments so long as the flag is renamed. This patch moves the extra documentation to an optional commit - I'm fine with just applying #1 and #3 if people prefer, but it's there if the conversation goes the other way. Also, I think this is better, but can also live with the v2 patch, so long as the other notes remain in. ___ 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] web/download: Refer to trac downstreams page
On Thu, Jun 13, 2024 at 09:39:34PM +0200, Michael Niedermayer wrote: > From: Andrew Sayers > > Signed-off-by: Michael Niedermayer > --- > src/download | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/download b/src/download > index 3ebf773..acef214 100644 > --- a/src/download > +++ b/src/download > @@ -642,7 +642,9 @@ libpostproc53. 3.100 >Old Releases > > Older versions are available at the Old > - Releases page. > + Releases page. Releases are usually moved there after the last > + https://trac.ffmpeg.org/wiki/Downstreams";>downstream > + drops support. > > > > -- > 2.45.2 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] web: move 6.0 to olddownload
On Thu, Jun 13, 2024 at 12:10:21PM +0200, Michael Niedermayer wrote: > I see no users on https://trac.ffmpeg.org/wiki/Downstreams I don't have a strong opinion about the move itself, but can that page be linked from the Old Releases section? Something like: Older versions are available at the Old - Releases page. + Releases page. Releases are usually moved there after the last + https://trac.ffmpeg.org/wiki/Downstreams";>downstream + drops support. A hint like that would help drive people to maintain the list, making your life easier in future. I haven't formatted the above as a proper patch because I assume you'd want to choose the wording, but can do if that would be better. ___ 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] lavfi: add perlin noise generator
Some documentation nitpicks. Nothing jumped out about the code, but I don't
know the algorithm well enough to spot anything deep.
> From 9932cfc19500acbd0685eb2cc8fd88e9af3f5dbd Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini
> Date: Mon, 27 May 2024 11:19:08 +0200
> Subject: [PATCH] lavfi: add Perlin noise generator
>
> ---
> Changelog | 1 +
> doc/filters.texi | 100 +
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/perlin.c | 224 ++
> libavfilter/perlin.h | 101 +
> libavfilter/vsrc_perlin.c | 169
> 7 files changed, 597 insertions(+)
> create mode 100644 libavfilter/perlin.c
> create mode 100644 libavfilter/perlin.h
> create mode 100644 libavfilter/vsrc_perlin.c
>
> diff --git a/Changelog b/Changelog
> index 03d6b29ad8..b8dcf452ac 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -12,6 +12,7 @@ version :
> - qsv_params option added for QSV encoders
> - VVC decoder compatible with DVB test content
> - xHE-AAC decoder
> +- perlin source
>
>
> version 7.0:
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 347103c04f..7af299b2a2 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -17290,6 +17290,9 @@ The command accepts the same syntax of the
> corresponding option.
> If the specified expression is not valid, it is kept at its current
> value.
>
> +@anchor{lutrgb}
> +@anchor{lutyuv}
> +@anchor{lut}
> @section lut, lutrgb, lutyuv
>
> Compute a look-up table for binding each pixel component input value
> @@ -29281,6 +29284,103 @@ ffplay -f lavfi
> life=s=300x200:mold=10:r=60:ratio=0.1:death_color=#C83232:life_c
> @end example
> @end itemize
>
> +@section perlin
> +Generate Perlin noise.
> +
> +Perlin noise is a kind of noise with local continuity in space. This
> +can be used to generate patterns with continuity in space and time,
> +e.g. to simulate smoke, fluids, or terrain.
> +
> +In case more than one octave is specified through the @option{octaves}
> +option, Perlin noise is generated as a sum of components, each one
> +with doubled frequency. In this case the @option{persistence} option
> +specify the ratio of the amplitude with respect to the previous
> +component. More octave components enable to specify more high
> +frequency details in the generated noise (e.g. small size variations
> +due to bolders in a generated terrain).
Typo: s/bolders/boulders/
> +
> +@subsection Options
> +@table @option
> +
> +@item size, s
> +Specify the size (width and height) of the buffered video frames. For the
> +syntax of this option, check the
> +@ref{video size syntax,,"Video size" section in the ffmpeg-utils
> manual,ffmpeg-utils}.
> +
> +@item rate, r
> +Specify the frame rate expected for the video stream, expressed as a
> +number of frames per second.
> +
> +@item octaves
> +Specify the total number of components making up the noise, each one
> +with doubled frequency.
> +
> +@item persistence
> +Set the ratio used to compute the amplitude of the next octave
> +component with respect to the previous component amplitude.
> +
> +@item xscale
> +@item yscale
> +Define a scale factor used to multiple the x, y coordinates. This can
> +be useful to define an effect with a pattern stretched along the x or
> +y axis.
> +
> +@item tscale
> +Define a scale factor used to multiple the time coordinate. This can
> +be useful to change the time variation speed.
> +
> +@item random_mode
> +Set random mode used to compute initial pattern.
> +
> +Supported values are:
> +@table @option
> +@item random
> +Compute and use random seed.
> +
> +@item ken
> +Use the predefined initial pattern defined by Ken Perlin in the
> +original article, can be useful to compare the output with other
> +sources.
> +
> +@item seed
> +Use the value specified by @option{random_seed} option.
> +@end table
Nit: "Define a...", "Use the..." etc. is redundant - remove them to
optimise for reading time.
> +
> +@item random_seed, seed
> +Use this value to compute the initial pattern, it is only considered
> +when @option{random_mode} is set to @var{random_seed}.
> +@end table
Nit: the reader needs to read the second clause before the first makes sense.
Consider something like:
When @option{random_mode} is set to @var{random_seed},
this value computes the initial pattern.
> +
> +@subsection Examples
> +@itemize
> +@item
> +Generate single component:
> +@example
> +perlin
> +@end example
> +
> +@item
> +Use Perlin noise with 7 components, each one with a halved contribute
s/contribute/contribution/
> +to total amplitude:
> +@example
> +perlin=octaves=7:persistence=0.5
> +@end example
> +
> +@item
> +Chain Perlin noise with the @ref{lutyuv} to generate a black&white
> +effect:
> +@example
> +perlin:octaves=7:tscale=0.3,lutyuv=y='if(lt(val\,128)\,255\,0)'
> +@end example
> +
> +@item
> +Stretch noise along the y axis, and convert
Re: [FFmpeg-devel] [PATCH v6 1/4] doc: Explain what "context" means
On Wed, Jun 12, 2024 at 10:52:00PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2024-06-04 15:47:21 +0100, Andrew Sayers wrote:
[...]
> My impression is that this is growing out of scope for a
> reference. The doxy is a reference, therefore it should be clean and
> terse, and we should avoid adding too much information, enough
> information should be right enough. In fact, a reference is different
> from a tutorial, and much different from a C tutorial. Also this is
> not a treatise comparing different languages and frameworks, as this
> would confuse beginners and would annoy experienced developers.
>
> I propose to cut this patch to provide the minimal information you can
> expect in a reference, but not more than that. Addition can be added
> later, but I think we should try to avoid any unnecessary content, in
> the spirit of keeping this a reference. More extensive discussions
> might be done in a separate place (the wiki, a blog post etc.), but in
> the spirit of a keeping this a reference they should not be put here.
I would agree if we had a tradition of linking to the wiki or regular blog
posts, but even proposing internal links has generated pushback in this thread,
so that feels like making the perfect the enemy of the good. Let's get this
committed, see how people react, then look for improvements.
In fact, once this is available in the trunk version of the website,
we should ask for feedback from the libav-user ML and #ffmpeg IRC channel.
Then we can expand/move/remove stuff based on feedback.
>
> > +
> > +@section Context_general “Context” as a general concept
[...]
> I'd skip all this part, as we assume the reader is already familiar
> with C language and with data encapsulation through struct, if he is
> not this is not the right place where to teach about C language
> fundamentals.
I disagree, for a reason I've been looking for an excuse to mention :)
Let's assume 90% of people who use FFmpeg already know something in the doc.
You could say that part of the doc is useless to 90% of the audience.
Or you could say that 90% of FFmpeg users are not our audience.
Looking at it the second way means you need to spend more time on "routing" -
linking to the document in ways that (only) attract your target audience,
making a table of contents with headings that aid skip-readers, etc.
But once you've routed people around the bits they don't care about,
it's fine to have documentation that's only needed by a minority.
Also, less interesting but equally important - context is not a C language
fundamental, it's more like an emergent property of large C projects. A
developer that came here without knowing e.g. what a struct is could read
any of the online tutorials that explain the concept better than we could.
I'd be happy to link to a good tutorial about contexts if we found one,
but we have to meet people where they are, and this is the best solution
I've been able to find.
>
> > +
> > +When reading code that *is* explicitly described in terms of contexts,
> > +remember that the term's meaning is guaranteed by *the project's
> > community*,
> > +not *the language it's written in*. That means guarantees may be more
> > flexible
> > +and change more over time. For example, programming languages that use
> > +[encapsulation](https://en.wikipedia.org/wiki/Encapsulation_(computer_programming))
> > +will simply refuse to compile code that violates its rules about access,
> > +while communities can put up with special cases if they improve code
> > quality.
> > +
>
> This looks a bit vague so I'd rather drop this.
This probably looks vague to you because you're part of the 90% of people this
paragraph isn't for. All programming languages provide some guarantees, and
leave others up to the community to enforce (or not). Over time, people stop
seeing the language guarantees at all, and assume the only alternative is
anarchy. For example, if you got involved in a large JavaScript project,
you might be horrified to see almost all structs are the same type ("Object"),
and are implemented as dictionaries that are expected to have certain keys.
But in practice, this stuff gets enforced at the community level well enough.
Similarly, a JS programmer might be horrified to learn FFmpeg needs a whole
major version bump just to add a key to a struct. This paragraph is there to
nudge people who have stopped seeing things we need them to look out for.
If you'd like to maintain an official FFmpeg blog, I'd be happy to expand the
paragraph above into a medium-sized post, then just link it from the doc.
But that post would be too subjective to be a wiki page - JavaScript is
evolving in a more strongly-typed direction, so it would
[FFmpeg-devel] [PATCH v2] lavu/opt: Discuss AV_OPT_FLAG_RUNTIME_PARAM more explicitly
After a struct is initialized, only options with the
AV_OPT_FLAG_RUNTIME_PARAM flag can be modified.
Make that clearer, for the sake of readers who would otherwise
assume all options can be modified at any time.
---
libavutil/opt.h | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..d23c10bcf5 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -53,6 +53,9 @@
* question is allowed to access the field. This allows us to extend the
* semantics of those fields without breaking API compatibility.
*
+ * Note: only options with the AV_OPT_FLAG_RUNTIME_PARAM flag can be
+ * modified after the struct is initialized.
+ *
* @section avoptions_scope Scope of AVOptions
*
* AVOptions is designed to support any set of multimedia configuration options
@@ -300,7 +303,7 @@ enum AVOptionType{
#define AV_OPT_FLAG_BSF_PARAM (1 << 8)
/**
- * A generic parameter which can be set by the user at runtime.
+ * A generic parameter which can be set by the user after initialization.
*/
#define AV_OPT_FLAG_RUNTIME_PARAM (1 << 15)
/**
@@ -483,6 +486,9 @@ typedef struct AVOptionRanges {
/**
* Set the values of all AVOption fields to their default values.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_RUNTIME_PARAM flag can be modified.
+ *
* @param s an AVOption-enabled struct (its first member must be a pointer to
AVClass)
*/
void av_opt_set_defaults(void *s);
@@ -492,6 +498,9 @@ void av_opt_set_defaults(void *s);
* AVOption fields for which (opt->flags & mask) == flags will have their
* default applied to s.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_RUNTIME_PARAM flag can be modified.
+ *
* @param s an AVOption-enabled struct (its first member must be a pointer to
AVClass)
* @param mask combination of AV_OPT_FLAG_*
* @param flags combination of AV_OPT_FLAG_*
@@ -661,6 +670,9 @@ enum {
* key. ctx must be an AVClass context, storing is done using
* AVOptions.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_RUNTIME_PARAM flag can be modified.
+ *
* @param opts options string to parse, may be NULL
* @param key_val_sep a 0-terminated list of characters used to
* separate key from value
@@ -679,6 +691,9 @@ int av_set_options_string(void *ctx, const char *opts,
* Parse the key-value pairs list in opts. For each key=value pair found,
* set the value of the corresponding option in ctx.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_RUNTIME_PARAM flag can be modified.
+ *
* @param ctx the AVClass object to set options on
* @param opts the options string, key-value pairs separated by a
* delimiter
@@ -709,6 +724,9 @@ int av_opt_set_from_string(void *ctx, const char *opts,
/**
* Set all the options from a given dictionary on an object.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_RUNTIME_PARAM flag can be modified.
+ *
* @param obj a struct whose first element is a pointer to AVClass
* @param options options to process. This dictionary will be freed and
replaced
*by a new one containing all options not found in obj.
@@ -726,6 +744,9 @@ int av_opt_set_dict(void *obj, struct AVDictionary
**options);
/**
* Set all the options from a given dictionary on an object.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_RUNTIME_PARAM flag can be modified.
+ *
* @param obj a struct whose first element is a pointer to AVClass
* @param options options to process. This dictionary will be freed and
replaced
*by a new one containing all options not found in obj.
@@ -764,6 +785,9 @@ int av_opt_copy(void *dest, const void *src);
* @{
* Those functions set the field of obj with the given name to value.
*
+ * Note: after a struct is initialized, only options with the
+ * AV_OPT_FLAG_RUNTIME_PARAM flag can be modified.
+ *
* @param[in] obj A struct whose first element is a pointer to an AVClass.
* @param[in] name the name of the field to set
* @param[in] val The value to set. In case of av_opt_set() if the field is not
--
2.45.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] lavu/opt: Mention that AVOptions is not reentrant
On Thu, Jun 06, 2024 at 05:21:00PM +0200, Andreas Rheinhardt wrote: > Andrew Sayers: > > On Thu, Jun 06, 2024 at 04:24:11PM +0200, Michael Niedermayer wrote: > >> On Thu, Jun 06, 2024 at 09:29:24AM +0100, Andrew Sayers wrote: > >>> On Thu, Jun 06, 2024 at 01:17:48AM +0200, Michael Niedermayer wrote: > >>> [...] > >>>> AVOption simply provides light weight access to the struct fields. > >>>> Calling AVOption non re-entrant in modifying a field you arent even > >>>> allowed > >>>> to modify from 2 threads is confusing > >>> > >>> I think you're saying there's already a rule about modifying AVOptions > >>> from > >>> 2 threads. Could you explain that in more detail? > >> > >> Well, one way this can be argued is this: > >> Latest C draft: (but i doubt this is different) ISO/IEC 9899:2017 C17 > >> ballot N2176 > >> > >> "Two expression evaluations conflict if one of them modifies a memory > >> location and the other one > >> reads or modifies the same memory location" > >> > >> so if you have 2 threads and one writes into a int and another reads it at > >> the > >> same time, the code is broken. > >> The code doing said act through some API doesnt become less broken > >> > >> Calling AVOption non re-rentrant because of that is false thats as if one > >> executed > >> strtok_r(a,b,c) with the VERY same a,b,c from 2 threads and then said > >> its not thread safe > >> > >> strtok_r() is thread safe and reentrant if its used correctly, so is > >> AVOption > > [...] > > > > Ok, how about if the patch avoided the word "reentrant" and just said: > > > > + * Note: AVOptions values should not be modified after a struct is > > initialized. > > This is wrong either. As Paul has already pointed out to you, some > options are allowed to be modified lateron. Ah, I'd interpreted "runtime" to be the opposite of "compile-time", not "initialization-time". I'll propose a new patch that should be clearer. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/opt: Mention that AVOptions is not reentrant
On Thu, Jun 06, 2024 at 04:24:11PM +0200, Michael Niedermayer wrote: > On Thu, Jun 06, 2024 at 09:29:24AM +0100, Andrew Sayers wrote: > > On Thu, Jun 06, 2024 at 01:17:48AM +0200, Michael Niedermayer wrote: > > [...] > > > AVOption simply provides light weight access to the struct fields. > > > Calling AVOption non re-entrant in modifying a field you arent even > > > allowed > > > to modify from 2 threads is confusing > > > > I think you're saying there's already a rule about modifying AVOptions from > > 2 threads. Could you explain that in more detail? > > Well, one way this can be argued is this: > Latest C draft: (but i doubt this is different) ISO/IEC 9899:2017 C17 > ballot N2176 > > "Two expression evaluations conflict if one of them modifies a memory > location and the other one > reads or modifies the same memory location" > > so if you have 2 threads and one writes into a int and another reads it at the > same time, the code is broken. > The code doing said act through some API doesnt become less broken > > Calling AVOption non re-rentrant because of that is false thats as if one > executed > strtok_r(a,b,c) with the VERY same a,b,c from 2 threads and then said > its not thread safe > > strtok_r() is thread safe and reentrant if its used correctly, so is AVOption [...] Ok, how about if the patch avoided the word "reentrant" and just said: + * Note: AVOptions values should not be modified after a struct is initialized. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/opt: Mention that AVOptions is not reentrant
On Thu, Jun 06, 2024 at 01:17:48AM +0200, Michael Niedermayer wrote: [...] > AVOption simply provides light weight access to the struct fields. > Calling AVOption non re-entrant in modifying a field you arent even allowed > to modify from 2 threads is confusing I think you're saying there's already a rule about modifying AVOptions from 2 threads. Could you explain that in more detail? > If you want to modify a field from 2 threads that field could be some sort > of atomic type. This can then easily be added to AVOption Doing that for a single option would involve publicly guaranteeing its representation for at least one major version. At that point, you might as well just tell people to access it as a member of a public struct. To be clear - this isn't a programming problem, it's a design problem. The interface currently allows external developers to assume something will always work when it's actually just something that could be supported some day. Writing up the current behaviour as a guarantee lets them avoid writing code that will generate hard-to-reproduce bugs, at the cost of making it slightly harder for us to do something we've never needed to do in the past. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/opt: Mention that AVOptions is not reentrant
On Wed, Jun 05, 2024 at 09:46:16AM -0400, Ronald S. Bultje wrote: > Hi, > > On Wed, Jun 5, 2024 at 9:44 AM Andrew Sayers > wrote: > > > On Wed, Jun 05, 2024 at 03:34:50PM +0200, Paul B Mahol wrote: > > > On Wed, Jun 5, 2024 at 3:18 PM Andrew Sayers < > > ffmpeg-de...@pileofstuff.org> > > > wrote: > > > > > > > An external API developer might think they can use AVOptions to modify > > > > values > > > > during playback (e.g. putting a playback quality slider next to the > > volume > > > > slider). Make it clear that behaviour is not recommended. > > > > > > > > > > There are options that can be changed at runtime, > > > And it works just fine. > > > > How would an external developer know which options can be safely changed > > (preferably including in future versions of FFmpeg) vs. ones where their > > tests > > got lucky and happened not to trigger a read during a non-atomic write? > > > > If you see that happening, it would be good to submit a bug report. Right > now it's very abstract. I think we might be talking past each other - here's a concrete example: The private struct "SetTSContext" includes an AVOptions-accessible member "time_base", currently implemented as an AVRational (i.e. a pair of ints). write_number() in libavutil/opt.c sets options of type AV_OPT_TYPE_RATIONAL in such a way that a poorly-timed read could see the new numerator and old denominator (or the other way around). If I wrote a program that let users dynamically change the time base, and someone switched their timebase from 1/30 to 100/3000, one unlucky user might have a few frames encoded with a timebase of 100/30. Is that something the AVOptions API is supposed to support? If yes, the bug is that AVOptions access isn't guarded by a mutex. If no, there's no bug, just an edge case worth mentioning in the docs. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu/opt: Mention that AVOptions is not reentrant
On Wed, Jun 05, 2024 at 03:34:50PM +0200, Paul B Mahol wrote: > On Wed, Jun 5, 2024 at 3:18 PM Andrew Sayers > wrote: > > > An external API developer might think they can use AVOptions to modify > > values > > during playback (e.g. putting a playback quality slider next to the volume > > slider). Make it clear that behaviour is not recommended. > > > > There are options that can be changed at runtime, > And it works just fine. How would an external developer know which options can be safely changed (preferably including in future versions of FFmpeg) vs. ones where their tests got lucky and happened not to trigger a read during a non-atomic write? ___ 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] lavu/opt: Mention that AVOptions is not reentrant
An external API developer might think they can use AVOptions to modify values
during playback (e.g. putting a playback quality slider next to the volume
slider). Make it clear that behaviour is not recommended.
Include the warning in the group description and the text for every function
that sets options, but leave it implicit in functions that get options.
This reflects the fact that *modifying* options is likely to cause weird bugs,
while *reading* options isn't guaranteed but is likely to be safe.
---
libavutil/opt.h | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..13292c6473 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -53,11 +53,16 @@
* question is allowed to access the field. This allows us to extend the
* semantics of those fields without breaking API compatibility.
*
+ * Note that AVOptions functions are not reentrant, and options may be accessed
+ * from internal FFmpeg threads. Unless otherwise noted, it is best to avoid
+ * modifying options once a struct has been initialized.
+ *
* @section avoptions_scope Scope of AVOptions
*
* AVOptions is designed to support any set of multimedia configuration options
- * that can be defined at compile-time. Although it is mainly used to expose
- * FFmpeg options, you are welcome to adapt it to your own use case.
+ * that can be defined at compile-time and set at object creation time.
Although
+ * it is mainly used to expose FFmpeg options, you are welcome to adapt it
+ * to your own use case.
*
* No single approach can ever fully solve the problem of configuration,
* but please submit a patch if you believe you have found a problem
@@ -483,6 +488,9 @@ typedef struct AVOptionRanges {
/**
* Set the values of all AVOption fields to their default values.
*
+ * Note: like all AVOptions functions, this is not reentrant. Unless
+ * otherwise noted, it should only be used before initializing the struct.
+ *
* @param s an AVOption-enabled struct (its first member must be a pointer to
AVClass)
*/
void av_opt_set_defaults(void *s);
@@ -492,6 +500,9 @@ void av_opt_set_defaults(void *s);
* AVOption fields for which (opt->flags & mask) == flags will have their
* default applied to s.
*
+ * Note: like all AVOptions functions, this is not reentrant. Unless
+ * otherwise noted, it should only be used before initializing the struct.
+ *
* @param s an AVOption-enabled struct (its first member must be a pointer to
AVClass)
* @param mask combination of AV_OPT_FLAG_*
* @param flags combination of AV_OPT_FLAG_*
@@ -661,6 +672,9 @@ enum {
* key. ctx must be an AVClass context, storing is done using
* AVOptions.
*
+ * Note: like all AVOptions functions, this is not reentrant. Unless
+ * otherwise noted, it should only be used before initializing the struct.
+ *
* @param opts options string to parse, may be NULL
* @param key_val_sep a 0-terminated list of characters used to
* separate key from value
@@ -679,6 +693,9 @@ int av_set_options_string(void *ctx, const char *opts,
* Parse the key-value pairs list in opts. For each key=value pair found,
* set the value of the corresponding option in ctx.
*
+ * Note: like all AVOptions functions, this is not reentrant. Unless
+ * otherwise noted, it should only be used before initializing the struct.
+ *
* @param ctx the AVClass object to set options on
* @param opts the options string, key-value pairs separated by a
* delimiter
@@ -709,6 +726,9 @@ int av_opt_set_from_string(void *ctx, const char *opts,
/**
* Set all the options from a given dictionary on an object.
*
+ * Note: like all AVOptions functions, this is not reentrant. Unless
+ * otherwise noted, it should only be used before initializing the struct.
+ *
* @param obj a struct whose first element is a pointer to AVClass
* @param options options to process. This dictionary will be freed and
replaced
*by a new one containing all options not found in obj.
@@ -726,6 +746,9 @@ int av_opt_set_dict(void *obj, struct AVDictionary
**options);
/**
* Set all the options from a given dictionary on an object.
*
+ * Note: like all AVOptions functions, this is not reentrant. Unless
+ * otherwise noted, it should only be used before initializing the struct.
+ *
* @param obj a struct whose first element is a pointer to AVClass
* @param options options to process. This dictionary will be freed and
replaced
*by a new one containing all options not found in obj.
@@ -764,6 +787,9 @@ int av_opt_copy(void *dest, const void *src);
* @{
* Those functions set the field of obj with the given name to value.
*
+ * Note: like all AVOptions functions, these are not reentrant. Unless
+ * otherwise noted, they should only be used before initializing the struct.
+ *
* @param[in] obj A struct whose first element is a pointer to
Re: [FFmpeg-devel] [PATCH v6 3/4] all: Link to "context" from all public contexts with documentation
Note: I somehow managed to send this message directly to Anton before - sorry Anton for the message spam, please reply to this one if you want the list to see it! On Wed, Jun 05, 2024 at 10:12:47AM +0200, Anton Khirnov wrote: > Quoting Andrew Sayers (2024-06-04 16:47:23) > > The goal of putting these links in "@see" blocks is to provide hooks > > for future developers to add links to other useful parts of the codebase. > > --- > > libavcodec/avcodec.h | 3 +++ > > libavcodec/bsf.h | 3 +++ > > libavcodec/d3d11va.h | 3 +++ > > libavcodec/mediacodec.h | 2 ++ > > libavcodec/qsv.h | 3 +++ > > libavcodec/vdpau.h | 3 +++ > > libavcodec/videotoolbox.h| 3 +++ > > libavfilter/avfilter.h | 7 ++- > > libavformat/avformat.h | 3 +++ > > libavformat/avio.h | 3 +++ > > libavutil/audio_fifo.h | 3 +++ > > libavutil/hwcontext.h| 6 ++ > > libavutil/hwcontext_cuda.h | 3 +++ > > libavutil/hwcontext_d3d11va.h| 6 ++ > > libavutil/hwcontext_d3d12va.h| 6 ++ > > libavutil/hwcontext_drm.h| 3 +++ > > libavutil/hwcontext_dxva2.h | 6 ++ > > libavutil/hwcontext_mediacodec.h | 3 +++ > > libavutil/hwcontext_opencl.h | 6 ++ > > libavutil/hwcontext_qsv.h| 6 ++ > > libavutil/hwcontext_vaapi.h | 6 ++ > > libavutil/hwcontext_vdpau.h | 3 +++ > > libavutil/hwcontext_vulkan.h | 6 ++ > > libavutil/lfg.h | 3 +++ > > 24 files changed, 98 insertions(+), 1 deletion(-) > > IMO this is pointless churn. That's like saying caches are pointless bloat - it's not about correctness, it's about performance. Actually, I've been talking about "performance" a lot round here, but not really explained what I mean... Imagine a smart young developer who learned C at university and is now dipping their toe in the world of actual C development. They've come up with an idea for a little app that's different enough from the examples to force themselves not to just copy/paste example code. It's simple enough that an experienced developer like you would only need half a day for the whole project, so they allow themselves a weekend to get it done. "Performance" in this case means "can they finish the project in their ~16 hour time budget?" Assuming they're half as productive as you would be, 8 of their 16 hours go on programming, leaving 8 hours to learn FFmpeg. They've never written real-world C before, so they don't know what a context is. And they only have a narrow understanding of OOP, so as soon as they see words like "object" and "class", they assume you're referring to the precise version of OOP their Java lecturer taught them about. So the longer they spend poking round looking for information, the more misconceptions they're going to develop. Even if we assume they find the context document after one hour of searching, they fully understand the document with no reading time, and only need one extra hour to unlearn the misconceptions they developed, that's still cost a quarter of their remaining time budget. To an expert, these links are unnecessary verbiage pointing to a document that explains things that are blindingly obvious. But to a newbie, they're an important optimisation that can make the difference between finishing a project or having to give up on FFmpeg altogether. ___ 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 v6 2/4] lavu: Clarify relationship between AVClass, AVOption and context
On Wed, Jun 05, 2024 at 12:34:48PM +0200, Stefano Sabatini wrote: > On date Tuesday 2024-06-04 15:47:22 +0100, Andrew Sayers wrote: > > + * Structs that only use logging facilities are often referred to as > > + * "AVClass context structures", while those that provide configuration > > + * options are called "AVOptions-enabled structs". > > A struct with an AVClass as its first member can be used for both > logging and options management, there is no difference between the > two. My take: > |Structs that use AVClass might be referred to as AVClass > |contexts/structures, those that in addition define options might be > |called AVOptions contexts/structures. I think you were away when this came up, and anyway this thread is getting quite unwieldy. See [1] and [2] for the full version, but in short, defining AVClass by layout leads to conclusions that are at best unintuitive. > > + * Note that AVOptions is not reentrant, and that many FFmpeg functions > > access > > ... AVOptions access is not reeentrant ... > > > + * options from separate threads. Unless otherwise indicated, it is best > > to > > + * avoid modifying options once a struct has been initialized. > > But this note is not relevant to the change, and should probably be > discussed separately Short version: I'll make a separate patch now, let's come back to this after Long version... If you assume options can be set at any time, they broadly resemble a reflection mechanism. If you assume they can only be set during the configuration stage, they broadly resemble an OOP constructor. The document needs to address the one they resemble (even if just to say "this is why they're different"), and needs to steer clear of any possible comparison with the one they don't resemble. So it would be too risky to bump this to the upcoming omnibus patchset, but it's fine to apply this *before* the context document. [1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/328058.html [2] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/328087.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] STF 2025
On Fri, May 17, 2024 at 03:49:58PM +0200, Michael Niedermayer wrote: > Hi all > > Before this is forgotten again, better start some dicsussion too early than > too late Unless there's a better place to put these, I plan to reply to this message whenever I notice someone bring up something that seems relevant. Hopefully it will be a good reference if and when the time comes. Sebastian Ramacher recently said this in another thread[1]: > Maintainers and developers of reverse dependencies repeatedly ask for > upgrade guides that go beyond "use this function instead" This strikes me as an excellent bit of boring-but-important STF work. The bug reports in that e-mail make it relatively easy to quantify impact - measure the number of breakages in each revision, make a chart of numbers over time, agree a target number for next time. [1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-June/328852.html ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v6 4/4] all: Rewrite documentation for contexts
Make their documentation more readable and similar to each other,
(hopefully) without changing the meaning.
---
libavcodec/aac/aacdec.h | 2 +-
libavcodec/aacenc.h | 2 +-
libavcodec/ac3enc.h | 2 +-
libavcodec/amfenc.h | 2 +-
libavcodec/atrac.h | 2 +-
libavcodec/avcodec.h | 3 ++-
libavcodec/bsf.h | 2 +-
libavcodec/cbs.h | 2 +-
libavcodec/d3d11va.h | 3 +--
libavcodec/mediacodec.h | 4 ++--
libavcodec/pthread_frame.c | 4 ++--
libavcodec/qsv.h | 6 --
libavcodec/sbr.h | 2 +-
libavcodec/vdpau.h | 5 +++--
libavcodec/videotoolbox.h| 5 +++--
libavfilter/avfilter.h | 2 +-
libavformat/avformat.h | 3 ++-
libavformat/avio.h | 3 ++-
libavutil/audio_fifo.h | 2 +-
libavutil/hwcontext.h| 21 -
libavutil/hwcontext_cuda.h | 2 +-
libavutil/hwcontext_d3d11va.h| 4 ++--
libavutil/hwcontext_d3d12va.h| 6 +++---
libavutil/hwcontext_drm.h| 2 +-
libavutil/hwcontext_dxva2.h | 4 ++--
libavutil/hwcontext_mediacodec.h | 2 +-
libavutil/hwcontext_opencl.h | 4 ++--
libavutil/hwcontext_qsv.h| 4 ++--
libavutil/hwcontext_vaapi.h | 4 ++--
libavutil/hwcontext_vdpau.h | 2 +-
libavutil/hwcontext_vulkan.h | 5 +++--
libavutil/lfg.h | 3 ++-
32 files changed, 65 insertions(+), 54 deletions(-)
diff --git a/libavcodec/aac/aacdec.h b/libavcodec/aac/aacdec.h
index ee21a94007..f0d613afd9 100644
--- a/libavcodec/aac/aacdec.h
+++ b/libavcodec/aac/aacdec.h
@@ -441,7 +441,7 @@ typedef struct AACDecDSP {
} AACDecDSP;
/**
- * main AAC decoding context
+ * Context for decoding AAC
*/
struct AACDecContext {
const struct AVClass *class;
diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h
index d07960620e..3e710c7fac 100644
--- a/libavcodec/aacenc.h
+++ b/libavcodec/aacenc.h
@@ -207,7 +207,7 @@ typedef struct AACPCEInfo {
} AACPCEInfo;
/**
- * AAC encoder context
+ * Context for encoding AAC
*/
typedef struct AACEncContext {
AVClass *av_class;
diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
index 5e98ad188b..e5abe0a856 100644
--- a/libavcodec/ac3enc.h
+++ b/libavcodec/ac3enc.h
@@ -153,7 +153,7 @@ typedef struct AC3Block {
struct PutBitContext;
/**
- * AC-3 encoder private context.
+ * Private context for encoding AC-3
*/
typedef struct AC3EncodeContext {
AVClass *av_class; ///< AVClass used for AVOption
diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
index 2dbd378ef8..184897beeb 100644
--- a/libavcodec/amfenc.h
+++ b/libavcodec/amfenc.h
@@ -43,7 +43,7 @@ typedef struct AmfTraceWriter {
} AmfTraceWriter;
/**
-* AMF encoder context
+* Context for encoding AMF
*/
typedef struct AmfContext {
diff --git a/libavcodec/atrac.h b/libavcodec/atrac.h
index 05208bbee6..e760f0384d 100644
--- a/libavcodec/atrac.h
+++ b/libavcodec/atrac.h
@@ -39,7 +39,7 @@ typedef struct AtracGainInfo {
} AtracGainInfo;
/**
- * Gain compensation context structure.
+ * Context for gain compensation
*/
typedef struct AtracGCContext {
float gain_tab1[16]; ///< gain compensation level table
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index abc00ab394..2fed4757ed 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -430,7 +430,8 @@ typedef struct RcOverride{
#define AV_GET_ENCODE_BUFFER_FLAG_REF (1 << 0)
/**
- * main external API structure.
+ * Context for an encode or decode session
+ *
* New fields can be added to the end with minor version bumps.
* Removal, reordering and changes to existing fields require a major
* version bump.
diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
index ee5fdd48d2..fadcfc5d47 100644
--- a/libavcodec/bsf.h
+++ b/libavcodec/bsf.h
@@ -56,7 +56,7 @@
*/
/**
- * The bitstream filter state.
+ * Context for bitstream filtering
*
* This struct must be allocated with av_bsf_alloc() and freed with
* av_bsf_free().
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index d479b1ac2d..c074dd11ec 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -214,7 +214,7 @@ typedef void (*CBSTraceWriteCallback)(void *trace_context,
int64_t value);
/**
- * Context structure for coded bitstream operations.
+ * Context for coded bitstream operations
*/
typedef struct CodedBitstreamContext {
/**
diff --git a/libavcodec/d3d11va.h b/libavcodec/d3d11va.h
index 686974b083..5ffee2b3be 100644
--- a/libavcodec/d3d11va.h
+++ b/libavcodec/d3d11va.h
@@ -46,8 +46,7 @@
*/
/**
- * This structure is used to provides the necessary configurations and data
- * to the Direct3D11 FFmpeg HWAccel implementation.
+ * Context for the Direct3D11 FFmpeg HWAccel implementation
*
* The application must make it available as AVCodec
[FFmpeg-devel] [PATCH v6 3/4] all: Link to "context" from all public contexts with documentation
The goal of putting these links in "@see" blocks is to provide hooks
for future developers to add links to other useful parts of the codebase.
---
libavcodec/avcodec.h | 3 +++
libavcodec/bsf.h | 3 +++
libavcodec/d3d11va.h | 3 +++
libavcodec/mediacodec.h | 2 ++
libavcodec/qsv.h | 3 +++
libavcodec/vdpau.h | 3 +++
libavcodec/videotoolbox.h| 3 +++
libavfilter/avfilter.h | 7 ++-
libavformat/avformat.h | 3 +++
libavformat/avio.h | 3 +++
libavutil/audio_fifo.h | 3 +++
libavutil/hwcontext.h| 6 ++
libavutil/hwcontext_cuda.h | 3 +++
libavutil/hwcontext_d3d11va.h| 6 ++
libavutil/hwcontext_d3d12va.h| 6 ++
libavutil/hwcontext_drm.h| 3 +++
libavutil/hwcontext_dxva2.h | 6 ++
libavutil/hwcontext_mediacodec.h | 3 +++
libavutil/hwcontext_opencl.h | 6 ++
libavutil/hwcontext_qsv.h| 6 ++
libavutil/hwcontext_vaapi.h | 6 ++
libavutil/hwcontext_vdpau.h | 3 +++
libavutil/hwcontext_vulkan.h | 6 ++
libavutil/lfg.h | 3 +++
24 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 2da63c87ea..abc00ab394 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -441,6 +441,9 @@ typedef struct RcOverride{
* The AVOption/command line parameter names differ in some cases from the C
* structure field names for historic reasons or brevity.
* sizeof(AVCodecContext) must not be used outside libav*.
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVCodecContext {
/**
diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
index a09c69f242..ee5fdd48d2 100644
--- a/libavcodec/bsf.h
+++ b/libavcodec/bsf.h
@@ -64,6 +64,9 @@
* The fields in the struct will only be changed (by the caller or by the
* filter) as described in their documentation, and are to be considered
* immutable otherwise.
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVBSFContext {
/**
diff --git a/libavcodec/d3d11va.h b/libavcodec/d3d11va.h
index 27f40e5519..686974b083 100644
--- a/libavcodec/d3d11va.h
+++ b/libavcodec/d3d11va.h
@@ -52,6 +52,9 @@
* The application must make it available as AVCodecContext.hwaccel_context.
*
* Use av_d3d11va_alloc_context() exclusively to allocate an AVD3D11VAContext.
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVD3D11VAContext {
/**
diff --git a/libavcodec/mediacodec.h b/libavcodec/mediacodec.h
index 4e9b56a618..43f049a609 100644
--- a/libavcodec/mediacodec.h
+++ b/libavcodec/mediacodec.h
@@ -29,6 +29,8 @@
* This structure holds a reference to a android/view/Surface object that will
* be used as output by the decoder.
*
+ * @see
+ * - @ref Context
*/
typedef struct AVMediaCodecContext {
diff --git a/libavcodec/qsv.h b/libavcodec/qsv.h
index c156b08d07..8ab93af6b6 100644
--- a/libavcodec/qsv.h
+++ b/libavcodec/qsv.h
@@ -32,6 +32,9 @@
* - decoding: hwaccel_context must be set on return from the get_format()
* callback
* - encoding: hwaccel_context must be set before avcodec_open2()
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVQSVContext {
/**
diff --git a/libavcodec/vdpau.h b/libavcodec/vdpau.h
index 8021c25761..934c96b88c 100644
--- a/libavcodec/vdpau.h
+++ b/libavcodec/vdpau.h
@@ -74,6 +74,9 @@ typedef int (*AVVDPAU_Render2)(struct AVCodecContext *,
struct AVFrame *,
*
* The size of this structure is not a part of the public ABI and must not
* be used outside of libavcodec.
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVVDPAUContext {
/**
diff --git a/libavcodec/videotoolbox.h b/libavcodec/videotoolbox.h
index d68d76e400..81d90d63b6 100644
--- a/libavcodec/videotoolbox.h
+++ b/libavcodec/videotoolbox.h
@@ -53,6 +53,9 @@
* between the caller and libavcodec for initializing Videotoolbox decoding.
* Its size is not a part of the public ABI, it must be allocated with
* av_videotoolbox_alloc_context() and freed with av_free().
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVVideotoolboxContext {
/**
diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
index a34e61f23c..25ccd80433 100644
--- a/libavfilter/avfilter.h
+++ b/libavfilter/avfilter.h
@@ -403,7 +403,12 @@ unsigned avfilter_filter_pad_count(const AVFilter *filter,
int is_output);
*/
#define AVFILTER_THREAD_SLICE (1 << 0)
-/** An instance of a filter */
+/**
+ * An instance of a filter
+ *
+ * @see
+ * - @ref Context
+ */
struct AVFilterContext {
const AVClass *av_class;///< needed for av_log() and filters
common options
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 8afdcd9fd0..18f20f0bb0 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1253,6 +1253,9 @@ enum AVDurationEstimationMethod {
* can be found in libavformat/options_table.h.
* The AVOption/command line par
[FFmpeg-devel] [PATCH v6 2/4] lavu: Clarify relationship between AVClass, AVOption and context
---
libavutil/log.h | 16 +---
libavutil/opt.h | 26 +-
2 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/libavutil/log.h b/libavutil/log.h
index ab7ceabe22..88b35897c6 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -59,9 +59,19 @@ typedef enum {
struct AVOptionRanges;
/**
- * Describe the class of an AVClass context structure. That is an
- * arbitrary struct of which the first field is a pointer to an
- * AVClass struct (e.g. AVCodecContext, AVFormatContext etc.).
+ * Generic Logging facilities and configuration options
+ *
+ * Logging and AVOptions functions expect to be passed structs
+ * whose first member is a pointer-to-@ref AVClass.
+ *
+ * Structs that only use logging facilities are often referred to as
+ * "AVClass context structures", while those that provide configuration
+ * options are called "AVOptions-enabled structs".
+ *
+ * @see
+ * * @ref lavu_log
+ * * @ref avoptions
+ * * @ref Context
*/
typedef struct AVClass {
/**
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..cdee8f7d28 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -39,9 +39,16 @@
* @defgroup avoptions AVOptions
* @ingroup lavu_data
* @{
- * AVOptions provide a generic system to declare options on arbitrary structs
- * ("objects"). An option can have a help text, a type and a range of possible
- * values. Options may then be enumerated, read and written to.
+ *
+ * Inspection and configuration for AVClass context structures
+ *
+ * Provides a generic API to declare and manage options on any struct
+ * whose first member is a pointer-to-@ref AVClass. Structs with private
+ * contexts can use that AVClass to return further @ref AVClass "AVClass"es
+ * that allow access to options in the private structs.
+ *
+ * Each option can have a help text, a type and a range of possible values.
+ * Options may be enumerated, read and written to.
*
* There are two modes of access to members of AVOption and its child structs.
* One is called 'native access', and refers to access from the code that
@@ -53,11 +60,20 @@
* question is allowed to access the field. This allows us to extend the
* semantics of those fields without breaking API compatibility.
*
+ * Note that AVOptions is not reentrant, and that many FFmpeg functions access
+ * options from separate threads. Unless otherwise indicated, it is best to
+ * avoid modifying options once a struct has been initialized.
+ *
+ * @see
+ * * @ref lavu_log
+ * * @ref Context
+ *
* @section avoptions_scope Scope of AVOptions
*
* AVOptions is designed to support any set of multimedia configuration options
- * that can be defined at compile-time. Although it is mainly used to expose
- * FFmpeg options, you are welcome to adapt it to your own use case.
+ * that can be defined at compile-time and set at object creation time.
Although
+ * it is mainly used to expose FFmpeg options, you are welcome to adapt it
+ * to your own use case.
*
* No single approach can ever fully solve the problem of configuration,
* but please submit a patch if you believe you have found a problem
--
2.45.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 v6 1/4] doc: Explain what "context" means
Derived from explanations kindly provided by Stefano Sabatini and others:
https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325903.html
---
doc/context.md | 430 +
1 file changed, 430 insertions(+)
create mode 100644 doc/context.md
diff --git a/doc/context.md b/doc/context.md
new file mode 100644
index 00..bd8cb58696
--- /dev/null
+++ b/doc/context.md
@@ -0,0 +1,430 @@
+@page Context Introduction to contexts
+
+@tableofcontents
+
+FFmpeg uses the term “context” to refer to an idiom
+you have probably used before:
+
+```c
+// C structs often share context between functions:
+
+FILE *my_file; // my_file stores information about a filehandle
+
+printf(my_file, "hello "); // my_file provides context to this function,
+printf(my_file, "world!"); // and also to this function
+```
+
+```python
+# Python classes provide context for the methods they contain:
+
+class MyClass:
+def print(self,message):
+if self.prev_message != message:
+self.prev_message = message
+print(message)
+```
+
+
+```c
+// Many JavaScript callbacks accept an optional context argument:
+
+const my_object = {};
+
+my_array.forEach(function_1, my_object);
+my_array.forEach(function_2, my_object);
+```
+
+Be careful comparing FFmpeg contexts to things you're already familiar with -
+FFmpeg may sometimes happen to reuse words you recognise, but mean something
+completely different. For example, the AVClass struct has nothing to do with
+[object-oriented
classes](https://en.wikipedia.org/wiki/Class_(computer_programming)).
+
+If you've used contexts in other C projects, you may want to read
+@ref Context_comparison before the rest of the document.
+
+@section Context_general “Context” as a general concept
+
+@par
+A context is any data structure used by several functions
+(or several instances of the same function) that all operate on the same
entity.
+
+In the broadest sense, “context” is just a way to think about code.
+You can even use it to think about code written by people who have never
+heard the term, or who would disagree with you about what it means.
+Consider the following snippet:
+
+```c
+struct DualWriter {
+int fd1, fd2;
+};
+
+ssize_t write_to_two_files(
+struct DualWriter *my_writer,
+uint8_t *buf,
+int buf_size
+) {
+
+ssize_t bytes_written_1 = write(my_writer->fd1, buf, buf_size);
+ssize_t bytes_written_2 = write(my_writer->fd2, buf, buf_size);
+
+if ( bytes_written_1 != bytes_written_2 ) {
+// ... handle this edge case ...
+}
+
+return bytes_written_1;
+
+}
+
+int main() {
+
+struct DualWriter my_writer;
+my_writer.fd1 = open("file1", 0644, "wb");
+my_writer.fd2 = open("file2", 0644, "wb");
+
+write_to_two_files(&my_writer, "hello ", sizeof("hello "));
+write_to_two_files(&my_writer, "world!", sizeof("world!"));
+
+close( my_writer.fd1 );
+close( my_writer.fd2 );
+
+}
+```
+
+The term “context” doesn't appear anywhere in the snippet. But `DualWriter`
+is passed to several instances of `write_to_two_files()` that operate on
+the same entity, so it fits the definition of a context.
+
+When reading code that isn't explicitly described in terms of contexts,
+remember that your interpretation may differ from other people's.
+For example, FFmpeg's avio_alloc_context() accepts a set of callback functions
+and an `opaque` argument - even though this function guarantees to *return*
+a context, it does not require `opaque` to *provide* context for the callback
+functions. So you could choose to pass a struct like `DualWriter` as the
+`opaque` argument, or you could pass callbacks that use `stdin` and `stdout`
+and just pass a `NULL` argument for `opaque`.
+
+When reading code that *is* explicitly described in terms of contexts,
+remember that the term's meaning is guaranteed by *the project's community*,
+not *the language it's written in*. That means guarantees may be more flexible
+and change more over time. For example, programming languages that use
+[encapsulation](https://en.wikipedia.org/wiki/Encapsulation_(computer_programming))
+will simply refuse to compile code that violates its rules about access,
+while communities can put up with special cases if they improve code quality.
+
+The next section will discuss what specific conventions FFmpeg developers mean
+when they describe parts of their code as using “contexts”.
+
+@section Context_ffmpeg FFmpeg contexts
+
+This section discusses specific context-related conventions used in FFmpeg.
+Some of these are used in other projects, others are unique to this project.
+
+@subsection Context_indicating Indicating context: “Context”, “ctx” etc.
+
+```c
+// Context struct names usually end with `Context`:
+struct AVSomeContext {
+ ...
+};
+
+// Functions are usually named after their context,
+// context parameters usually come first and are often called `ctx`:
+void av_some_function(AVSomeContext *ctx, ...);
[FFmpeg-devel] [PATCH v6 0/4] doc: Explain what "context" means
I'm making a list of little documentation patches to submit as a set once this
patchset is done. I've put Sw{r,s}Context on the list, and will think about
their relationship to other opaque AVOptions-enabled structs as part of that.
I don't see anything in that discussion that affects this patchset,
so let's park that discussion for now.
One thing we haven't talked about before, but is worth stating explicitly -
smart people tend to conflate "I can't think of anything complex about X"
with "there are no complex things about X", so this document needs to sell
people on the complexity of each problem before laying out the solution.
The AVOptions section is a particularly good example, tackling the implicit
question "can't you just use `getopt`?" first then moving on to describe how
it solves that problem. Some parts of the document function as responses to
questions an FFmpeg developer would never think to ask, because why would you
even think to compare AVOptions with getopt?
This version emphasises how AVOptions should only be set during configuration.
It avoids the words "reflection" and "introspection" altogether, because IMHO
they imply an API that can be used in any stage of a struct's lifetime.
Aside: this is the latest in a series of issues where it initially seemed like
I was adding unnecessary terminology in places where it was confusing,
but turned out to be a symptom of a fundamental misunderstanding on my part.
If there's any such language remaining in this version, we should probably
look for those misunderstandings first and worry about language second.
___
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] FFmpeg 7.0 blocking issues
On Mon, Jun 03, 2024 at 11:32:37PM +0200, Michael Niedermayer wrote:
> On Sun, Jun 02, 2024 at 03:49:42PM +0200, Sebastian Ramacher wrote:
[...]
> > Just as a FYI: ffmpeg 7.0 breaks close to 70 reverse dependencies in
> > Debian. The list is available at [1]. So if you want ffmpeg X to be in
> > Debian Y or Ubuntu Z, X needs to be released at least half a year before
> > Y or Z freeze.
>
> Is there something that ffmpeg can do to reduce this breakage ?
> (i know its a bit of a lame question as its API brekages but i mean
> can the policy we have about deprecating API/ABI be amended in some way
> to make this easier ?
A quick look through Sebastian's list suggests two main groups of issues:
* channel layout migration
* things that didn't have attribute_deprecated before being removed
There are probably ways to reduce migration issues (e.g. Anton's point about
frequent small changes), but the quick win is to use attribute_deprecated more.
To be clear - attribute_deprecated is already used a lot. If those messages
were being ignored, we would expect to see more bugs reported for those things.
The fact that channel layout is the only deprecated functionality to get this
far suggests that attribute_deprecated is a good way to notify people about
all but the largest changes.
> Also am i correct that it should be easier if a X.1 with same API/ABI that is
> released 6 month after X.0 is targetet for the release ? Thats in fact kind
> of what i would have preferred anyway as the .1 likely has also fewer bugs
>
> And last but not least, someone needs to write down when .0 and .1 releases
> should
> be made so I dont forget it :)
Whatever decision is made about this, it would be good to update the
attribute_deprecated macro to point to a URL that lays out the policy.
Something like:
#define attribute_deprecated __attribute__((deprecated("see for
details")))
___
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 v5 0/6] avformat/network: improve ff_neterrno()
On Thu, May 16, 2024 at 12:59:05PM +0100, Andrew Sayers wrote: > On Thu, May 16, 2024 at 01:42:23PM +0300, Rémi Denis-Courmont wrote: > > Err, please. Keep this to the Windows back-end. Nothing good is going to > > happen with a function that does nothing (on other platforms) and has a > > nondescript numbered name. > > I have no strong opinion either way, and it feels rather bikesheddable. > Here's a version with the offending part moved to its own patch - > I'm happy for whoever applies this to decide whether they want to > keep or chuck patch 4/6 :) Ping? ___ 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] git problems
On Thu, May 30, 2024 at 01:30:09AM +0200, Michael Niedermayer wrote: > Hi all > > It seems the security update (https://ubuntu.com/security/notices/USN-6793-1) > broke public git > > We use gitolite that runs under its own user and serve git through apache > which runs under a different user. > Apache has only read access to the repositories > > Since the security update that stoped working, the logs are full of messages > telling that we need to add the repositories to safe.directory > (the commands suggested dont work and seem to mix up \t with a tab but thats > besides the point) > once the repository is added to safe.directory, which ive done with > https://git.ffmpeg.org/michael.git > the error is gone and everything looks fine in the logs on the server but it > still > doesnt work. (i have not touched ffmpeg.git config as i first wanted to test > this) > > So like i just said on IRC. i hope some of the other root admins will have > some more insight here. Or if you (yes YOU!) want to help or know something > please speak up. > > This is totally not my area and i think other people could find the issue > with less effort in less time and it would be more efficient if i work > on FFmpeg instead where the return per hour of my time should be much greater. > > Also gitweb and git over ssh seem uneffected and theres github > > If people want i could downgrade git OR > upgrade git to latest git ignoring official ubuntu packages > otherwise, i intend to leave this for someone else to investigate and rather > work on FFmpeg which just seems like a much better use of my time You've talked recently about looking for STF money to upgrade the servers. You might want to write up a postmortem when the bug is fixed, focussing on improvements that are unlikely to happen without money. Then you can say "we had X hours of downtime, we think Y jobs will reduce that by Z%". One thing for the postmortem - I don't know enough about these specific programs to do much with the description provided. And even if I did, I could only offer prose hints at a solution. But containerising these services would let me replicate the server locally, and suggest solutions as normal patches on the mailing list. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/4] doc: Explain what "context" means
On Wed, May 29, 2024 at 01:06:30PM +0200, Paul B Mahol wrote: > On Wed, May 29, 2024 at 12:50 PM Andrew Sayers > wrote: > [...] > > *Are AVOptions just command-line options?* > > > > I have trouble with statements like "AVOptions is a framework for options", > > both because it's circular and because the term "option" is too broad to be > > meaningful. > > > > I've previously pushed the word "reflection" on the assumption that options > > can be used anywhere variables are used. For example, imagine a decoder > > that > > could be reconfigured on-the-fly to reduce CPU usage at the cost of > > displaying > > blurier images. That can't be part of the public interface because it's > > codec-specific, but I could imagine updating some kind of "output_quality" > > AVOption as a user slides a slider up and down. > > > > But the CLI tools are largely non-interactive, so have I just > > misunderstood? > > > > How about saying "AVOptions is a framework for command-line options"? > > > > ffmpeg is cli tool > > libavfilter is library > > AVOptions is certainly and primarily not framework for command-line options. "Command-line option" might be the wrong word, but I've just checked write_number() in libavutil/opt.c, and it seems to do non-atomic updates without locking anything. That suggests I was indeed wrong to think it could be used on-the-fly - maybe "initial configuration options" would be a better term? Possibly with a warning somewhere about how AVOptions is not reentrant? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/4] doc: Explain what "context" means
Posting this separately, as these are practical "how does FFmpeg work" issues
vaguely inspired by recent discussions.
*How do namespaces work in FFmpeg?*
We've talked a bit about function namespaces recently. One reason I've
suggested they're a weak signal is because they aren't really addressed in the
documentation. How about adding something like this to the context doc:
Most FFmpeg functions are grouped into namespaces, usually indicated by
prefixes (e.g. `av_format_*`) but sometimes indicated by infixes
(e.g. av_alloc_format_context()). Namespaces group functions by *topic*,
which doesn't always correlate with *context*. For example, `av_find_*`
functions search for information across various contexts.
*Should external API devs treat Sw{r,s}Context as AVClass context structures?*
This is probably an uninteresting edge case, but just to be sure...
The website says Sw{r,s}Context start with AVClass[1], they have _get_class
functions, are shown in `ffmpeg -h full`, and I haven't found anything that says
to treat them differently to e.g. AVCodecContext. So I'm pretty sure these
are AVClass context structures, at least as far as internal devs are concerned.
But their definitions are only in the private interface, so the layout is just
an implementation detail that can change without even a major version bump.
AVFrame used to have a _get_class function despite never having an actual
AVClass member, so that's not a signal to external API devs. And `URLContext`
appears in `ffmpeg -h full` despite having being made private long ago,
so that's not much of a signal either.
My guess is that the above should be addressed with a patch like:
+/**
+ * @brief Context for SWResampler
+ *
+ * @note The public ABI only guarantees this is an AVOptions-enabled struct.
+ * Its size and other members are not a part of the public ABI.
+ *
+ * @see
+ * - @ref Context
+ */
struct SwrContext {
Let me know if the above is on the right track. If so, I'll queue up a patch
for after the context document is done.
*Are AVOptions just command-line options?*
I have trouble with statements like "AVOptions is a framework for options",
both because it's circular and because the term "option" is too broad to be
meaningful.
I've previously pushed the word "reflection" on the assumption that options
can be used anywhere variables are used. For example, imagine a decoder that
could be reconfigured on-the-fly to reduce CPU usage at the cost of displaying
blurier images. That can't be part of the public interface because it's
codec-specific, but I could imagine updating some kind of "output_quality"
AVOption as a user slides a slider up and down.
But the CLI tools are largely non-interactive, so have I just misunderstood?
How about saying "AVOptions is a framework for command-line options"?
[1] https://ffmpeg.org/doxygen/trunk/structSwrContext.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/4] doc: Explain what "context" means
On Tue, May 28, 2024 at 07:24:55PM +0200, Stefano Sabatini wrote: > > I think we start with different assumptions: you assume that most of > the readers are familiar with OOP jargon, and that they will leverage > the OOP jargon to understand the FFmpeg API. I think this is Not exactly. I'm saying the document has two equally valid use cases: 1. non-OOP developers, who need us to add new information to their brains 2. OOP developers, who need us to replace existing information in their brains So it's not a matter of "leveraging OOP" jargon so much as "calling out OOP assumptions". For example, I originally assumed AVClass was to FFmpeg as the Object struct[1] is to GObject, and struggled to take anything onboard until that been explicitly debunked. You may even be able to see that opinion gradually eroding in each rewrite of the context document. But actually, two groups is probably over-simplifying. Another group would be people coming from other C projects, who have incompatible ideas about contexts that would never occur to us. Or Fortran developers, who (according to the first link on Google[2]) can be expected to understand modules but not objects. > misleading: historically what defined FFmpeg is its minimalistic > approach, in this case we don't want the user, familiar with C, to be > familiar with OOP in order to use and understand the FFmpeg API, as > this is simply not necessary as FFmpeg is plain C language. > > In fact, if this might help with some users, this will probably > confuse all the others. Even more, if we adopt the FFmpeg jargon to > OOP (using "context") we are adding more confusion, as now the OOP > reader will have to adopt the FFmpeg jargon applied to OOP. > > My advice is to move all the content to OOP to a dedicated section, or > to make the references to OOP jargon as less as possible, to not get > in the way with the non-OOP reader. Earlier versions of the document probably drew too strong an analogy with OOP, but I suspect the problem may be simpler in the latest draft. The majority of OOP references explain how things are different to OOP - it might be possible to make these more readable, but they can't be moved or removed, because replacing information involves calling out the old information first. Only the first few OOP references draw the analogy in the positive, aiming to show OOP devs how this is similar but different. Given the OOP vs. non-OOP distinction is too simplistic, the solution might be to start the document with a collection of code samples - a generic C function, a Python object, a JavaScript event listener, and an FFmpeg sample. That would show how "context" is a name for a broad concept you've probably used before, without singling out OOP. It would also reinforce the idea of the document as a buffet of concepts - don't like the OOP example? Never mind, here are some procedural ones for you to enjoy. > > > Aside: if FFmpeg had a blog, I could turn this discussion into a great post > > called something like "reflections on object- vs. context-oriented > > development". > > But the project's voice is more objective than that, so this document is > > limited > > to discussing the subset of issues that relate specifically to the FFmpeg > > API. > > One option would be the wiki: > http://trac.ffmpeg.org/ > > but probably a personal blog entry would be better, given that this > would not be reference material but more as an article (since the API > and its own philosophy is a moving target). > > > > > On Sat, May 25, 2024 at 01:00:14PM +0200, Stefano Sabatini wrote: > > > > +Some functions fit awkwardly within FFmpeg's context idiom. For > > > > example, > > > > +av_ambient_viewing_environment_create_side_data() creates an > > > > +AVAmbientViewingEnvironment context, then adds it to the side-data of > > > > an > > > > +AVFrame context. > > > > > > To go back to this unfitting example, can you state what would be > > > fitting in this case? > > > > "Awkwardly" probably isn't the right word to use, but that's a language > > choice > > we can come back to. > > > > The problem with FFmpeg's interface isn't that any one part is illogical, > > it's that different parts of the interface follow incompatible logic. > > > > It's hard to give specific examples, because any given learner's journey > > looks > > like a random walk through the API, and you can always say "well nobody else > > would have that problem". But if everyone has a different problem, that > > means > > everyone has *a* problem, even though there's no localised code fix. > > Fine, but can you propose the signature of the functions that you > would consider optimal in this case? I think we largely agree on this, but since you ask - The code solution would be to pick a rule and rewrite the entire codebase to fit that rule. I don't have a strong opinion about what the rule is, but one example might be "the first context parameter to a function must be that fu
Re: [FFmpeg-devel] [PATCH] avcodec/mediacodec: Add support of dynamic bitrate
On Mon, May 27, 2024 at 01:49:47PM +0100, Dmitrii Okunev wrote: > MediaCodec supports parameter "video-bitrate" to change the bitrate > on fly. This commit add possibility to use it. > > It adds option -bitrate_ctrl_socket to the encoder which makes > the encoder to create an UNIX socket and listen for messages > to change the bitrate. > > An example of ffmpeg execution: > > ffmpeg -listen 1 -i rtmp://0.0.0.0:1935/live/myStream -c:v > hevc_mediacodec -bitrate_ctrl_socket /run/bitrate.sock -b:v 8M -f rtsp > rtsp://127.0.0.1:1935/live/reEncoded > > An example of changing the bitrate to 1000 BPS: > > printf '%016X' 1000 | xxd -r -p | socat -u STDIN UNIX:/run/bitrate.sock Nitpick: please do s/\* / \*/g on the following lines: > +const FFAMediaFormat* format_ctx) > +static int mediacodec_ndk_setParameters(FFAMediaCodec* ctx, > +const FFAMediaFormat* format_ctx) > +int (*setParameters)(FFAMediaCodec* codec, const FFAMediaFormat* format); (found by an in-progress review bot) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 2/3] avfilter/af_volumedetect.c: Add 32bit float audio support
On Mon, May 20, 2024 at 11:16:05PM +0300, Yigithan Yigit wrote:
> ---
> libavfilter/af_volumedetect.c | 159 --
> 1 file changed, 133 insertions(+), 26 deletions(-)
>
> diff --git a/libavfilter/af_volumedetect.c b/libavfilter/af_volumedetect.c
> index 327801a7f9..dbbcd037a5 100644
> --- a/libavfilter/af_volumedetect.c
> +++ b/libavfilter/af_volumedetect.c
> @@ -20,27 +20,51 @@
>
> #include "libavutil/channel_layout.h"
> #include "libavutil/avassert.h"
> +#include "libavutil/mem.h"
> #include "audio.h"
> #include "avfilter.h"
> #include "internal.h"
>
> +#define MAX_DB_FLT 1024
> #define MAX_DB 91
> +#define HISTOGRAM_SIZE 0x1
> +#define HISTOGRAM_SIZE_FLT (MAX_DB_FLT*2)
>
> typedef struct VolDetectContext {
> -/**
> - * Number of samples at each PCM value.
> - * histogram[0x8000 + i] is the number of samples at value i.
> - * The extra element is there for symmetry.
> - */
> -uint64_t histogram[0x10001];
> +uint64_t* histogram; ///< for integer number of samples at each PCM
> value, for float number of samples at each dB
Nitpick (from an in-progress review bot): s/\* / \*/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC 00/13] flvdec/flvenc: add support for enhanced rtmp codecs and multitrack/multichannel
On Tue, May 21, 2024 at 11:02:09AM +0200, Timo Rothenpieler wrote:
> This is based on the preliminary spec for enhanced rtmp v2:
> https://veovera.org/docs/enhanced/enhanced-rtmp-v2
>
> The spec is not final, and can still undergo breaking changes, hence this set
> is purely for comments and review, and not ready to be merged until the final
> v2 spec is published.
>
> There are no samples out in the wild yet, so testing interoperability with
> other software has not happened yet either.
> Specially the two other multitrack modes, where multiple tracks are in the
> same packet, have not been tested at all, since no software can write such
> files.
>
> The set can also be found on GitHub, where ignoring whitespaces makes
> specially the last patch a lot more readable:
> https://github.com/BtbN/FFmpeg/tree/enhanced-flv
>
I ran this against a little review bot I'm working on.
Please do s/\* / \*/g on the following:
avformat/flvenc: add support for writing multi track audio
> +static void flv_write_multichannel_header(AVFormatContext* s,
> AVCodecParameters* par, int64_t ts, int stream_index)
avformat/flvenc: write enhanced rtmp multichannel info for audio with more than
two channels
> +static void flv_write_multichannel_body(AVFormatContext* s,
> AVCodecParameters* par)
> +static int flv_get_multichannel_body_size(AVCodecParameters* par)
> +static void flv_write_multichannel_header(AVFormatContext* s,
> AVCodecParameters* par, int64_t ts)
avformat/flvenc: add enhanced audio codecs
> +static void flv_write_aac_header(AVFormatContext* s, AVCodecParameters* par)
avformat/flvenc: Implement support for multi-track video
> +static void flv_write_codec_header(AVFormatContext* s, AVCodecParameters*
> par, int64_t ts, int stream_index) {
___
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 v10 12/13] avcodec: add D3D12VA hardware HEVC encoder
On Wed, May 22, 2024 at 09:26:25AM +0800, tong1.wu-at-intel@ffmpeg.org wrote: > +static int d3d12va_create_encoder_heap(AVCodecContext* avctx) Nitpick: s/\* / \*/ I'm trying to write typo-detection bot. This is the only problem it noticed in this patchset, but more nits incoming elsewhere. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/4] doc: Explain what "context" means
It feels like we've got through most of the mid-level "how FFmpeg works" stuff, and now we're left with language choices (e.g "options" vs. "introspection") and philosophical discussions (e.g. the relationship between contexts and OOP). It's probably best to philosophise first, then come back to language. This message has been sent as a reply to one specific message, but is actually springboarding off messages from two sub-threads. Hopefully that will keep the big questions contained in one place. On Sat, May 25, 2024 at 11:49:48AM +0200, Stefano Sabatini wrote: > What perplexes me is that "context" is not part of the standard OOP > jargon, so this is probably adding more to the confusion. This actually speaks to a more fundamental issue about how we learn. To be clear, everything I'm about to describe applies every human that ever lived, but starting with this message makes it easier to explain (for reasons that will hopefully become obvious). When you ask why "context" is not part of OOP jargon, one could equally ask why "object" isn't part of FFmpeg jargon. The document hints at some arguments: their lifetime stages are different, their rules are enforced at the language vs. community level, OOP encourages homogenous interfaces while FFmpeg embraces unique interfaces that precisely suit each use case, and so on. But the honest answer is much simpler - humans are lazy, and we want the things we learn today to resemble the things we learnt yesterday. Put another way - if we had infinite time every day, we could probably write an object-oriented interface to FFmpeg. But our time is sadly finite so we stick with the thing that's proven to work. Similarly, if our readers had infinite free time every day, they could probably learn a completely new approach to programming. But their time is finite, so they stick to what they know. That means people reading this document aren't just passively soaking up information, they're looking for shortcuts that fit their assumptions. And as anyone that's ever seen a political discussion can tell you, humans are *really good* at finding shortcuts that fit their assumptions. For example, when an OOP developer sees the words "alloc" and "init", they will assume these map precisely to OOP allocators and initializers. One reason for the long section about context lifetimes is because it needs to meet them where they are, then walk them step-by-step to a better place. Aside: if FFmpeg had a blog, I could turn this discussion into a great post called something like "reflections on object- vs. context-oriented development". But the project's voice is more objective than that, so this document is limited to discussing the subset of issues that relate specifically to the FFmpeg API. On Sat, May 25, 2024 at 01:00:14PM +0200, Stefano Sabatini wrote: > > +Some functions fit awkwardly within FFmpeg's context idiom. For example, > > +av_ambient_viewing_environment_create_side_data() creates an > > +AVAmbientViewingEnvironment context, then adds it to the side-data of an > > +AVFrame context. > > To go back to this unfitting example, can you state what would be > fitting in this case? "Awkwardly" probably isn't the right word to use, but that's a language choice we can come back to. The problem with FFmpeg's interface isn't that any one part is illogical, it's that different parts of the interface follow incompatible logic. It's hard to give specific examples, because any given learner's journey looks like a random walk through the API, and you can always say "well nobody else would have that problem". But if everyone has a different problem, that means everyone has *a* problem, even though there's no localised code fix. For sake of argument, let's imagine a user who was a world-leading expert in Microsoft QBasic in the eighties, then fell into a forty-year coma and woke up in front of the FFmpeg documentation. In other words, a highly adept programmer with zero knowledge of programming conventions more recent than "a function is a special type of subroutine for returning a value". Their journey might look like... 1. there's this thing called "context", and some functions "have" contexts 2. sws_init_context() says "Initialize the swscaler context sws_context", and `sws_context` is a `SwsContext *`, so I think it has a SwsContext context 3. sws_alloc_context() says "Allocate an empty SwsContext", and it returns a `SwsContext *`, so I think it has the same context as sws_init_context() 4. avio_alloc_context() and avio_open2() are both variations on this theme, so I should look for creative ways to interpret things as "having" contexts 5. av_alloc_format_context() puts the type in the middle of the function name, so I should only treat prefixes as a weak signal 6. av_ambient_viewing_environment_create_side_data() allocates like an alloc, so I think the return value is the context; but it also operates on AVFrame in a way that affects related func
Re: [FFmpeg-devel] [RFC] STF 2025
On Fri, May 17, 2024 at 03:49:58PM +0200, Michael Niedermayer wrote: > Hi all > > Before this is forgotten again, better start some dicsussion too early than > too late This comment is inspired by the other subthread, but not directly in reply to it. I'm replying to this post rather than get in the middle of all that... What happens if someone is hired to do a job that requires access to the ML, then gets involved in a situation where there's talk of a ban? If they're banned, does that translate to suspension without pay? With pay? Banning such a person would jeopardise future funding - if they aren't banned, will people be concerned about the apparent conflict of interest? In a wider sense, hiring a single person to do a job we come to rely on (like code review) gives the project a bus number of 1. How would the STF react to a proposal like "we plan to do XYZ in 2025, but if we don't get funding for 2026, we'll drop Z and spend the time on a transition plan instead"? ___ 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 v5 0/4] Explain what "context" means
On Fri, May 24, 2024 at 03:50:52AM +0200, Michael Niedermayer wrote:
> On Thu, May 23, 2024 at 09:00:39PM +0100, Andrew Sayers wrote:
> > Imagine you wanted to write a system that nudged people to try new codecs.
> > It might say e.g. "you seem to be using H.264, would you like to try H.265?"
> > Implementing that would probably involve a struct like:
> >
> > struct AVOldNew {
> > AVClass* old;
> > AVClass* new;
> > };
>
> AVClass would describe the internal decoder structures. This would not be
> correct at all in this example.
> Thats like handing a man 2 CAD documents about 2 engines of 2 cars
>
> If you wanted to suggest to get a tesla instead of a ford. One would have to
> describe the 2 cars and their differences
> thats 2 AVCodecDescriptor maybe
Hmm, yes fair point. A better example might be a simple linked list:
struct AVClassList {
AVClass* cur;
AVClassList* next;
};
Again, that clearly is a struct that begins with AVClass*, but clearly isn't an
AVClass context structure.
I realise it's a bit of an academic distinction, but IMHO these hypotheticals
suggest it's more accurate to define the term "AVClass context structure"
in terms of usage rather than layout.
___
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 v5 4/4] all: Rewrite documentation for contexts
Make their documentation more readable and similar to each other,
(hopefully) without changing the meaning.
---
libavcodec/aac/aacdec.h | 2 +-
libavcodec/aacenc.h | 2 +-
libavcodec/ac3enc.h | 2 +-
libavcodec/amfenc.h | 2 +-
libavcodec/atrac.h | 2 +-
libavcodec/avcodec.h | 3 ++-
libavcodec/bsf.h | 2 +-
libavcodec/cbs.h | 2 +-
libavcodec/d3d11va.h | 3 +--
libavcodec/mediacodec.h | 4 ++--
libavcodec/pthread_frame.c | 4 ++--
libavcodec/qsv.h | 6 --
libavcodec/sbr.h | 2 +-
libavcodec/vdpau.h | 5 +++--
libavcodec/videotoolbox.h| 5 +++--
libavfilter/avfilter.h | 2 +-
libavformat/avformat.h | 3 ++-
libavformat/avio.h | 3 ++-
libavutil/audio_fifo.h | 2 +-
libavutil/hwcontext.h| 21 -
libavutil/hwcontext_cuda.h | 2 +-
libavutil/hwcontext_d3d11va.h| 4 ++--
libavutil/hwcontext_d3d12va.h| 6 +++---
libavutil/hwcontext_drm.h| 2 +-
libavutil/hwcontext_dxva2.h | 4 ++--
libavutil/hwcontext_mediacodec.h | 2 +-
libavutil/hwcontext_opencl.h | 4 ++--
libavutil/hwcontext_qsv.h| 4 ++--
libavutil/hwcontext_vaapi.h | 4 ++--
libavutil/hwcontext_vdpau.h | 2 +-
libavutil/hwcontext_vulkan.h | 5 +++--
libavutil/lfg.h | 3 ++-
32 files changed, 65 insertions(+), 54 deletions(-)
diff --git a/libavcodec/aac/aacdec.h b/libavcodec/aac/aacdec.h
index eed53c6c96..87a834797d 100644
--- a/libavcodec/aac/aacdec.h
+++ b/libavcodec/aac/aacdec.h
@@ -253,7 +253,7 @@ typedef struct AACDecDSP {
} AACDecDSP;
/**
- * main AAC decoding context
+ * Context for decoding AAC
*/
struct AACDecContext {
const struct AVClass *class;
diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h
index d07960620e..3e710c7fac 100644
--- a/libavcodec/aacenc.h
+++ b/libavcodec/aacenc.h
@@ -207,7 +207,7 @@ typedef struct AACPCEInfo {
} AACPCEInfo;
/**
- * AAC encoder context
+ * Context for encoding AAC
*/
typedef struct AACEncContext {
AVClass *av_class;
diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
index 5e98ad188b..e5abe0a856 100644
--- a/libavcodec/ac3enc.h
+++ b/libavcodec/ac3enc.h
@@ -153,7 +153,7 @@ typedef struct AC3Block {
struct PutBitContext;
/**
- * AC-3 encoder private context.
+ * Private context for encoding AC-3
*/
typedef struct AC3EncodeContext {
AVClass *av_class; ///< AVClass used for AVOption
diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
index 2dbd378ef8..184897beeb 100644
--- a/libavcodec/amfenc.h
+++ b/libavcodec/amfenc.h
@@ -43,7 +43,7 @@ typedef struct AmfTraceWriter {
} AmfTraceWriter;
/**
-* AMF encoder context
+* Context for encoding AMF
*/
typedef struct AmfContext {
diff --git a/libavcodec/atrac.h b/libavcodec/atrac.h
index 05208bbee6..e760f0384d 100644
--- a/libavcodec/atrac.h
+++ b/libavcodec/atrac.h
@@ -39,7 +39,7 @@ typedef struct AtracGainInfo {
} AtracGainInfo;
/**
- * Gain compensation context structure.
+ * Context for gain compensation
*/
typedef struct AtracGCContext {
float gain_tab1[16]; ///< gain compensation level table
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index abc00ab394..2fed4757ed 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -430,7 +430,8 @@ typedef struct RcOverride{
#define AV_GET_ENCODE_BUFFER_FLAG_REF (1 << 0)
/**
- * main external API structure.
+ * Context for an encode or decode session
+ *
* New fields can be added to the end with minor version bumps.
* Removal, reordering and changes to existing fields require a major
* version bump.
diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
index ee5fdd48d2..fadcfc5d47 100644
--- a/libavcodec/bsf.h
+++ b/libavcodec/bsf.h
@@ -56,7 +56,7 @@
*/
/**
- * The bitstream filter state.
+ * Context for bitstream filtering
*
* This struct must be allocated with av_bsf_alloc() and freed with
* av_bsf_free().
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index d479b1ac2d..c074dd11ec 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -214,7 +214,7 @@ typedef void (*CBSTraceWriteCallback)(void *trace_context,
int64_t value);
/**
- * Context structure for coded bitstream operations.
+ * Context for coded bitstream operations
*/
typedef struct CodedBitstreamContext {
/**
diff --git a/libavcodec/d3d11va.h b/libavcodec/d3d11va.h
index 686974b083..5ffee2b3be 100644
--- a/libavcodec/d3d11va.h
+++ b/libavcodec/d3d11va.h
@@ -46,8 +46,7 @@
*/
/**
- * This structure is used to provides the necessary configurations and data
- * to the Direct3D11 FFmpeg HWAccel implementation.
+ * Context for the Direct3D11 FFmpeg HWAccel implementation
*
* The application must make it available as AVCodec
[FFmpeg-devel] [PATCH v5 3/4] all: Link to "context" from all public contexts with documentation
The goal of putting these links in "@see" blocks is to provide hooks
for future developers to add links to other useful parts of the codebase.
---
libavcodec/avcodec.h | 3 +++
libavcodec/bsf.h | 3 +++
libavcodec/d3d11va.h | 3 +++
libavcodec/mediacodec.h | 2 ++
libavcodec/qsv.h | 3 +++
libavcodec/vdpau.h | 3 +++
libavcodec/videotoolbox.h| 3 +++
libavfilter/avfilter.h | 7 ++-
libavformat/avformat.h | 3 +++
libavformat/avio.h | 3 +++
libavutil/audio_fifo.h | 3 +++
libavutil/hwcontext.h| 6 ++
libavutil/hwcontext_cuda.h | 3 +++
libavutil/hwcontext_d3d11va.h| 6 ++
libavutil/hwcontext_d3d12va.h| 6 ++
libavutil/hwcontext_drm.h| 3 +++
libavutil/hwcontext_dxva2.h | 6 ++
libavutil/hwcontext_mediacodec.h | 3 +++
libavutil/hwcontext_opencl.h | 6 ++
libavutil/hwcontext_qsv.h| 6 ++
libavutil/hwcontext_vaapi.h | 6 ++
libavutil/hwcontext_vdpau.h | 3 +++
libavutil/hwcontext_vulkan.h | 6 ++
libavutil/lfg.h | 3 +++
24 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 2da63c87ea..abc00ab394 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -441,6 +441,9 @@ typedef struct RcOverride{
* The AVOption/command line parameter names differ in some cases from the C
* structure field names for historic reasons or brevity.
* sizeof(AVCodecContext) must not be used outside libav*.
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVCodecContext {
/**
diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
index a09c69f242..ee5fdd48d2 100644
--- a/libavcodec/bsf.h
+++ b/libavcodec/bsf.h
@@ -64,6 +64,9 @@
* The fields in the struct will only be changed (by the caller or by the
* filter) as described in their documentation, and are to be considered
* immutable otherwise.
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVBSFContext {
/**
diff --git a/libavcodec/d3d11va.h b/libavcodec/d3d11va.h
index 27f40e5519..686974b083 100644
--- a/libavcodec/d3d11va.h
+++ b/libavcodec/d3d11va.h
@@ -52,6 +52,9 @@
* The application must make it available as AVCodecContext.hwaccel_context.
*
* Use av_d3d11va_alloc_context() exclusively to allocate an AVD3D11VAContext.
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVD3D11VAContext {
/**
diff --git a/libavcodec/mediacodec.h b/libavcodec/mediacodec.h
index 4e9b56a618..43f049a609 100644
--- a/libavcodec/mediacodec.h
+++ b/libavcodec/mediacodec.h
@@ -29,6 +29,8 @@
* This structure holds a reference to a android/view/Surface object that will
* be used as output by the decoder.
*
+ * @see
+ * - @ref Context
*/
typedef struct AVMediaCodecContext {
diff --git a/libavcodec/qsv.h b/libavcodec/qsv.h
index c156b08d07..8ab93af6b6 100644
--- a/libavcodec/qsv.h
+++ b/libavcodec/qsv.h
@@ -32,6 +32,9 @@
* - decoding: hwaccel_context must be set on return from the get_format()
* callback
* - encoding: hwaccel_context must be set before avcodec_open2()
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVQSVContext {
/**
diff --git a/libavcodec/vdpau.h b/libavcodec/vdpau.h
index 8021c25761..934c96b88c 100644
--- a/libavcodec/vdpau.h
+++ b/libavcodec/vdpau.h
@@ -74,6 +74,9 @@ typedef int (*AVVDPAU_Render2)(struct AVCodecContext *,
struct AVFrame *,
*
* The size of this structure is not a part of the public ABI and must not
* be used outside of libavcodec.
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVVDPAUContext {
/**
diff --git a/libavcodec/videotoolbox.h b/libavcodec/videotoolbox.h
index d68d76e400..81d90d63b6 100644
--- a/libavcodec/videotoolbox.h
+++ b/libavcodec/videotoolbox.h
@@ -53,6 +53,9 @@
* between the caller and libavcodec for initializing Videotoolbox decoding.
* Its size is not a part of the public ABI, it must be allocated with
* av_videotoolbox_alloc_context() and freed with av_free().
+ *
+ * @see
+ * - @ref Context
*/
typedef struct AVVideotoolboxContext {
/**
diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
index a34e61f23c..25ccd80433 100644
--- a/libavfilter/avfilter.h
+++ b/libavfilter/avfilter.h
@@ -403,7 +403,12 @@ unsigned avfilter_filter_pad_count(const AVFilter *filter,
int is_output);
*/
#define AVFILTER_THREAD_SLICE (1 << 0)
-/** An instance of a filter */
+/**
+ * An instance of a filter
+ *
+ * @see
+ * - @ref Context
+ */
struct AVFilterContext {
const AVClass *av_class;///< needed for av_log() and filters
common options
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 8afdcd9fd0..18f20f0bb0 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1253,6 +1253,9 @@ enum AVDurationEstimationMethod {
* can be found in libavformat/options_table.h.
* The AVOption/command line par
[FFmpeg-devel] [PATCH v5 2/4] lavu: Clarify relationship between AVClass, AVOption and context
---
libavutil/log.h | 16 +---
libavutil/opt.h | 17 ++---
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/libavutil/log.h b/libavutil/log.h
index ab7ceabe22..d599ab506e 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -59,9 +59,19 @@ typedef enum {
struct AVOptionRanges;
/**
- * Describe the class of an AVClass context structure. That is an
- * arbitrary struct of which the first field is a pointer to an
- * AVClass struct (e.g. AVCodecContext, AVFormatContext etc.).
+ * Generic Logging and introspection facilities
+ *
+ * Logging and introspection functions expect to be passed structs
+ * whose first member is a pointer-to-@ref AVClass.
+ *
+ * Structs that only use the logging facilities are often referred to as
+ * "AVClass context structures", while those that use introspection facilities
+ * are called "AVOptions-enabled structs".
+ *
+ * @see
+ * * @ref lavu_log
+ * * @ref avoptions
+ * * @ref Context
*/
typedef struct AVClass {
/**
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..b14c120e36 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -39,9 +39,16 @@
* @defgroup avoptions AVOptions
* @ingroup lavu_data
* @{
- * AVOptions provide a generic system to declare options on arbitrary structs
- * ("objects"). An option can have a help text, a type and a range of possible
- * values. Options may then be enumerated, read and written to.
+ *
+ * Generic introspection facilities for AVClass context structures
+ *
+ * Provides a generic system to declare and manage options on any struct
+ * whose first member is a pointer-to-@ref AVClass. Structs with private
+ * contexts can use that AVClass to return further @ref AVClass "AVClass"es
+ * that enable introspection of the private structs.
+ *
+ * Each option can have a help text, a type and a range of possible values.
+ * Options may be enumerated, read and written to.
*
* There are two modes of access to members of AVOption and its child structs.
* One is called 'native access', and refers to access from the code that
@@ -53,6 +60,10 @@
* question is allowed to access the field. This allows us to extend the
* semantics of those fields without breaking API compatibility.
*
+ * @see
+ * * @ref lavu_log
+ * * @ref Context
+ *
* @section avoptions_scope Scope of AVOptions
*
* AVOptions is designed to support any set of multimedia configuration options
--
2.43.0
___
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 v5 1/4] doc: Explain what "context" means
Derived from explanations kindly provided by Stefano Sabatini and others:
https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325903.html
---
doc/context.md | 439 +
1 file changed, 439 insertions(+)
create mode 100644 doc/context.md
diff --git a/doc/context.md b/doc/context.md
new file mode 100644
index 00..21469a6e58
--- /dev/null
+++ b/doc/context.md
@@ -0,0 +1,439 @@
+@page Context Introduction to contexts
+
+@tableofcontents
+
+“Context” is a name for a widely-used programming idiom.
+This document explains the general idiom and some conventions used by FFmpeg.
+
+This document uses object-oriented analogies for readers familiar with
+[object-oriented
programming](https://en.wikipedia.org/wiki/Object-oriented_programming).
+But contexts can also be used outside of OOP, and even in situations where OOP
+isn't helpful. So these analogies should only be used as a rough guide.
+
+@section Context_general “Context” as a general concept
+
+Many projects use some kind of “context” idiom. You can safely skip this
+section if you have used contexts in another project. You might also prefer to
+read @ref Context_comparison before continuing with the rest of the document.
+
+@subsection Context_think “Context” as a way to think about code
+
+A context is any data structure that is passed to several functions
+(or several instances of the same function) that all operate on the same
entity.
+For example, [object-oriented
programming](https://en.wikipedia.org/wiki/Object-oriented_programming)
+languages usually provide member functions with a `this` or `self` value:
+
+```python
+# Python methods (functions within classes) must start with an object argument,
+# which does a similar job to a context:
+class MyClass:
+def my_func(self):
+...
+```
+
+Contexts can also be used in C-style procedural code. If you have ever written
+a callback function, you have probably used a context:
+
+```c
+struct FileReader {
+FILE* file;
+};
+
+int my_callback(void *my_var_, uint8_t* buf, int buf_size) {
+
+// my_var provides context for the callback function:
+struct FileReader *my_var = (struct FileReader *)my_var_;
+
+return read(my_var->file, sizeof(*buf), buf_size);
+}
+
+void init() {
+
+struct FileReader my_var;
+my_var->file = fopen("my-file", "rb");
+
+register_callback(my_callback, &my_var);
+
+...
+
+fclose( my_var->file );
+
+}
+```
+
+In the broadest sense, a context is just a way to think about some code.
+You can even use it to think about code written by people who have never
+heard the term, or who would disagree with you about what it means.
+But when FFmpeg developers say “context”, they're usually talking about
+a more specific set of conventions.
+
+@subsection Context_communication “Context” as a tool of communication
+
+“Context“ can just be a word to understand code in your own head,
+but it can also be a term you use to explain your interfaces.
+Here is a version of the callback example that makes the context explicit:
+
+```c
+struct FileReaderContext {
+FILE *file;
+};
+
+int my_callback(void *ctx_, uint8_t *buf, int buf_size) {
+
+// ctx provides context for the callback function:
+struct FileReaderContext *ctx = (struct FileReaderContext *)ctx_;
+
+return read(ctx->file, sizeof(*buf), buf_size);
+}
+
+void init() {
+
+struct FileReader ctx;
+ctx->file = fopen("my-file", "rb");
+
+register_callback(my_callback, &ctx);
+
+...
+
+fclose( ctx->file );
+
+}
+```
+
+The difference here is subtle, but important. If a piece of code
+*appears compatible with contexts*, then you are *allowed to think
+that way*, but if a piece of code *explicitly states it uses
+contexts*, then you are *required to follow that approach*.
+
+For example, take a look at avio_alloc_context().
+The function name and return value both state it uses contexts,
+so failing to follow that approach is a bug you can report.
+But its arguments are a set of callbacks that merely appear compatible with
+contexts, so it's fine to write a `read_packet` function that just reads
+from standard input.
+
+When a programmer says their code is "a context", they're guaranteeing
+to follow a set of conventions enforced by their community - for example,
+the FFmpeg community enforces that contexts have separate allocation,
+configuration, and initialization steps. That's different from saying
+their code is "an object", which normally guarantees to follow conventions
+enforced by their programming language (e.g. using a constructor function).
+
+@section Context_ffmpeg FFmpeg contexts
+
+This section discusses specific context-related conventions used in FFmpeg.
+Some of these are used in other projects, others are unique to this project.
+
+@subsection Context_naming Naming: “Context” and “ctx”
+
+```c
+// Context struct names usually end with `Context`:
+struct AVSomeContext {
+ ...
+};
+
+// Function
[FFmpeg-devel] [PATCH v5 0/4] Explain what "context" means
NOTE: this patchset depends on [1], and should not be applied before that.
I think it's important to guide readers between parts of FFmpeg, because
learning how the pieces of the puzzle fit together is a big part of the
newbie experience. So this patchset replaces the "@ref Context for foo"
statements in public structs with "@see" blocks, giving us a hook we can
hang more links on in future.
That said, there's a rule against internal links from private structs,
so I've removed the @ref's from them. By the way, is this rule written
somewhere? If not, where would be a good place to write it?
And either way, it would be good to link to this as part of [2].
Previous patches had to change the language for many structs, but "@see" blocks
avoid the need to include those changes in this patchset. Rather than waste
that work, I've temporarily moved those changes to the final patch in this set.
My feelings about that last patch aren't strong, but I guess I'll propose them
in a separate thread unless anyone wants them here or chucked altogether.
I've rewritten AVOptions and AVClass based on feedback. The new version
reflects a hypothetical that's been going round my head all week...
Imagine you wanted to write a system that nudged people to try new codecs.
It might say e.g. "you seem to be using H.264, would you like to try H.265?"
Implementing that would probably involve a struct like:
struct AVOldNew {
AVClass* old;
AVClass* new;
};
That's a struct that begins with an AVClass*, but is clearly not an AVClass
context structure. So the new version defines "AVClass context structure" and
"AVOptions-enabled struct" in terms of the way the structs are used instead of
their layout, which should be more useful and accurate to current practice,
while remaining compatible(ish) with the way the words are used in conversation.
I mentioned hwaccels in a previous message. From another look around the code,
I think these are supposed to be completely invisible to an external API dev.
If not, please point me in the direction of any documentation I missed.
[1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/327958.html
[2] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/327624.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 4/4] lavf: Add documentation for private "Context" classes
On Wed, May 22, 2024 at 05:24:36PM +0200, Andreas Rheinhardt wrote: > These structures should be renamed instead of adding these comments > (which are pointless for internal developers). I just sent a patch for that. > Thanks for pointing out the issue. Oh, great! So the next version of this patchset will skip this patch, and will reduce links like this: > + * @brief @ref md_doc_2context "Context" for a cache down to: > + * @brief @ref Context for a cache I don't see a way of removing the "@ref" without doing something nasty, like making a fake "Context" struct and shoving the documentation in there. Also, if someone does something strange like this: > + * @brief @ref Context "structure" (actually an enum) doxygen won't render it the way the author expects. I don't expect that to happen much in the real world though. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/4] doc: Explain what "context" means
On Wed, May 22, 2024 at 11:31:52AM +0200, Stefano Sabatini wrote:
> Sorry for the slow reply.
Welcome back :)
I've gathered some critiques of my own over the past week, which I'll pepper
throughout the reply. Starting with...
The document assumes (or is at least designed to be secure against) readers
starting at the top and reading through to the bottom. I found doxygen's
@tableofcontents command while writing this e-mail, which I will definitely
use in the next version, and which might provoke a rewrite aimed at people
jumping around the document looking for answers to specific questions.
>
> On date Wednesday 2024-05-15 16:54:19 +0100, Andrew Sayers wrote:
> > Derived from detailed explanations kindly provided by Stefano Sabatini:
> > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325903.html
> > ---
> > doc/context.md | 394 +
> > 1 file changed, 394 insertions(+)
> > create mode 100644 doc/context.md
> >
> > diff --git a/doc/context.md b/doc/context.md
> > new file mode 100644
> > index 00..fb85b3f366
> > --- /dev/null
> > +++ b/doc/context.md
> > @@ -0,0 +1,394 @@
> > +# Introduction to contexts
> > +
> > +“%Context”
>
> Is this style of quoting needed? Especially I'd avoid special markup
> to simplify unredendered text reading (which is the point of markdown
> afterall).
Short answer: I'll change it in the next patch and see what happens.
Long answer: HTML quotes are ugly for everyone, UTF-8 is great until someone
turns up complaining we broke their Latin-1 workflow. I've always preferred
ASCII-only representations for that reason, but happy to try the other way
and see if anyone still cares.
>
> > is a name for a widely-used programming idiom.
>
> > +This document explains the general idiom and the conventions FFmpeg has
> > built around it.
> > +
> > +This document uses object-oriented analogies to help readers familiar with
> > +[object-oriented
> > programming](https://en.wikipedia.org/wiki/Object-oriented_programming)
> > +learn about contexts. But contexts can also be used outside of OOP,
> > +and even in situations where OOP isn't helpful. So these analogies
> > +should only be used as a first step towards understanding contexts.
> > +
> > +## “Context” as a way to think about code
> > +
> > +A context is any data structure that is passed to several functions
> > +(or several instances of the same function) that all operate on the same
> > entity.
> > +For example, [object-oriented
> > programming](https://en.wikipedia.org/wiki/Object-oriented_programming)
> > +languages usually provide member functions with a `this` or `self` value:
> > +
>
> > +```c
> > +class my_cxx_class {
> > + void my_member_function() {
> > +// the implicit object parameter provides context for the member
> > function:
> > +std::cout << this;
> > + }
> > +};
> > +```
>
> I'm not convinced this is really useful: if you know C++ this is
> redundant, if you don't this is confusing and don't add much information.
The example is there to break up a wall of text (syntax-highlighted in the
rendered output), and to let the reader know that this is going to be one of
those documents that alternates between text and code, so they're ready for the
more substantive examples later on. I take the point about C++ though -
would this Python example be more readable?
class MyClass:
def my_func(self):
# If a Python function is part of a class,
# its first parameter must be an instance of that class
>
> > +
> > +Contexts are a fundamental building block of OOP, but can also be used in
> > procedural code.
>
> I'd drop this line, and drop the anchor on OOP at the same time since
> it's adding no much information.
Fundamentally, this document addresses two audiences:
1. people coming from a non-OOP background, who want to learn contexts
from first principles, and at best see OOP stuff as background information
2. people coming from an OOP background. There's no polite way to say this -
their incentive is to write FFmpeg off as a failed attempt at OOP, so they
don't have to learn a new way of working that's just different enough to
make them feel dumb
I think a good way to evaluate the document might be to read it through twice,
stopping after each paragraph to ask two unfair questions...
1. what has this told me about FFmpeg itself, as opposed to some other thing
you wish I cared about?
2. couldn't you have just done this the standard OOP way?
The ear
Re: [FFmpeg-devel] [PATCH v4 4/4] lavf: Add documentation for private "Context" classes
On Wed, May 22, 2024 at 12:08:29PM +0200, Stefano Sabatini wrote:
> On date Wednesday 2024-05-15 16:54:22 +0100, Andrew Sayers wrote:
> > Doxygen thinks any text like "Context for foo" is a link to a struct called
> > "Context".
> > Add a description and a better link, to avoid confusing readers.
> > ---
> > libavformat/async.c | 3 +++
> > libavformat/cache.c | 3 +++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/libavformat/async.c b/libavformat/async.c
> > index e096b0bc6f..3c28d418ae 100644
> > --- a/libavformat/async.c
> > +++ b/libavformat/async.c
> > @@ -53,6 +53,9 @@ typedef struct RingBuffer
> > int read_pos;
> > } RingBuffer;
> >
> > +/**
> > + * @brief @ref md_doc_2context "Context" for testing async
> > + */
> > typedef struct Context {
> > AVClass*class;
> > URLContext *inner;
> > diff --git a/libavformat/cache.c b/libavformat/cache.c
> > index 5f78adba9d..3cc0edec82 100644
> > --- a/libavformat/cache.c
> > +++ b/libavformat/cache.c
> > @@ -52,6 +52,9 @@ typedef struct CacheEntry {
> > int size;
> > } CacheEntry;
> >
> > +/**
> > + * @brief @ref md_doc_2context "Context" for a cache
> > + */
> > typedef struct Context {
> > AVClass *class;
> > int fd;
>
> Not sure, these are private structs and we enforce no use of internal
> markup for those, and we can assume internals developers are already
> acquainted with contexts and such so this is probably adding no value.
Ah, yeah the use case isn't obvious if you haven't tripped over it...
Imagine you're a new user trying to learn FFmpeg, and you find yourself at
https://ffmpeg.org/doxygen/trunk/structAVAudioFifo.html - the first word
appears to be a link for this "context" thing you've been hearing about[1],
so you drop what you're doing to investigate. It links you to a struct that
looks promisingly general, but eventually turns out to be some random internal
struct with a misleading name. Now you've wasted a bunch of time and forgotten
what you were doing.
The other patch fixes the examples I've found in the code, but doxygen links
all instances of the word "Context" to this struct, confusing newbies. This
patch ensures that when future developers say "Context" in a comment, the page
doxygen links them to will point them in the right direction.
I had previously assumed it was a deliberate decision to include private files
in the documentation, so I didn't look that carefully into workarounds that
would break links to these private structures. But you've implied elsewhere
that's not the case, so is it worth looking into solutions that made "Context"
link to the context document, at the cost of making it impossible to link to
these private structs at all?
[1] to be clear, this is the current behaviour of the page on the live site
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 1/3] doc: Explain what "context" means
On Wed, May 22, 2024 at 12:37:37PM +0200, Stefano Sabatini wrote: > On date Sunday 2024-05-05 22:04:36 +0100, Andrew Sayers wrote: > > I'm still travelling, so the following thoughts might be a bit > > half-formed. But I wanted to get some feedback before sitting down > > for a proper think. > [...] > > > > I've also gone through the code looking for edge cases we haven't > > > > covered. > > > > Here are some questions trying to prompt an "oh yeah I forgot to mention > > > > that"-type answer. Anything where the answer is more like "that should > > > > probably be rewritten to be clearer", let me know and I'll avoid > > > > confusing > > > > newbies with it. > > > > > > > > > > > av_ambient_viewing_environment_create_side_data() takes an AVFrame as > > > > its > > > > first argument, and returns a new AVAmbientViewingEnvironment. What is > > > > the > > > > context object for that function - AVFrame or > > > > AVAmbientViewingEnvironment? > > > > > > But this should be clear from the doxy: > > > > > > /** > > > * Allocate and add an AVAmbientViewingEnvironment structure to an > > > existing > > > * AVFrame as side data. > > > * > > > * @return the newly allocated struct, or NULL on failure > > > */ > > > AVAmbientViewingEnvironment > > > *av_ambient_viewing_environment_create_side_data(AVFrame *frame); > > > > I'm afraid it's not clear, at least to me. I think you're saying the > > AVFrame is the context because a "create" function can't have a > > context any more than a C++ "new" can be called as a member. But the > > function's prefix points to the other conclusion, and neither signal > > is clear enough on its own. > > No, what I'm saying is that in some cases you don't need to think in > terms of contexts, in this case there is no context at all, the > function takes a frame and modify it, and returns the ambient > environment to be used by the following functions. This should be very > clear by reading the doxy. There is no rule dictating the first param > of each FFmpeg function should be a "context". I'm still writing up a reply to your longer feedback, but on this topic... This function is in the "av_ambient_viewing_environment" namespace, and returns an object that is clearly used as a context for other functions. So saying "stop thinking about contexts" just leaves a negative space and a bad thing to fill it with (confusion in my case). I've found it useful to think about "receiving" vs. "producing" a context: * avcodec_alloc_context3() produces a context, but does not receive one * sws_init_context() receives a context, but does not produce one * av_ambient_viewing_environment_create_side_data() receives one context, and produces another How about if the document mostly talks about functions as having contexts, then follows it up with something like: There are some edge cases where this doesn't work. . If you find contexts a useful metaphor in these cases, you might prefer to think about them as "receiving" and "producing" contexts. ... or something similar that acknowledges contexts are unnecessary here, but provides a solution for people that want to use them anyway. ___ 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 v9 11/13] avutil/hwcontext_d3d12va: add Flags for resource creation
(Only reviewing documentation, not code)
On Mon, May 20, 2024 at 10:52:20PM +0800, tong1.wu-at-intel@ffmpeg.org
wrote:
> From: Tong Wu
>
> Flags field is added to support diffferent resource creation.
>
> Signed-off-by: Tong Wu
> ---
> doc/APIchanges| 3 +++
> libavutil/hwcontext_d3d12va.c | 2 +-
> libavutil/hwcontext_d3d12va.h | 8
> libavutil/version.h | 2 +-
> 4 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 269fd36559..808ba02f2d 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
>
> API changes, most recent first:
>
> +2024-01-xx - xx - lavu 59.20.100 - hwcontext_d3d12va.h
> + Add AVD3D12VAFramesContext.flags
> +
> 2024-05-xx - xx - lavu 59.19.100 - hwcontext_qsv.h
>Add AVQSVFramesContext.info
>
> diff --git a/libavutil/hwcontext_d3d12va.c b/libavutil/hwcontext_d3d12va.c
> index cfc016315d..6507cf69c1 100644
> --- a/libavutil/hwcontext_d3d12va.c
> +++ b/libavutil/hwcontext_d3d12va.c
> @@ -247,7 +247,7 @@ static AVBufferRef *d3d12va_pool_alloc(void *opaque,
> size_t size)
> .Format = hwctx->format,
> .SampleDesc = {.Count = 1, .Quality = 0 },
> .Layout = D3D12_TEXTURE_LAYOUT_UNKNOWN,
> -.Flags= D3D12_RESOURCE_FLAG_NONE,
> +.Flags= hwctx->flags,
> };
>
> frame = av_mallocz(sizeof(AVD3D12VAFrame));
> diff --git a/libavutil/hwcontext_d3d12va.h b/libavutil/hwcontext_d3d12va.h
> index ff06e6f2ef..608dbac97f 100644
> --- a/libavutil/hwcontext_d3d12va.h
> +++ b/libavutil/hwcontext_d3d12va.h
> @@ -129,6 +129,14 @@ typedef struct AVD3D12VAFramesContext {
> * If unset, will be automatically set.
> */
> DXGI_FORMAT format;
> +
> +/**
> + * This field is used to specify options for working with resources.
> + * If unset, this will be D3D12_RESOURCE_FLAG_NONE.
> + *
> + * @see:
> https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ne-d3d12-d3d12_resource_flags.
> + */
> +D3D12_RESOURCE_FLAGS flags;
Some nitpicks:
* "This field is used to specify" is redundant, you can save the reader
a few seconds by starting the sentence with just "Options..."
* "@see" starts a paragraph, so the rendered documentation will look better
without the ":"
* the full stop after the URL makes it harder to copy/paste the text -
remove the full stop or use a [markdown link](...)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] STF 2025
On Sun, May 19, 2024 at 01:29:43PM +0200, Thilo Borgmann via ffmpeg-devel wrote: > > [...] > > > * Fund administrative / maintainance work (one example is the mailman > > > upgrade that is needed > > > with the next OS upgrade on one of our servers (this is not as trivial > > > as one might > > > expect). Another example here may be some git related tools if we find > > > something that > > > theres a broad consensus about. > > > > I agree that this should be paid but I would expect that STF would not be > > too keen on it, not that I'd know really. > > We should absolutely pay for such activity and STF is very well willing to > fund such things. > > > > > * Fund maintaince on the bug tracker, try to reproduce bugs, ask users to > > > provide > > > reproduceable cases, close bugs still unreproduceable, ... > > > ATM we have over 2000 "new" bugs that are not even marked as open > > > > This is a double-edged sword. If somebody gets paid to do that, then that > > is one more reason for others not to do it. > > > > And again, it is completely reasonable to be paid for that, and also for > > code reviews and writing test cases (if we want to complete the menial task > > list), but I am perplexed as to STF's stance on that. > > Same as above about that we should and STF would. Especially since no > corporate interest usually pays anyone for these tasks (in case of reviews > it might of course be considered a good thing). > > The one problem to solve here AFAICT is we don't know exactly what quantity > of bugs, reviewable code submissions and other maintenance work will come up > in the next 12 months. > So it renders impossible to define in prior the workload, milestones and > compensation per contributor interested as we did this year for well-defined > tasks. > > What we should consider IMO is defining the tasks (patch review, bug review > & fix, FATE extensions, checkasm extensions, etc. as well such things for > the administrative tasks from above) and defining a budget for these tasks. > Then, allow 'everyone interested' (aka git push access?) to claim a part of > that budget every N-months, depending what the corresponding contributor > actually did and can somehow be determined. Another solution would be to have a variable-sized primary task, with a secondary task that can absorb leftover time. For example, if your primary task was reviewing patches, your secondary task might be improving the patch review process. So when you get to the point where you'd rather let someone else claim a bounty than say "fix your indentation" one more time, your incentive is instead to write a tutorial, or a review bot, or otherwise get to the root cause. ___ 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] doc/developer: add examples to clarify code style
I would have found this very helpful!
On Sat, May 18, 2024 at 07:00:50PM +0200, Marvin Scholz wrote:
> Given the frequency that new developers, myself included, get the
> code style wrong, it is useful to add some examples to clarify how
> things should be done.
> ---
> doc/developer.texi | 57 +-
> 1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index 63835dfa06..d7bf3f9cb8 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -115,7 +115,7 @@ Objective-C where required for interacting with
> macOS-specific interfaces.
>
> @section Code formatting conventions
>
> -There are the following guidelines regarding the indentation in files:
> +There are the following guidelines regarding the code style in files:
>
> @itemize @bullet
> @item
> @@ -135,6 +135,61 @@ K&R coding style is used.
> @end itemize
> The presentation is one inspired by 'indent -i4 -kr -nut'.
>
> +@subsection Examples
> +Some notable examples to illustrate common code style in FFmpeg:
> +
> +@itemize @bullet
> +
> +@item
> +Spaces around @code{if}/@code{do}/@code{while}/@code{for} conditions and
> assigments:
s/assigments/assignments/
Also, this might be more readable as "Space around assignments and after
@code{if}... keywords"? On first pass, I assumed this was telling me
`( condition )` is correct, then had to re-read when the example showed
it wasn't.
> +
> +@example c
> +if (condition)
`condition` here differs from `cond` below, despite conveying the same meaning.
Either word is fine so long as it's the same word in both places.
> +av_foo();
> +@end example
> +
> +@example c
> +for (size_t i = 0; i < len; i++)
This lightly implies we prefer `i < len` over `i != len` and `i++` over `++i`.
Is that something people round here have strong opinions about? Maybe iterate
over a linked list if this is a controversial question?
> +av_bar(i);
> +@end example
> +
> +@example c
> +size_t size = 0;
> +@end example
> +
> +However no spaces between the parentheses and condition, unless it helps
> +readability of complex conditions, so the following should not be done:
> +
> +@example c
> +// Wrong:
Nitpick: if you're going to say "// Wrong" here, it might be better to introduce
the mechanism with some "// Good"s or something above. The consistency reduces
cognitive load on the learner, and it's a good excuse to add a little positivity
to a nerve-wracking experience.
> +if ( cond )
> +av_foo();
> +@end example
> +
> +@item
> +No unnecessary parentheses, unless it helps readability:
> +
> +@example c
> +flags = s->mb_x ? RIGHT_EDGE : LEFT_EDGE | RIGHT_EDGE;
> +@end example
Can the example use "+" or "*" instead of "|"? I've had so many bugs where
I got the precedence wrong, I'm not sure whether this is supposed to be a good
or bad example of readability.
> +
> +@item
> +No braces around single-line blocks:
> +
> +@example c
> +if (bits_pixel == 24)
> +avctx->pix_fmt = AV_PIX_FMT_BGR24;
> +else if (bits_pixel == 8)
> +avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> +else @{
> +av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> +return AVERROR_INVALIDDATA;
> +@}
> +@end example
> +
> +@end itemize
> +
> +
> @subsection Vim configuration
> In order to configure Vim to follow FFmpeg formatting conventions, paste
> the following snippet into your @file{.vimrc}:
Some other things that could help (in decreasing order of importance)...
* if you find a piece of code that looks wrong, should you...
a) ignore the guide and match your style to the surroundings?
b) follow the guide and accept the file will look inconsistent?
c) add an extra patch to fix the formatting?
(I suspect the answer is (b), but could well be wrong)
* example of brace style for both functions and structs
(as a newbie you don't know if you're about to meet one of those people
who get all bent out of shape when they see a bracket on a line on its own
)
* prefer `foo=bar; if (foo)` over `if ((foo=bar))`
(the latter is sadly used in the code, but is a speedbump for reviewers)
* `foo *bar`, not `foo* bar`
(I always forget this, not important if it's just me)
Also, way outside the scope of this patch, but a linter that checks these things
would be very much appreciated. There's a lot to get wrong with your first
patch,
and a program that just said "yep that's formatted correctly" might save a
newbie
enough time to learn git-send-email instead.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] STF 2025
On Fri, May 17, 2024 at 03:49:58PM +0200, Michael Niedermayer wrote: > Hi all > > Before this is forgotten again, better start some dicsussion too early than > too late > > I propose that if we have the oppertunity again next year to receive a grant > from STF. That we use it to fund: > > * Paul to work on FFmpeg full time. My idea here is that he can work on > whatever > he likes in FFmpeg (so its not full time employment for specific work but > simply full time employment for him to work on whatever he likes in FFmpeg > any > way he likes) Paul is the 2nd largest contributor to FFmpeg (git shortlog > -s -n) Instead of one person creating code five days a week, how about paying five people to review code one day a week each? As well as being less divisive among maintainers, a public list of people who are obliged to do reviews would make us peripheral developers feel less like we're shouting into a void. ___ 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/6] lavf/tls_mbedtls: handle more error codes for human-readable message
On Fri, May 17, 2024 at 10:34:26AM +0200, Sfan5 wrote: > Signed-off-by: sfan5 > --- > libavformat/tls_mbedtls.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/libavformat/tls_mbedtls.c b/libavformat/tls_mbedtls.c > index 1a182e735e..fd6ba0b1f5 100644 > --- a/libavformat/tls_mbedtls.c > +++ b/libavformat/tls_mbedtls.c > @@ -138,6 +138,9 @@ static void handle_handshake_error(URLContext *h, int > ret) > case MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE: > av_log(h, AV_LOG_ERROR, "TLS handshake failed.\n"); > break; > +case MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION: > +av_log(h, AV_LOG_ERROR, "TLS protocol version mismatches.\n"); "... mismatch" or "... does not match" would be more readable than "mismatches". The word "matches" can mean either "does match" or "plural of match". It's technically valid to use "mismatches" to mean "does not match", but in practice the word is only ever used to mean "plural of mismatch". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Value analysis with Frama-C's Eva plugin (and an UB fix)
On Thu, May 16, 2024 at 02:31:31PM +0200, Tomas Härdin wrote: > tor 2024-05-16 klockan 13:12 +0100 skrev Andrew Sayers: > > On Wed, May 15, 2024 at 09:39:43PM +0200, Tomas Härdin wrote: > > > Hi > > > > > > So as I said in the coverity thread it would be good if we could > > > get at > > > least part of the codebase covered using formal tools. To this > > > effect I > > > sat down for an hour just now and gave libavutil/common.h a go with > > > Frama-C's Eva plugin [1;2]. This plugin performs value analysis, > > > which > > > is a much simpler analysis compared to say the weakest predicate > > > (WP) > > > plugin. > > > > > > Going through the functions from top to bottom it only took until > > > av_clipl_int32_c() to find my first UB, a patch for which is > > > attached. > > > Thus my harping on this has born at least some fruit. > > > > > > To run the analysis implemented in this set of patches (all of > > > which > > > I've attached here because I don't want to bother writing six > > > follow-up > > > email), first install frama-c using opam. I'm using 28.0~beta > > > (Nickel). > > > Then run "make verify" in libavutil/ and Eva should tell you that > > > 33% > > > of functions are covered and 100% of statements in those functions > > > are > > > covered, with zero alarms. > > > > > > If the project isn't interested in this then I'll probably continue > > > fiddling with it on my own mostly as exercise. But I suspect it > > > will > > > bear even more fruit in time. > > > > > > /Tomas > > > > > > [1] https://frama-c.com/ > > > [2] https://frama-c.com/fc-plugins/eva.html > > > > I'm all for automated checks, but in my experience they're only > > worthwhile > > if two conditions are met: > > > > * they run automatically on a regular basis > > They could easily be incorporated into FATE or a post-commit hook FATE is a good idea, but post-commit hooks break some workflows. For example, I like to start a test in one window, then put together a commit in another window. I can always amend the commit if there's a problem. The documentation suggests there are some hooks around e-mailing[1], but I haven't tried them. > > > * their output doesn't get boring > > The output of Frama-C in general tends to be quite chatty. I've asked a > couple of time for them to add exit codes, for example returning with > zero only if there are no alarms and no unproven proof obligations. > With Eva grepping for " 0 alarms generated by the analysis." is one > way, but that's also quite ugly Yeah, IMHO the refusal to listen to such reasonable requests is the standard way for these projects to sabotage themselves. Diffing against the previous run tends to work a little better than grepping for a magic word, but still ugly, and you end up having to get rid of line numbers with `sed` or something. I find it's easier to put up with such hacks by constantly reminding myself: No code solution can ever be as ugly as having to do it all by hand. [1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks#_email_hooks ___ 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/opt: Say more often that AV_OPT_SEARCH_CHILDREN searches children first
This behaviour is already mentioned in the documentation for AV_OPT_SEARCH_CHILDREN itself, but that's quite easy to miss. Knowing that child options *override* parent ones is useful for users, so it's worth mentioning in all the places they would look. av_opt_find() had a note that av_opt_find2() was missing. Assume that wasn't deliberate, and copy it over. --- libavutil/opt.h | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/libavutil/opt.h b/libavutil/opt.h index 07e27a9208..24b807ec66 100644 --- a/libavutil/opt.h +++ b/libavutil/opt.h @@ -211,7 +211,7 @@ * * The situation is more complicated with nesting. An AVOptions-enabled struct * may have AVOptions-enabled children. Passing the AV_OPT_SEARCH_CHILDREN flag - * to av_opt_find() will make the function search children recursively. + * to av_opt_find() will make the function recursively search children first. * * For enumerating there are basically two cases. The first is when you want to * get all options that may potentially exist on the struct and its children @@ -570,10 +570,11 @@ const AVClass *av_opt_child_class_iterate(const AVClass *parent, void **iter); * @return A pointer to the option found, or NULL if no option * was found. * - * @note Options found with AV_OPT_SEARCH_CHILDREN flag may not be settable - * directly with av_opt_set(). Use special calls which take an options - * AVDictionary (e.g. avformat_open_input()) to set options found with this - * flag. + * @note If AV_OPT_SEARCH_CHILDREN is set, this function will return an + * option from the first matching child class, or the specified class if + * no child matches. Options from child classes may not be settable directly + * with av_opt_set(), so use special calls which take an options AVDictionary + * (e.g. avformat_open_input()) to set options found with this flag. */ const AVOption *av_opt_find(void *obj, const char *name, const char *unit, int opt_flags, int search_flags); @@ -598,6 +599,12 @@ const AVOption *av_opt_find(void *obj, const char *name, const char *unit, * * @return A pointer to the option found, or NULL if no option * was found. + * + * @note If AV_OPT_SEARCH_CHILDREN is set, this function will return an + * option from the first matching child class, or the specified class if + * no child matches. Options from child classes may not be settable directly + * with av_opt_set(), so use special calls which take an options AVDictionary + * (e.g. avformat_open_input()) to set options found with this flag. */ const AVOption *av_opt_find2(void *obj, const char *name, const char *unit, int opt_flags, int search_flags, void **target_obj); @@ -780,7 +787,8 @@ int av_opt_copy(void *dest, const void *src); * key=value parameters. Values containing ':' special characters must be * escaped. * @param search_flags flags passed to av_opt_find2. I.e. if AV_OPT_SEARCH_CHILDREN - * is passed here, then the option may be set on a child of obj. + * is passed here, av_opt_set() will set the first matching child if possible, + * or obj if no child matches. * * @return 0 if the value has been set, or an AVERROR code in case of * error: -- 2.43.0 ___ 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] all: s/Open/Allocate and initialize/ in comments
Comments for a few prominent functions claim to "open" something, when they actually "allocate and initialize" that thing. Using a different word misleads users into thinking it's doing a different thing, making the interface more time-consuming to learn. Replace "open" with the more standard "allocate and initialize". --- libavformat/avformat.h | 4 ++-- libavformat/avio.h | 4 ++-- libavutil/hwcontext.h | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 8afdcd9fd0..2fb89807a1 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -2235,8 +2235,8 @@ int av_probe_input_buffer(AVIOContext *pb, const AVInputFormat **fmt, unsigned int offset, unsigned int max_probe_size); /** - * Open an input stream and read the header. The codecs are not opened. - * The stream must be closed with avformat_close_input(). + * Allocate and initialize an input stream, then read the header. + * The codecs are not opened. The stream must be closed with avformat_close_input(). * * @param ps Pointer to user-supplied AVFormatContext (allocated by * avformat_alloc_context). May be a pointer to NULL, in diff --git a/libavformat/avio.h b/libavformat/avio.h index ebf611187d..1f56c58f9a 100644 --- a/libavformat/avio.h +++ b/libavformat/avio.h @@ -330,7 +330,7 @@ const char *avio_find_protocol_name(const char *url); int avio_check(const char *url, int flags); /** - * Open directory for reading. + * Allocate and initialize a directory for reading. * * @param s directory read context. Pointer to a NULL pointer must be passed. * @param url directory to be listed. @@ -707,7 +707,7 @@ int avio_closep(AVIOContext **s); /** - * Open a write only memory stream. + * Allocate and initialize a write only memory stream. * * @param s new IO context * @return zero if no error. diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h index bac30debae..be03f565f2 100644 --- a/libavutil/hwcontext.h +++ b/libavutil/hwcontext.h @@ -264,7 +264,8 @@ AVBufferRef *av_hwdevice_ctx_alloc(enum AVHWDeviceType type); int av_hwdevice_ctx_init(AVBufferRef *ref); /** - * Open a device of the specified type and create an AVHWDeviceContext for it. + * Allocate and initialize a device of the specified type, + * and create an AVHWDeviceContext for it. * * This is a convenience function intended to cover the simple cases. Callers * who need to fine-tune device creation/management should open the device -- 2.43.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Value analysis with Frama-C's Eva plugin (and an UB fix)
On Wed, May 15, 2024 at 09:39:43PM +0200, Tomas Härdin wrote: > Hi > > So as I said in the coverity thread it would be good if we could get at > least part of the codebase covered using formal tools. To this effect I > sat down for an hour just now and gave libavutil/common.h a go with > Frama-C's Eva plugin [1;2]. This plugin performs value analysis, which > is a much simpler analysis compared to say the weakest predicate (WP) > plugin. > > Going through the functions from top to bottom it only took until > av_clipl_int32_c() to find my first UB, a patch for which is attached. > Thus my harping on this has born at least some fruit. > > To run the analysis implemented in this set of patches (all of which > I've attached here because I don't want to bother writing six follow-up > email), first install frama-c using opam. I'm using 28.0~beta (Nickel). > Then run "make verify" in libavutil/ and Eva should tell you that 33% > of functions are covered and 100% of statements in those functions are > covered, with zero alarms. > > If the project isn't interested in this then I'll probably continue > fiddling with it on my own mostly as exercise. But I suspect it will > bear even more fruit in time. > > /Tomas > > [1] https://frama-c.com/ > [2] https://frama-c.com/fc-plugins/eva.html I'm all for automated checks, but in my experience they're only worthwhile if two conditions are met: * they run automatically on a regular basis * their output doesn't get boring One simple way to meet both criteria would be a cron job that runs overnight, and messages the ML with just the issues that didn't exist in yesterday's run. Plenty of other ways to do it, but something like that would be a great start. ___ 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 v5 6/6] avformat/avformat: Document return codes for av_format_(de)init
--- libavformat/avformat.h | 4 1 file changed, 4 insertions(+) diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 8afdcd9fd0..f624fb1e2e 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1944,12 +1944,16 @@ const char *avformat_license(void); * This function will be deprecated once support for older GnuTLS and * OpenSSL libraries is removed, and this function has no purpose * anymore. + * + * @return 0 for success or AVERROR code */ int avformat_network_init(void); /** * Undo the initialization done by avformat_network_init. Call it only * once for each time you called avformat_network_init. + * + * @return 0 for success or AVERROR code */ int avformat_network_deinit(void); -- 2.43.0 ___ 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 v5 5/6] avformat/network: Return 0/AVERROR from ff_network_init()
---
libavformat/avio.c| 7 +--
libavformat/network.c | 7 +++
libavformat/rtsp.c| 14 --
libavformat/rtspdec.c | 5 +++--
libavformat/sapdec.c | 5 +++--
libavformat/sapenc.c | 5 +++--
6 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/libavformat/avio.c b/libavformat/avio.c
index d109f3adff..8c94bfeb14 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -123,8 +123,11 @@ static int url_alloc_for_protocol(URLContext **puc, const
URLProtocol *up,
int err;
#if CONFIG_NETWORK
-if (up->flags & URL_PROTOCOL_FLAG_NETWORK && !ff_network_init())
-return AVERROR(EIO);
+if (up->flags & URL_PROTOCOL_FLAG_NETWORK) {
+err = ff_network_init();
+if (err<0)
+return err;
+}
#endif
if ((flags & AVIO_FLAG_READ) && !up->url_read) {
av_log(NULL, AV_LOG_ERROR,
diff --git a/libavformat/network.c b/libavformat/network.c
index 351dc34bb6..643294efe4 100644
--- a/libavformat/network.c
+++ b/libavformat/network.c
@@ -59,11 +59,10 @@ int ff_network_init(void)
{
#if HAVE_WINSOCK2_H
WSADATA wsaData;
-
-if (WSAStartup(MAKEWORD(1,1), &wsaData))
-return 0;
+return ff_neterror2(WSAStartup(MAKEWORD(1,1), &wsaData));
+#else
+return 0;
#endif
-return 1;
}
int ff_network_wait_fd(int fd, int write)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index b0c61ee00a..d50d0b7fc0 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1740,8 +1740,9 @@ int ff_rtsp_connect(AVFormatContext *s)
return AVERROR(EINVAL);
}
-if (!ff_network_init())
-return AVERROR(EIO);
+err = ff_network_init();
+if (err<0)
+return err;
if (s->max_delay < 0) /* Not set by the caller */
s->max_delay = s->iformat ? DEFAULT_REORDERING_DELAY : 0;
@@ -2395,8 +2396,9 @@ static int sdp_read_header(AVFormatContext *s)
char url[MAX_URL_SIZE];
AVBPrint bp;
-if (!ff_network_init())
-return AVERROR(EIO);
+err = ff_network_init();
+if (err<0)
+return err;
if (s->max_delay < 0) /* Not set by the caller */
s->max_delay = DEFAULT_REORDERING_DELAY;
@@ -2522,8 +2524,8 @@ static int rtp_read_header(AVFormatContext *s)
AVBPrint sdp;
AVDictionary *opts = NULL;
-if (!ff_network_init())
-return AVERROR(EIO);
+if ((ret = ff_network_init())<0)
+return ret;
opts = map_to_opts(rt);
ret = ffurl_open_whitelist(&in, s->url, AVIO_FLAG_READ,
diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
index 10078ce2fa..1b4b478170 100644
--- a/libavformat/rtspdec.c
+++ b/libavformat/rtspdec.c
@@ -663,8 +663,9 @@ static int rtsp_listen(AVFormatContext *s)
int ret;
enum RTSPMethod methodcode;
-if (!ff_network_init())
-return AVERROR(EIO);
+ret = ff_network_init();
+if (ret<0)
+return ret;
/* extract hostname and port */
av_url_split(proto, sizeof(proto), auth, sizeof(auth), host, sizeof(host),
diff --git a/libavformat/sapdec.c b/libavformat/sapdec.c
index 357c0dd514..393e544556 100644
--- a/libavformat/sapdec.c
+++ b/libavformat/sapdec.c
@@ -70,8 +70,9 @@ static int sap_read_header(AVFormatContext *s)
int port;
int ret, i;
-if (!ff_network_init())
-return AVERROR(EIO);
+ret = ff_network_init();
+if (ret<0)
+return ret;
av_url_split(NULL, 0, NULL, 0, host, sizeof(host), &port,
path, sizeof(path), s->url);
diff --git a/libavformat/sapenc.c b/libavformat/sapenc.c
index 87a834a8d8..5760e3a0c2 100644
--- a/libavformat/sapenc.c
+++ b/libavformat/sapenc.c
@@ -80,8 +80,9 @@ static int sap_write_header(AVFormatContext *s)
int udp_fd;
AVDictionaryEntry* title = av_dict_get(s->metadata, "title", NULL, 0);
-if (!ff_network_init())
-return AVERROR(EIO);
+ret = ff_network_init();
+if (ret<0)
+return ret;
/* extract hostname and port */
av_url_split(NULL, 0, NULL, 0, host, sizeof(host), &base_port,
--
2.43.0
___
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 v5 4/6] avformat/network: add ff_neterror2() for compatibility with Windows
This is not currently used anywhere, but included to avoid potential future surprises. --- libavformat/network.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/libavformat/network.h b/libavformat/network.h index 1ac067f09f..7c8f81a050 100644 --- a/libavformat/network.h +++ b/libavformat/network.h @@ -82,6 +82,12 @@ int ff_neterror2(int err); * @note Error is based on WSAGetLastError() (Windows) or `errno` (otherwise) */ #define ff_neterror() AVERROR(errno) +/* + * @brief ff_neterror()-style AVERROR + * @param err error code (usually an errno or Windows Sockets Error Code) + * @note Windows Sockets Error Codes are only supported in Windows + */ +#define ff_neterror2(ERRNO) AVERROR(ERRNO) #endif /* HAVE_WINSOCK2_H */ #if HAVE_ARPA_INET_H -- 2.43.0 ___ 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 v5 3/6] avformat/network: add ff_neterror2() for cases that don't use WSAGetLastError
For example, WSAStartup()'s documentation says:
"A call to the WSAGetLastError function is not needed and should not be
used"
---
libavformat/network.c | 5 -
libavformat/network.h | 6 ++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/libavformat/network.c b/libavformat/network.c
index 5d0d05c5f1..351dc34bb6 100644
--- a/libavformat/network.c
+++ b/libavformat/network.c
@@ -123,7 +123,10 @@ void ff_network_close(void)
#if HAVE_WINSOCK2_H
int ff_neterror(void)
{
-int err = WSAGetLastError();
+return ff_neterror2(WSAGetLastError());
+}
+int ff_neterror2(int err)
+{
switch (err) {
case WSAEWOULDBLOCK:
return AVERROR(EAGAIN);
diff --git a/libavformat/network.h b/libavformat/network.h
index f338694212..1ac067f09f 100644
--- a/libavformat/network.h
+++ b/libavformat/network.h
@@ -63,6 +63,12 @@
* @note Error is based on WSAGetLastError() (Windows) or `errno` (otherwise)
*/
int ff_neterror(void);
+/*
+ * @brief ff_neterror()-style AVERROR
+ * @param err error code (usually an errno or Windows Sockets Error Code)
+ * @note Windows Sockets Error Codes are only supported in Windows
+ */
+int ff_neterror2(int err);
#else
#include
#include
--
2.43.0
___
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 v5 2/6] Rename "ff_neterrno()" to "ff_neterror()"
This function does not check errno on Windows, so the old name was misleading.
Actual command:
sed -i -e 's/ff_neterrno/ff_neterror/g' $( git grep -l ff_neterrno )
---
libavformat/network.c | 24
libavformat/network.h | 4 ++--
libavformat/rtpproto.c | 8
libavformat/sctp.c | 10 +-
libavformat/tcp.c | 8
libavformat/udp.c | 32
libavformat/unix.c | 6 +++---
libavformat/url.h | 2 +-
8 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/libavformat/network.c b/libavformat/network.c
index f752efc411..5d0d05c5f1 100644
--- a/libavformat/network.c
+++ b/libavformat/network.c
@@ -72,7 +72,7 @@ int ff_network_wait_fd(int fd, int write)
struct pollfd p = { .fd = fd, .events = ev, .revents = 0 };
int ret;
ret = poll(&p, 1, POLLING_TIME);
-return ret < 0 ? ff_neterrno() : p.revents & (ev | POLLERR | POLLHUP) ? 0
: AVERROR(EAGAIN);
+return ret < 0 ? ff_neterror() : p.revents & (ev | POLLERR | POLLHUP) ? 0
: AVERROR(EAGAIN);
}
int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout,
AVIOInterruptCB *int_cb)
@@ -121,7 +121,7 @@ void ff_network_close(void)
}
#if HAVE_WINSOCK2_H
-int ff_neterrno(void)
+int ff_neterror(void)
{
int err = WSAGetLastError();
switch (err) {
@@ -168,7 +168,7 @@ static int ff_poll_interrupt(struct pollfd *p, nfds_t nfds,
int timeout,
ret = poll(p, nfds, POLLING_TIME);
if (ret != 0) {
if (ret < 0)
-ret = ff_neterrno();
+ret = ff_neterror();
if (ret == AVERROR(EINTR))
continue;
break;
@@ -217,11 +217,11 @@ int ff_listen(int fd, const struct sockaddr *addr,
}
ret = bind(fd, addr, addrlen);
if (ret)
-return ff_neterrno();
+return ff_neterror();
ret = listen(fd, 1);
if (ret)
-return ff_neterrno();
+return ff_neterror();
return ret;
}
@@ -236,7 +236,7 @@ int ff_accept(int fd, int timeout, URLContext *h)
ret = accept(fd, NULL, NULL);
if (ret < 0)
-return ff_neterrno();
+return ff_neterror();
if (ff_socket_nonblock(ret, 1) < 0)
av_log(h, AV_LOG_DEBUG, "ff_socket_nonblock failed\n");
@@ -267,7 +267,7 @@ int ff_listen_connect(int fd, const struct sockaddr *addr,
av_log(h, AV_LOG_DEBUG, "ff_socket_nonblock failed\n");
while ((ret = connect(fd, addr, addrlen))) {
-ret = ff_neterrno();
+ret = ff_neterror();
switch (ret) {
case AVERROR(EINTR):
if (ff_check_interrupt(&h->interrupt_callback))
@@ -280,7 +280,7 @@ int ff_listen_connect(int fd, const struct sockaddr *addr,
return ret;
optlen = sizeof(ret);
if (getsockopt (fd, SOL_SOCKET, SO_ERROR, &ret, &optlen))
-ret = AVUNERROR(ff_neterrno());
+ret = AVUNERROR(ff_neterror());
if (ret != 0) {
char errbuf[100];
ret = AVERROR(ret);
@@ -365,7 +365,7 @@ static int start_connect_attempt(struct ConnectionAttempt
*attempt,
attempt->fd = ff_socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol,
h);
if (attempt->fd < 0)
-return ff_neterrno();
+return ff_neterror();
attempt->deadline_us = av_gettime_relative() + timeout_ms * 1000;
attempt->addr = ai;
@@ -381,7 +381,7 @@ static int start_connect_attempt(struct ConnectionAttempt
*attempt,
}
while ((ret = connect(attempt->fd, ai->ai_addr, ai->ai_addrlen))) {
-ret = ff_neterrno();
+ret = ff_neterror();
switch (ret) {
case AVERROR(EINTR):
if (ff_check_interrupt(&h->interrupt_callback)) {
@@ -478,7 +478,7 @@ int ff_connect_parallel(struct addrinfo *addrs, int
timeout_ms_per_address,
// a successful connection or an error).
optlen = sizeof(last_err);
if (getsockopt(attempts[i].fd, SOL_SOCKET, SO_ERROR,
&last_err, &optlen))
-last_err = ff_neterrno();
+last_err = ff_neterror();
else if (last_err != 0)
last_err = AVERROR(last_err);
if (last_err == 0) {
@@ -587,6 +587,6 @@ int ff_http_match_no_proxy(const char *no_proxy, const char
*hostname)
void ff_log_net_error(void *ctx, int level, const char* prefix)
{
char errbuf[100];
-av_strerror(ff_neterrno(), errbuf, sizeof(errbuf));
+av_strerror(ff_neterror(), errbuf, sizeof(errbuf));
av_log(ctx, level, "%s: %s\n", prefix, errbuf);
}
diff --git a/libavformat/network.h b/libavformat/network.h
index 728c20c9bb..f338694212 100644
--- a/libavformat/network.h
+++ b/libavformat/network.h
@@ -62,7 +62,7 @@
* @return platform-specific AVERROR value
* @note Error is based on WSAGetLastError() (Windows) or `errno` (otherwise)
*/
-int ff_ne
[FFmpeg-devel] [PATCH v5 0/6] avformat/network: improve ff_neterrno()
On Thu, May 16, 2024 at 01:42:23PM +0300, Rémi Denis-Courmont wrote: > Err, please. Keep this to the Windows back-end. Nothing good is going to > happen with a function that does nothing (on other platforms) and has a > nondescript numbered name. I have no strong opinion either way, and it feels rather bikesheddable. Here's a version with the offending part moved to its own patch - I'm happy for whoever applies this to decide whether they want to keep or chuck patch 4/6 :) ___ 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 v5 1/6] Add documentation for ff_neterrno()
--- libavformat/network.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/libavformat/network.h b/libavformat/network.h index ca214087fc..728c20c9bb 100644 --- a/libavformat/network.h +++ b/libavformat/network.h @@ -57,6 +57,11 @@ #define getsockopt(a, b, c, d, e) getsockopt(a, b, c, (char*) d, e) #define setsockopt(a, b, c, d, e) setsockopt(a, b, c, (const char*) d, e) +/* + * @brief AVERROR for the latest network function + * @return platform-specific AVERROR value + * @note Error is based on WSAGetLastError() (Windows) or `errno` (otherwise) + */ int ff_neterrno(void); #else #include @@ -65,6 +70,11 @@ int ff_neterrno(void); #include #include +/* + * @brief AVERROR for the latest network function + * @return platform-specific AVERROR value + * @note Error is based on WSAGetLastError() (Windows) or `errno` (otherwise) + */ #define ff_neterrno() AVERROR(errno) #endif /* HAVE_WINSOCK2_H */ -- 2.43.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 3/4] all: Link to "context" from all contexts with documentation
On Wed, May 15, 2024 at 06:46:19PM +0200, Lynne via ffmpeg-devel wrote: > On 15/05/2024 17:54, Andrew Sayers wrote: > > Some headings needed to be rewritten to accomodate the text, > > (hopefully) without changing the meaning. > > --- > > libavcodec/aac/aacdec.h| 2 +- > > libavcodec/aacenc.h| 2 +- > > libavcodec/ac3enc.h| 2 +- > > libavcodec/amfenc.h| 2 +- > > libavcodec/atrac.h | 2 +- > > libavcodec/avcodec.h | 3 ++- > > libavcodec/bsf.h | 2 +- > > libavcodec/cbs.h | 2 +- > > libavcodec/d3d11va.h | 3 +-- > > libavcodec/h264dsp.h | 2 +- > > libavcodec/h264pred.h | 2 +- > > libavcodec/mediacodec.h| 2 +- > > libavcodec/mpegaudiodec_template.c | 2 +- > > libavcodec/pthread_frame.c | 4 ++-- > > libavcodec/qsv.h | 6 -- > > libavcodec/sbr.h | 2 +- > > libavcodec/smacker.c | 2 +- > > libavcodec/vdpau.h | 3 ++- > > libavcodec/videotoolbox.h | 5 +++-- > > libavfilter/avfilter.h | 2 +- > > libavformat/avformat.h | 3 ++- > > libavformat/avio.h | 3 ++- > > libavutil/audio_fifo.h | 2 +- > > libavutil/hwcontext.h | 21 - > > libavutil/hwcontext_cuda.h | 2 +- > > libavutil/hwcontext_d3d11va.h | 4 ++-- > > libavutil/hwcontext_d3d12va.h | 6 +++--- > > libavutil/hwcontext_drm.h | 2 +- > > libavutil/hwcontext_dxva2.h| 4 ++-- > > libavutil/hwcontext_mediacodec.h | 2 +- > > libavutil/hwcontext_opencl.h | 4 ++-- > > libavutil/hwcontext_qsv.h | 4 ++-- > > libavutil/hwcontext_vaapi.h| 6 +++--- > > libavutil/hwcontext_vdpau.h| 2 +- > > libavutil/hwcontext_vulkan.h | 4 ++-- > > libavutil/lfg.h| 2 +- > > 36 files changed, 66 insertions(+), 57 deletions(-) > > I still don't like this part. There's no reason to link this everywhere when > no one will be bothered to. The document alone is enough IMO. Readers who don't already know the word "context" need a sign that it's a word they need to pay attention to. For example, I come from an OOP background where the word "object" is used so widely, it simply never comes up. In fact, I'm probably not the only person who followed the link to AVClass instead, which just makes FFmpeg look like a failed attempt at OOP if you don't know about contexts. Linking widely lets an attentive reader see this *before* they get the wrong end of the stick, and gives an overwhelmed newbie enough hints that this is a word they need to pay attention to. To underline the previous point - an attentive reader could probably make do with e.g. just links from AVClass and a handful of the most popular contexts. But newbies are often inefficient learners who need many reminders before they stop paying attention to random things. So linking as widely as possible makes the project more accessible to people with less experience. ___ 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 v4 5/5] avformat/avformat: Document return codes for av_format_(de)init
--- libavformat/avformat.h | 4 1 file changed, 4 insertions(+) diff --git a/libavformat/avformat.h b/libavformat/avformat.h index 8afdcd9fd0..f624fb1e2e 100644 --- a/libavformat/avformat.h +++ b/libavformat/avformat.h @@ -1944,12 +1944,16 @@ const char *avformat_license(void); * This function will be deprecated once support for older GnuTLS and * OpenSSL libraries is removed, and this function has no purpose * anymore. + * + * @return 0 for success or AVERROR code */ int avformat_network_init(void); /** * Undo the initialization done by avformat_network_init. Call it only * once for each time you called avformat_network_init. + * + * @return 0 for success or AVERROR code */ int avformat_network_deinit(void); -- 2.43.0 ___ 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 v4 4/5] avformat/network: Return 0/AVERROR from ff_network_init()
---
libavformat/avio.c| 7 +--
libavformat/network.c | 7 +++
libavformat/rtsp.c| 14 --
libavformat/rtspdec.c | 5 +++--
libavformat/sapdec.c | 5 +++--
libavformat/sapenc.c | 5 +++--
6 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/libavformat/avio.c b/libavformat/avio.c
index d109f3adff..8c94bfeb14 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -123,8 +123,11 @@ static int url_alloc_for_protocol(URLContext **puc, const
URLProtocol *up,
int err;
#if CONFIG_NETWORK
-if (up->flags & URL_PROTOCOL_FLAG_NETWORK && !ff_network_init())
-return AVERROR(EIO);
+if (up->flags & URL_PROTOCOL_FLAG_NETWORK) {
+err = ff_network_init();
+if (err<0)
+return err;
+}
#endif
if ((flags & AVIO_FLAG_READ) && !up->url_read) {
av_log(NULL, AV_LOG_ERROR,
diff --git a/libavformat/network.c b/libavformat/network.c
index 351dc34bb6..643294efe4 100644
--- a/libavformat/network.c
+++ b/libavformat/network.c
@@ -59,11 +59,10 @@ int ff_network_init(void)
{
#if HAVE_WINSOCK2_H
WSADATA wsaData;
-
-if (WSAStartup(MAKEWORD(1,1), &wsaData))
-return 0;
+return ff_neterror2(WSAStartup(MAKEWORD(1,1), &wsaData));
+#else
+return 0;
#endif
-return 1;
}
int ff_network_wait_fd(int fd, int write)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index b0c61ee00a..d50d0b7fc0 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1740,8 +1740,9 @@ int ff_rtsp_connect(AVFormatContext *s)
return AVERROR(EINVAL);
}
-if (!ff_network_init())
-return AVERROR(EIO);
+err = ff_network_init();
+if (err<0)
+return err;
if (s->max_delay < 0) /* Not set by the caller */
s->max_delay = s->iformat ? DEFAULT_REORDERING_DELAY : 0;
@@ -2395,8 +2396,9 @@ static int sdp_read_header(AVFormatContext *s)
char url[MAX_URL_SIZE];
AVBPrint bp;
-if (!ff_network_init())
-return AVERROR(EIO);
+err = ff_network_init();
+if (err<0)
+return err;
if (s->max_delay < 0) /* Not set by the caller */
s->max_delay = DEFAULT_REORDERING_DELAY;
@@ -2522,8 +2524,8 @@ static int rtp_read_header(AVFormatContext *s)
AVBPrint sdp;
AVDictionary *opts = NULL;
-if (!ff_network_init())
-return AVERROR(EIO);
+if ((ret = ff_network_init())<0)
+return ret;
opts = map_to_opts(rt);
ret = ffurl_open_whitelist(&in, s->url, AVIO_FLAG_READ,
diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
index 10078ce2fa..1b4b478170 100644
--- a/libavformat/rtspdec.c
+++ b/libavformat/rtspdec.c
@@ -663,8 +663,9 @@ static int rtsp_listen(AVFormatContext *s)
int ret;
enum RTSPMethod methodcode;
-if (!ff_network_init())
-return AVERROR(EIO);
+ret = ff_network_init();
+if (ret<0)
+return ret;
/* extract hostname and port */
av_url_split(proto, sizeof(proto), auth, sizeof(auth), host, sizeof(host),
diff --git a/libavformat/sapdec.c b/libavformat/sapdec.c
index 357c0dd514..393e544556 100644
--- a/libavformat/sapdec.c
+++ b/libavformat/sapdec.c
@@ -70,8 +70,9 @@ static int sap_read_header(AVFormatContext *s)
int port;
int ret, i;
-if (!ff_network_init())
-return AVERROR(EIO);
+ret = ff_network_init();
+if (ret<0)
+return ret;
av_url_split(NULL, 0, NULL, 0, host, sizeof(host), &port,
path, sizeof(path), s->url);
diff --git a/libavformat/sapenc.c b/libavformat/sapenc.c
index 87a834a8d8..5760e3a0c2 100644
--- a/libavformat/sapenc.c
+++ b/libavformat/sapenc.c
@@ -80,8 +80,9 @@ static int sap_write_header(AVFormatContext *s)
int udp_fd;
AVDictionaryEntry* title = av_dict_get(s->metadata, "title", NULL, 0);
-if (!ff_network_init())
-return AVERROR(EIO);
+ret = ff_network_init();
+if (ret<0)
+return ret;
/* extract hostname and port */
av_url_split(NULL, 0, NULL, 0, host, sizeof(host), &base_port,
--
2.43.0
___
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 v4 3/5] avformat/network: add ff_neterror2() for cases where we already have an error
For example, WSAStartup()'s documentation says:
"A call to the WSAGetLastError function is not needed and should not be
used"
---
libavformat/network.c | 5 -
libavformat/network.h | 12
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/libavformat/network.c b/libavformat/network.c
index 5d0d05c5f1..351dc34bb6 100644
--- a/libavformat/network.c
+++ b/libavformat/network.c
@@ -123,7 +123,10 @@ void ff_network_close(void)
#if HAVE_WINSOCK2_H
int ff_neterror(void)
{
-int err = WSAGetLastError();
+return ff_neterror2(WSAGetLastError());
+}
+int ff_neterror2(int err)
+{
switch (err) {
case WSAEWOULDBLOCK:
return AVERROR(EAGAIN);
diff --git a/libavformat/network.h b/libavformat/network.h
index f338694212..7c8f81a050 100644
--- a/libavformat/network.h
+++ b/libavformat/network.h
@@ -63,6 +63,12 @@
* @note Error is based on WSAGetLastError() (Windows) or `errno` (otherwise)
*/
int ff_neterror(void);
+/*
+ * @brief ff_neterror()-style AVERROR
+ * @param err error code (usually an errno or Windows Sockets Error Code)
+ * @note Windows Sockets Error Codes are only supported in Windows
+ */
+int ff_neterror2(int err);
#else
#include
#include
@@ -76,6 +82,12 @@ int ff_neterror(void);
* @note Error is based on WSAGetLastError() (Windows) or `errno` (otherwise)
*/
#define ff_neterror() AVERROR(errno)
+/*
+ * @brief ff_neterror()-style AVERROR
+ * @param err error code (usually an errno or Windows Sockets Error Code)
+ * @note Windows Sockets Error Codes are only supported in Windows
+ */
+#define ff_neterror2(ERRNO) AVERROR(ERRNO)
#endif /* HAVE_WINSOCK2_H */
#if HAVE_ARPA_INET_H
--
2.43.0
___
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".
