[FFmpeg-devel] [PATCH 1/6] doc/developer: move a sentence to a more appropriate place

[FFmpeg-devel] [PATCH 1/6] doc/developer: move a sentence to a more appropriate place

[Anton Khirnov]

It's targeted at our users, not developers, so it makes more sense to
group it with other text targeted at our users.
---
 doc/developer.texi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index 0c2f2cd7d1..85515f5d37 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -24,6 +24,10 @@ generated from the headers
 the examples under @file{doc/examples}
 @end itemize
 
+For more detailed legal information about the use of FFmpeg in
+external programs read the @file{LICENSE} file in the source tree and
+consult @url{https://ffmpeg.org/legal.html}.
+
 If you modify FFmpeg code for your own use case, you are highly encouraged to
 @emph{submit your changes back to us}, using this document as a guide. There are
 both pragmatic and ideological reasons to do so:
@@ -40,10 +44,6 @@ By supporting the project you find useful you ensure it continues to be
 maintained and developed.
 @end itemize
 
-For more detailed legal information about the use of FFmpeg in
-external programs read the @file{LICENSE} file in the source tree and
-consult @url{https://ffmpeg.org/legal.html}.
-
 @section Contributing code
 
 All proposed code changes should be submitted for review to
-- 
2.40.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/6] doc/developer: merge the 'contributing code' section into its parent chapter

[Anton Khirnov]

The section consistes of a single short paragraph linking to other
chapters. The enclosing chapter also has no other sections, all other
text is placed in the chapter directly.
Keeping a separate section for this paragraph just adds more clutter.
---
 doc/developer.texi | 2 --
 1 file changed, 2 deletions(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index 85515f5d37..d27716ab97 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -44,8 +44,6 @@ By supporting the project you find useful you ensure it continues to be
 maintained and developed.
 @end itemize
 
-@section Contributing code
-
 All proposed code changes should be submitted for review to
 @url{mailto:ffmpeg-devel@@ffmpeg.org, the development mailing list}, as
 described in more detail in the @ref{Submitting patches} chapter. The code
-- 
2.40.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/6] doc/developer: fix a nonsense statement

[Anton Khirnov]

Adding new fields to _functions_ makes no sense, it was supposed to be
structs.
---
 doc/developer.texi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index d27716ab97..df43119f98 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -425,7 +425,7 @@ number remains unchanged.
 @subsection Adding new interfaces
 Any new public identifiers in installed headers are considered new API - this
 includes new functions, structs, macros, enum values, typedefs, new fields in
-existing functions, new installed headers, etc. Consider the following
+existing structs, new installed headers, etc. Consider the following
 guidelines when adding new APIs.
 
 @subsubheading Motivation
-- 
2.40.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Anton Khirnov]

Document our longstanding de facto policies on things like correctness,
thread-safety, UB, etc.
---
 doc/developer.texi | 50 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index df43119f98..afa0148137 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -274,10 +274,6 @@ symbols. If in doubt, just avoid names starting with @code{_} altogether.
 @section Miscellaneous conventions
 
 @itemize @bullet
-@item
-fprintf and printf are forbidden in libavformat and libavcodec,
-please use av_log() instead.
-
 @item
 Casts should be used only when necessary. Unneeded parentheses
 should also be avoided if they don't make the code easier to understand.
@@ -286,6 +282,42 @@ should also be avoided if they don't make the code easier to understand.
 @anchor{Development Policy}
 @chapter Development Policy
 
+@section Code behaviour
+
+@subheading Correctness
+The code must be valid. It must not crash, abort, access invalid pointers, leak
+memory, cause data races or signed integer overflow, or otherwise invoke
+undefined behaviour. Error codes should be checked and, when applicable,
+forwarded to the caller.
+
+@subheading Thread- and library-safety
+Our libraries may be called by multiple independent callers in the same process.
+These calls may happen from any number of threads and the different call sites
+may not be aware of each other - e.g. a user program may be calling us directly,
+and use one or more libraries that also call us. The code must behave correctly
+under such conditions.
+
+@subheading Robustness
+The code must treat as untrusted any bytestream received from a caller or read
+from a file, network, etc. It must not misbehave when arbitrary data is sent to
+it - typically it should print an error message and return
+@code{AVERROR_INVALIDDATA} on encountering invalid input data.
+
+@subheading Memory allocation
+The code must use the @code{av_malloc()} family of functions from
+@file{libavutil/mem.h} to perform all memory allocation, except in special cases
+(e.g. when interacting with an external library that requires a specific
+allocator to be used).
+
+All allocations should be checked and @code{AVERROR(ENOMEM)} returned on
+failure. A common mistake is that error paths leak memory - make sure that does
+not happen.
+
+@subheading stdio
+Our libraries must not access the stdio streams stdin/stdout/stderr directly
+(e.g. via @code{printf()} family of functions), as that is not library-safe. For
+logging, use @code{av_log()}.
+
 @section Patches/Committing
 @subheading Licenses for patches must be compatible with FFmpeg.
 Contributions should be licensed under the
@@ -395,11 +427,6 @@ If it is a bug, the bug has to be fixed. If it is not, the code should
 be changed to not generate a warning unless that causes a slowdown
 or obfuscates the code.
 
-@subheading Check untrusted input properly.
-Never write to unallocated memory, never write over the end of arrays,
-always check values read from some untrusted source before using them
-as array index or other risky things.
-
 @section Library public interfaces
 Every library in FFmpeg provides a set of public APIs in its installed headers,
 which are those listed in the variable @code{HEADERS} in that library's
@@ -811,11 +838,6 @@ an explanation why to your patchset, its ok to not test if theres a reason.
 @item
 If you added YASM code please check that things still work with --disable-yasm.
 
-@item
-Make sure you check the return values of function and return appropriate
-error codes. Especially memory allocation functions like @code{av_malloc()}
-are notoriously left unchecked, which is a serious problem.
-
 @item
 Test your code with valgrind and or Address Sanitizer to ensure it's free
 of leaks, out of array accesses, etc.
-- 
2.40.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/6] doc/developer: drop an outdated item

[Anton Khirnov]

It dates back to pre-2005 days, when people generally tended to commit
their work directly without going through the mailing list. Few
developers do it today, and never outside of their standalone modules.
This item is thus confusing and misleading and is better removed.
---
 doc/developer.texi | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index afa0148137..e4ba263581 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -365,15 +365,6 @@ later on.
 Also if you have doubts about splitting or not splitting, do not hesitate to
 ask/discuss it on the developer mailing list.
 
-@subheading Ask before you change the build system (configure, etc).
-Do not commit changes to the build system (Makefiles, configure script)
-which change behavior, defaults etc, without asking first. The same
-applies to compiler warning fixes, trivial looking fixes and to code
-maintained by other developers. We usually have a reason for doing things
-the way we do. Send your changes as patches to the ffmpeg-devel mailing
-list, and if the code maintainers say OK, you may commit. This does not
-apply to files you wrote and/or maintain.
-
 @subheading Cosmetic changes should be kept in separate patches.
 We refuse source indentation and other cosmetic changes if they are mixed
 with functional changes, such commits will be rejected and removed. Every
-- 
2.40.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 6/6] doc/developer: deduplicate commit message rules

[Anton Khirnov]

The patches/committing section currently contains several
partially-overlapping rules on commit messages. Merge and simplify them
into one item.
---
 doc/developer.texi | 41 ++++++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/doc/developer.texi b/doc/developer.texi
index e4ba263581..fa417fc019 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -340,13 +340,24 @@ missing samples or an implementation with a small subset of features.
 Always check the mailing list for any reviewers with issues and test
 FATE before you push.
 
-@subheading Keep the main commit message short with an extended description below.
-The commit message should have a short first line in the form of
-a @samp{topic: short description} as a header, separated by a newline
-from the body consisting of an explanation of why the change is necessary.
-If the commit fixes a known bug on the bug tracker, the commit message
-should include its bug ID. Referring to the issue on the bug tracker does
-not exempt you from writing an excerpt of the bug in the commit message.
+@subheading Commit messages
+Commit messages are highly important tools for informing other developers on
+what a given change does and why. Every commit must always have a properly
+filled out commit message with the following format:
+@example
+area changed: Short 1 line description
+
+details describing what and why and giving references.
+@end example
+
+If the commit addresses a known bug on our bug tracker or other external issue
+(e.g. CVE), the commit message should include the relevant bug ID(s) or other
+external identifiers. Note that this should be done in addition to a proper
+explanation and not instead of it. Comments such as "fixed!" or "Changed it."
+are not acceptable.
+
+When applying patches that have been discussed at length on the mailing list,
+reference the thread in the commit message.
 
 @subheading Testing must be adequate but not excessive.
 If it works for you, others, and passes FATE then it should be OK to commit
@@ -379,28 +390,12 @@ NOTE: If you had to put if()@{ .. @} over a large (> 5 lines) chunk of code,
 then either do NOT change the indentation of the inner part within (do not
 move it to the right)! or do so in a separate commit
 
-@subheading Commit messages should always be filled out properly.
-Always fill out the commit log message. Describe in a few lines what you
-changed and why. You can refer to mailing list postings if you fix a
-particular bug. Comments such as "fixed!" or "Changed it." are unacceptable.
-Recommended format:
-
-@example
-area changed: Short 1 line description
-
-details describing what and why and giving references.
-@end example
-
 @subheading Credit the author of the patch.
 Make sure the author of the commit is set correctly. (see git commit --author)
 If you apply a patch, send an
 answer to ffmpeg-devel (or wherever you got the patch from) saying that
 you applied the patch.
 
-@subheading Complex patches should refer to discussion surrounding them.
-When applying patches that have been discussed (at length) on the mailing
-list, reference the thread in the log message.
-
 @subheading Always wait long enough before pushing changes
 Do NOT commit to code actively maintained by others without permission.
 Send a patch to ffmpeg-devel. If no one answers within a reasonable
-- 
2.40.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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Andreas Rheinhardt]

Anton Khirnov:
> Document our longstanding de facto policies on things like correctness,
> thread-safety, UB, etc.
> ---
>  doc/developer.texi | 50 +++++++++++++++++++++++++++++++++-------------
>  1 file changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/developer.texi b/doc/developer.texi
> index df43119f98..afa0148137 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -274,10 +274,6 @@ symbols. If in doubt, just avoid names starting with @code{_} altogether.
>  @section Miscellaneous conventions
>  
>  @itemize @bullet
> -@item
> -fprintf and printf are forbidden in libavformat and libavcodec,
> -please use av_log() instead.
> -
>  @item
>  Casts should be used only when necessary. Unneeded parentheses
>  should also be avoided if they don't make the code easier to understand.
> @@ -286,6 +282,42 @@ should also be avoided if they don't make the code easier to understand.
>  @anchor{Development Policy}
>  @chapter Development Policy
>  
> +@section Code behaviour
> +
> +@subheading Correctness
> +The code must be valid. It must not crash, abort, access invalid pointers, leak
> +memory, cause data races or signed integer overflow, or otherwise invoke
> +undefined behaviour. Error codes should be checked and, when applicable,
> +forwarded to the caller.
> +
> +@subheading Thread- and library-safety
> +Our libraries may be called by multiple independent callers in the same process.
> +These calls may happen from any number of threads and the different call sites
> +may not be aware of each other - e.g. a user program may be calling us directly,
> +and use one or more libraries that also call us. The code must behave correctly
> +under such conditions.

Some of this can no longer be guaranteed when FFmpeg is built without
threading support, but called from multiple threads concurrently. Should
this be mentioned or would it just confuse readers? (I'm leaning to the
latter.)

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Anton Khirnov]

Quoting Andreas Rheinhardt (2023-08-26 20:36:25)
> Anton Khirnov:
> > Document our longstanding de facto policies on things like correctness,
> > thread-safety, UB, etc.
> > ---
> >  doc/developer.texi | 50 +++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 36 insertions(+), 14 deletions(-)
> > 
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index df43119f98..afa0148137 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -274,10 +274,6 @@ symbols. If in doubt, just avoid names starting with @code{_} altogether.
> >  @section Miscellaneous conventions
> >  
> >  @itemize @bullet
> > -@item
> > -fprintf and printf are forbidden in libavformat and libavcodec,
> > -please use av_log() instead.
> > -
> >  @item
> >  Casts should be used only when necessary. Unneeded parentheses
> >  should also be avoided if they don't make the code easier to understand.
> > @@ -286,6 +282,42 @@ should also be avoided if they don't make the code easier to understand.
> >  @anchor{Development Policy}
> >  @chapter Development Policy
> >  
> > +@section Code behaviour
> > +
> > +@subheading Correctness
> > +The code must be valid. It must not crash, abort, access invalid pointers, leak
> > +memory, cause data races or signed integer overflow, or otherwise invoke
> > +undefined behaviour. Error codes should be checked and, when applicable,
> > +forwarded to the caller.
> > +
> > +@subheading Thread- and library-safety
> > +Our libraries may be called by multiple independent callers in the same process.
> > +These calls may happen from any number of threads and the different call sites
> > +may not be aware of each other - e.g. a user program may be calling us directly,
> > +and use one or more libraries that also call us. The code must behave correctly
> > +under such conditions.
> 
> Some of this can no longer be guaranteed when FFmpeg is built without
> threading support, but called from multiple threads concurrently. Should
> this be mentioned or would it just confuse readers? (I'm leaning to the
> latter.)

I tend to agree, it's a very obscure use case anyway.

But we should probably document more explicitly somewhere (and warn in
configure) what building without threads actually implies.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/6] doc/developer: move a sentence to a more appropriate place

[Stefano Sabatini]

Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:

> It's targeted at our users, not developers, so it makes more sense to
> group it with other text targeted at our users.
> ---
>  doc/developer.texi | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index 0c2f2cd7d1..85515f5d37 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -24,6 +24,10 @@ generated from the headers
>  the examples under @file{doc/examples}
>  @end itemize
>
> +For more detailed legal information about the use of FFmpeg in
> +external programs read the @file{LICENSE} file in the source tree and
> +consult @url{https://ffmpeg.org/legal.html}.
> +
>  If you modify FFmpeg code for your own use case, you are highly
> encouraged to
>  @emph{submit your changes back to us}, using this document as a guide.
> There are
>  both pragmatic and ideological reasons to do so:
> @@ -40,10 +44,6 @@ By supporting the project you find useful you ensure it
> continues to be
>  maintained and developed.
>  @end itemize
>
> -For more detailed legal information about the use of FFmpeg in
> -external programs read the @file{LICENSE} file in the source tree and
> -consult @url{https://ffmpeg.org/legal.html}.
> -
>  @section Contributing code


Lgtm, thanks.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] doc/developer: merge the 'contributing code' section into its parent chapter

[Stefano Sabatini]

Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:

> The section consistes of a single short paragraph linking to other
> chapters. The enclosing chapter also has no other sections, all other
> text is placed in the chapter directly.
> Keeping a separate section for this paragraph just adds more clutter.
> ---
>  doc/developer.texi | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index 85515f5d37..d27716ab97 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -44,8 +44,6 @@ By supporting the project you find useful you ensure it
> continues to be
>  maintained and developed.
>  @end itemize
>
> -@section Contributing code
> -
>  All proposed code changes should be submitted for review to
>  @url{mailto:ffmpeg-devel@@ffmpeg.org, the development mailing list}, as
>  described in more detail in the @ref{Submitting patches} chapter. The code
>

Lgtm, thanks.

>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/6] doc/developer: fix a nonsense statement

[Stefano Sabatini]

Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:

> Adding new fields to _functions_ makes no sense, it was supposed to be
> structs.
> ---
>  doc/developer.texi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index d27716ab97..df43119f98 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -425,7 +425,7 @@ number remains unchanged.
>  @subsection Adding new interfaces
>  Any new public identifiers in installed headers are considered new API -
> this
>  includes new functions, structs, macros, enum values, typedefs, new
> fields in
> -existing functions, new installed headers, etc. Consider the following
> +existing structs, new installed headers, etc. Consider the following
>  guidelines when adding new APIs.


Obviously ok, 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Stefano Sabatini]

Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:

> Document our longstanding de facto policies on things like correctness,
> thread-safety, UB, etc.
>

UB?

---
>  doc/developer.texi | 50 +++++++++++++++++++++++++++++++++-------------
>  1 file changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index df43119f98..afa0148137 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -274,10 +274,6 @@ symbols. If in doubt, just avoid names starting with
> @code{_} altogether.
>  @section Miscellaneous conventions
>
>  @itemize @bullet
> -@item
> -fprintf and printf are forbidden in libavformat and libavcodec,
> -please use av_log() instead.
> -
>  @item
>  Casts should be used only when necessary. Unneeded parentheses
>  should also be avoided if they don't make the code easier to understand.
> @@ -286,6 +282,42 @@ should also be avoided if they don't make the code
> easier to understand.
>  @anchor{Development Policy}
>  @chapter Development Policy
>
> +@section Code behaviour
> +
> +@subheading Correctness
> +The code must be valid. It must not crash, abort, access invalid
> pointers, leak
> +memory, cause data races or signed integer overflow, or otherwise invoke
> +undefined behaviour.


Invoke => assume?

Error codes should be checked and, when applicable,
> +forwarded to the caller.
> +
> +@subheading Thread- and library-safety
> +Our libraries may be called by multiple independent callers in the same
> process.
> +These calls may happen from any number of threads and the different call
> sites
> +may not be aware of each other - e.g. a user program may be calling us
> directly,
>

Calling us -> calling the/our libraries

+and use one or more libraries that also call us. The code must behave
> correctly
> +under such conditions.
> +
> +@subheading Robustness
> +The code must treat as untrusted any bytestream received from a caller or
> read
> +from a file, network, etc. It must not misbehave when arbitrary data is
> sent to
> +it - typically it should print an error message and return
> +@code{AVERROR_INVALIDDATA} on encountering invalid input data.
> +
> +@subheading Memory allocation
> +The code must use the @code{av_malloc()} family of functions from
> +@file{libavutil/mem.h} to perform all memory allocation, except in
> special cases
> +(e.g. when interacting with an external library that requires a specific
> +allocator to be used).
> +
> +All allocations should be checked and @code{AVERROR(ENOMEM)} returned on
> +failure. A common mistake is that error paths leak memory - make sure
> that does
> +not happen.
> +
> +@subheading stdio
> +Our libraries must not access the stdio streams stdin/stdout/stderr
> directly
> +(e.g. via @code{printf()} family of functions), as that is not
> library-safe. For
> +logging, use @code{av_log()}.


Lgtm otherwise, 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/6] doc/developer: drop an outdated item

[Stefano Sabatini]

Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:

> It dates back to pre-2005 days, when people generally tended to commit
> their work directly without going through the mailing list. Few
> developers do it today, and never outside of their standalone modules.
> This item is thus confusing and misleading and is better removed.
> ---
>  doc/developer.texi | 9 ---------
>  1 file changed, 9 deletions(-)
>

Agree, lgtm.

>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 6/6] doc/developer: deduplicate commit message rules

[Stefano Sabatini]

Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:

> The patches/committing section currently contains several
> partially-overlapping rules on commit messages. Merge and simplify them
> into one item.
> ---
>  doc/developer.texi | 41 ++++++++++++++++++-----------------------
>  1 file changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/doc/developer.texi b/doc/developer.texi
> index e4ba263581..fa417fc019 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -340,13 +340,24 @@ missing samples or an implementation with a small
> subset of features.
>  Always check the mailing list for any reviewers with issues and test
>  FATE before you push.
>
> -@subheading Keep the main commit message short with an extended
> description below.
> -The commit message should have a short first line in the form of
> -a @samp{topic: short description} as a header, separated by a newline
> -from the body consisting of an explanation of why the change is necessary.
> -If the commit fixes a known bug on the bug tracker, the commit message
> -should include its bug ID. Referring to the issue on the bug tracker does
> -not exempt you from writing an excerpt of the bug in the commit message.
> +@subheading Commit messages
> +Commit messages are highly important tools for informing other developers
> on
> +what a given change does and why. Every commit must always have a properly
> +filled out commit message with the following format:
> +@example
> +area changed: Short 1 line description


Short => short given that we do not Capitalize

+
> +details describing what and why and giving references.
> +@end example
> +
> +If the commit addresses a known bug on our bug tracker or other external
> issue
> +(e.g. CVE), the commit message should include the relevant bug ID(s) or
> other
> +external identifiers. Note that this should be done in addition to a
> proper
> +explanation and not instead of it. Comments such as "fixed!" or "Changed
> it."
> +are not acceptable.
> +
> +When applying patches that have been discussed at length on the mailing
> list,
> +reference the thread in the commit message.
>
>  @subheading Testing must be adequate but not excessive.
>  If it works for you, others, and passes FATE then it should be OK to
> commit
> @@ -379,28 +390,12 @@ NOTE: If you had to put if()@{ .. @} over a large (>
> 5 lines) chunk of code,
>  then either do NOT change the indentation of the inner part within (do not
>  move it to the right)! or do so in a separate commit
>
> -@subheading Commit messages should always be filled out properly.
> -Always fill out the commit log message. Describe in a few lines what you
> -changed and why. You can refer to mailing list postings if you fix a
> -particular bug. Comments such as "fixed!" or "Changed it." are
> unacceptable.
> -Recommended format:
> -
> -@example
> -area changed: Short 1 line description
> -
> -details describing what and why and giving references.
> -@end example
> -
>  @subheading Credit the author of the patch.
>  Make sure the author of the commit is set correctly. (see git commit
> --author)
>  If you apply a patch, send an
>  answer to ffmpeg-devel (or wherever you got the patch from) saying that
>  you applied the patch.
>
> -@subheading Complex patches should refer to discussion surrounding them.
> -When applying patches that have been discussed (at length) on the mailing
> -list, reference the thread in the log message.
> -
>  @subheading Always wait long enough before pushing changes
>  Do NOT commit to code actively maintained by others without permission.
>  Send a patch to ffmpeg-devel. If no one answers within a reasonable
>

Lgtm otherwise, 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Anton Khirnov]

Quoting Stefano Sabatini (2023-08-27 14:38:44)
> Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:
> 
> > Document our longstanding de facto policies on things like correctness,
> > thread-safety, UB, etc.
> >
> 
> UB?

undefined behavior

> 
> ---
> >  doc/developer.texi | 50 +++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 36 insertions(+), 14 deletions(-)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index df43119f98..afa0148137 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -274,10 +274,6 @@ symbols. If in doubt, just avoid names starting with
> > @code{_} altogether.
> >  @section Miscellaneous conventions
> >
> >  @itemize @bullet
> > -@item
> > -fprintf and printf are forbidden in libavformat and libavcodec,
> > -please use av_log() instead.
> > -
> >  @item
> >  Casts should be used only when necessary. Unneeded parentheses
> >  should also be avoided if they don't make the code easier to understand.
> > @@ -286,6 +282,42 @@ should also be avoided if they don't make the code
> > easier to understand.
> >  @anchor{Development Policy}
> >  @chapter Development Policy
> >
> > +@section Code behaviour
> > +
> > +@subheading Correctness
> > +The code must be valid. It must not crash, abort, access invalid
> > pointers, leak
> > +memory, cause data races or signed integer overflow, or otherwise invoke
> > +undefined behaviour.
> 
> 
> Invoke => assume?

No, 'assume' would mean something else. 'cause' maybe.

> 
> Error codes should be checked and, when applicable,
> > +forwarded to the caller.
> > +
> > +@subheading Thread- and library-safety
> > +Our libraries may be called by multiple independent callers in the same
> > process.
> > +These calls may happen from any number of threads and the different call
> > sites
> > +may not be aware of each other - e.g. a user program may be calling us
> > directly,
> >
> 
> Calling us -> calling the/our libraries

I wanted to avoid using the word 'libraries' multiple times, to avoid
possible confusion of which library is being talked about - whether ours
or some external one.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Stefano Sabatini]

On date Tuesday 2023-08-29 10:34:45 +0200, Anton Khirnov wrote:
> Quoting Stefano Sabatini (2023-08-27 14:38:44)
> > Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:
> > 
> > > Document our longstanding de facto policies on things like correctness,
> > > thread-safety, UB, etc.
> > >
> > 
> > UB?
> 
> undefined behavior

yes, let's avoid acronyms

> 
> > 
> > ---
> > >  doc/developer.texi | 50 +++++++++++++++++++++++++++++++++-------------
> > >  1 file changed, 36 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/doc/developer.texi b/doc/developer.texi
> > > index df43119f98..afa0148137 100644
> > > --- a/doc/developer.texi
> > > +++ b/doc/developer.texi
> > > @@ -274,10 +274,6 @@ symbols. If in doubt, just avoid names starting with
> > > @code{_} altogether.
> > >  @section Miscellaneous conventions
> > >
> > >  @itemize @bullet
> > > -@item
> > > -fprintf and printf are forbidden in libavformat and libavcodec,
> > > -please use av_log() instead.
> > > -
> > >  @item
> > >  Casts should be used only when necessary. Unneeded parentheses
> > >  should also be avoided if they don't make the code easier to understand.
> > > @@ -286,6 +282,42 @@ should also be avoided if they don't make the code
> > > easier to understand.
> > >  @anchor{Development Policy}
> > >  @chapter Development Policy
> > >
> > > +@section Code behaviour
> > > +
> > > +@subheading Correctness
> > > +The code must be valid. It must not crash, abort, access invalid
> > > pointers, leak
> > > +memory, cause data races or signed integer overflow, or otherwise invoke
> > > +undefined behaviour.
> > 
> > 
> > Invoke => assume?
> 

> No, 'assume' would mean something else. 'cause' maybe.

"cause" sounds good.

> 
> > 
> > Error codes should be checked and, when applicable,
> > > +forwarded to the caller.
> > > +
> > > +@subheading Thread- and library-safety
> > > +Our libraries may be called by multiple independent callers in the same
> > > process.
> > > +These calls may happen from any number of threads and the different call
> > > sites
> > > +may not be aware of each other - e.g. a user program may be calling us
> > > directly,
> > >
> > 
> > Calling us -> calling the/our libraries
> 

> I wanted to avoid using the word 'libraries' multiple times, to avoid
> possible confusion of which library is being talked about - whether ours
> or some external one.

But using "us" is even more confusing (who/what is "us"?), I have no
better ideas than repeating "our/the FFmpeg libraries".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 885 bytes --]

On Thu, Aug 31, 2023 at 05:28:48PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2023-08-29 10:34:45 +0200, Anton Khirnov wrote:
> > Quoting Stefano Sabatini (2023-08-27 14:38:44)
> > > Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:
> > > 
> > > > Document our longstanding de facto policies on things like correctness,
> > > > thread-safety, UB, etc.
> > > >
> > > 
> > > UB?
> > 
> > undefined behavior
> 
> yes, let's avoid acronyms

+1 (unless they are very widely established or not critical to the understaning
    of the text, like ISO in ISO C)

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Rémi Denis-Courmont]

Le torstaina 31. elokuuta 2023, 18.28.48 EEST Stefano Sabatini a écrit :
> On date Tuesday 2023-08-29 10:34:45 +0200, Anton Khirnov wrote:
> > Quoting Stefano Sabatini (2023-08-27 14:38:44)
> > 
> > > Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:
> > > > Document our longstanding de facto policies on things like
> > > > correctness,
> > > > thread-safety, UB, etc.
> > > 
> > > UB?
> > 
> > undefined behavior
> 
> yes, let's avoid acronyms

I think that developers should know common accronyms of the primary 
programming language of the project, and it is totally reasonable to expect 
that they are.

And yes, UB is a very well known accronym in this context. In my experience, 
people who don't know the accronym don't know the concept either, so spelling 
it out wouldn't even help.

TBH, this one is on you.

-- 
レミ・デニ-クールモン
http://www.remlab.net/



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Paul B Mahol]

On Fri, Sep 1, 2023 at 7:16 PM Rémi Denis-Courmont <remi@remlab.net> wrote:

> Le torstaina 31. elokuuta 2023, 18.28.48 EEST Stefano Sabatini a écrit :
> > On date Tuesday 2023-08-29 10:34:45 +0200, Anton Khirnov wrote:
> > > Quoting Stefano Sabatini (2023-08-27 14:38:44)
> > >
> > > > Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha
> scritto:
> > > > > Document our longstanding de facto policies on things like
> > > > > correctness,
> > > > > thread-safety, UB, etc.
> > > >
> > > > UB?
> > >
> > > undefined behavior
> >
> > yes, let's avoid acronyms
>
> I think that developers should know common accronyms of the primary
> programming language of the project, and it is totally reasonable to
> expect
> that they are.
>
> And yes, UB is a very well known accronym in this context. In my
> experience,
> people who don't know the accronym don't know the concept either, so
> spelling
> it out wouldn't even help.
>
> TBH, this one is on you.
>

TBH, novice and newbies lack bunch of general and specific knowledge anyway.


>
> --
> レミ・デニ-クールモン
> http://www.remlab.net/
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Stefano Sabatini]

On date Friday 2023-09-01 20:10:11 +0300, Rémi Denis-Courmont wrote:
> Le torstaina 31. elokuuta 2023, 18.28.48 EEST Stefano Sabatini a écrit :
> > On date Tuesday 2023-08-29 10:34:45 +0200, Anton Khirnov wrote:
> > > Quoting Stefano Sabatini (2023-08-27 14:38:44)
> > > 
> > > > Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:
> > > > > Document our longstanding de facto policies on things like
> > > > > correctness,
> > > > > thread-safety, UB, etc.
> > > > 
> > > > UB?
> > > 
> > > undefined behavior
> > 
> > yes, let's avoid acronyms
> 
> I think that developers should know common accronyms of the primary 
> programming language of the project, and it is totally reasonable to expect 
> that they are.
> 
> And yes, UB is a very well known accronym in this context. In my experience, 
> people who don't know the accronym don't know the concept either, so spelling 
> it out wouldn't even help.
> 
> TBH, this one is on you.

In general, it's good practice to avoid unnecessary use of acronyms as
they add ambiguity with very small gain (some fraction of a second
when typing and a few bytes in storage size). Also note that the
acronym is only used in the commit log and spelled out in the texi
file. That said, I'm not blocking on this but I added the note since
it was not very clear from the context when reading it.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1673 bytes --]

On Fri, Sep 01, 2023 at 08:10:11PM +0300, Rémi Denis-Courmont wrote:
> Le torstaina 31. elokuuta 2023, 18.28.48 EEST Stefano Sabatini a écrit :
> > On date Tuesday 2023-08-29 10:34:45 +0200, Anton Khirnov wrote:
> > > Quoting Stefano Sabatini (2023-08-27 14:38:44)
> > > 
> > > > Il sab 26 ago 2023, 20:08 Anton Khirnov <anton@khirnov.net> ha scritto:
> > > > > Document our longstanding de facto policies on things like
> > > > > correctness,
> > > > > thread-safety, UB, etc.
> > > > 
> > > > UB?
> > > 
> > > undefined behavior
> > 
> > yes, let's avoid acronyms
> 
> I think that developers should know common accronyms of the primary 
> programming language of the project, and it is totally reasonable to expect 
> that they are.
> 

> And yes, UB is a very well known accronym in this context. In my experience, 
> people who don't know the accronym don't know the concept either, so spelling 
> it out wouldn't even help.

There is a difference between not knowing a concept and not knowing what
concept is referenced.

"Undefined behavior" can be googled, and can to some extend be understood
from the meaning of the words by people with a math background.

googling "UB" results in "Universitätsbibliothek Wien" here and nothing on
the first page of links is related to "undefined"

So i do think, in case of a policy or some coding rules it should be spelled out

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Rémi Denis-Courmont]

Le lauantaina 2. syyskuuta 2023, 23.03.32 EEST Michael Niedermayer a écrit :
> > And yes, UB is a very well known accronym in this context. In my
> > experience, people who don't know the accronym don't know the concept
> > either, so spelling it out wouldn't even help.
> 
> "Undefined behavior" can be googled, and can to some extend be understood
> from the meaning of the words by people with a math background.

And? So can "C language ub" or "programming ub". I would think that adult 
software engineers know that you need to scope when searching a short 
accronym.

Either way, first result on Google is the Wikipedia page starting with "In 
computer programming, undefined behavior (UB)" (exact quote)...

-- 
雷米‧德尼-库尔蒙
http://www.remlab.net/



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 4/6] doc/developer: add a code behaviour section to development policy

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1095 bytes --]

On Sun, Sep 03, 2023 at 11:29:32AM +0300, Rémi Denis-Courmont wrote:
> Le lauantaina 2. syyskuuta 2023, 23.03.32 EEST Michael Niedermayer a écrit :
> > > And yes, UB is a very well known accronym in this context. In my
> > > experience, people who don't know the accronym don't know the concept
> > > either, so spelling it out wouldn't even help.
> > 
> > "Undefined behavior" can be googled, and can to some extend be understood
> > from the meaning of the words by people with a math background.
> 
> And? So can "C language ub" or "programming ub". I would think that adult 
> software engineers know that you need to scope when searching a short 
> accronym.

Its a policy not a puzzle
also are all software engneers here adults ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".