Re: [FFmpeg-devel] [REFUND-REQUEST] BananaPi BPI-F3 SpacemiT K1 RISC-V Devboard
On date Thursday 2024-08-01 17:14:52 +0200, Niklas Haas wrote: > Hi all, > > I would like to request reimbursement on the named RISC-V development board. > It will be used to develop and test H.264 RVV routines, and probably more in > the future (e.g. swscale). > > The total cost including tax and shipping to Germany is 153.41 EUR. Approved on my side. To get the refund, follow instructions here to generate a refund request: https://www.spi-inc.org/treasurer/reimbursement-form/ Then email it to: treasu...@rt.spi-inc.org putting me and Michael in CC:, and adding the thread link from: http://ffmpeg.org/pipermail/ffmpeg-devel/ ___ 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] avformat/dvdvideodec: Fix incorrect padding cell trim logic
On Fri, Jul 12, 2024 at 5:31 AM Marth64 wrote: > > Ping to apply on this set of 3, so I can send seeking and additional > cleanup set (as a collection). Thank you! Applied patches: eb07a59 avformat/dvdvideodec: Don't add chapter markers for empty/dummy PTTs f37f86a avformat/dvdvideodec: Remove redundant ret initializations f1abb75 avformat/dvdvideodec: Fix incorrect padding cell trim logic 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] lavfi: add perlin noise generator
On date Tuesday 2024-07-02 10:17:26 +0200, Paul B Mahol wrote: [...] > Source filters should use .activate instead. Can someone elaborate about why .activate is favoured in this case (sorry I missed the activate discussion altogether and I cannot graps it from the docs)? Also, is the direct AVFilterPad API to be considered deprecated in favor of .activate? ___ 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 8/8] fftools/ffprobe: only print AVStereo3D.view when it's defined
On date Saturday 2024-06-22 00:37:01 -0300, James Almer wrote:
> Signed-off-by: James Almer
> ---
> fftools/ffprobe.c | 3 ++-
> tests/ref/fate/matroska-spherical-mono | 1 -
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index d7ba980ff9..f9124ad5d7 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -2544,7 +2544,8 @@ static void print_pkt_side_data(WriterContext *w,
> const AVStereo3D *stereo = (AVStereo3D *)sd->data;
> print_str("type", av_stereo3d_type_name(stereo->type));
> print_int("inverted", !!(stereo->flags &
> AV_STEREO3D_FLAG_INVERT));
> -print_str("view", av_stereo3d_view_name(stereo->view));
> +if (stereo->type == AV_STEREO3D_VIEW)
> +print_str("view", av_stereo3d_view_name(stereo->view));
probably we should use print_str_opt to follow the other fields logic
[...]
___
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 date Tuesday 2024-07-02 10:56:34 +0100, Andrew Sayers wrote: > 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? The answer is definitively no, the point of AVClass is keeping the "core" functionality for a class of contexts. > * (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? Nothing prevents directly accessing a struct, but then yuo will miss the following benefits: * ABI/API compatibility in case of field renames in the context struct * validation * higher level setting functionality (e.g. to set options from a dictionary or from a string encoding multiple options) I'll try to write something (and probably we should have a dedicated class.h header to decouple it from the logging functionality). ___ 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] avformat/dvdvideodec: Fix incorrect padding cell trim logic
On date Tuesday 2024-07-02 01:41:31 -0500, Marth64 wrote:
> When -trim option is used (by default), padding cells
> at the beginning of the title are supposed to be ignored.
> The current implementation does the ignoring after we
> have locked on to the PGC navigation event stream,
> but does not set the PGC/PG state properly.
>
> This causes false positives and errors on some discs
> due to a search for a program stream cell that
> never succeeds. User would have to know to disable
> the -trim option to work around the issue.
>
> Simplify the logic and move it to the NAV packet
> event handling, in turn implementing the behaviour
> correctly and fixing the trim function for impacted discs.
>
> Signed-off-by: Marth64
> ---
> libavformat/dvdvideodec.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
> index e7132725b7..c694e8d00f 100644
> --- a/libavformat/dvdvideodec.c
> +++ b/libavformat/dvdvideodec.c
> @@ -624,7 +624,6 @@ static int dvdvideo_play_next_ps_block(AVFormatContext
> *s, DVDVideoPlaybackState
> dvdnav_vts_change_event_t *e_vts;
> dvdnav_cell_change_event_t *e_cell;
> int cur_title, cur_pgcn, cur_pgn, cur_angle, cur_title_unused, cur_ptt,
> cur_nb_angles;
> -int is_cell_promising = 0;
> pci_t *e_pci;
> dsi_t *e_dsi;
>
> @@ -706,23 +705,17 @@ static int dvdvideo_play_next_ps_block(AVFormatContext
> *s, DVDVideoPlaybackState
> continue;
>
> e_cell = (dvdnav_cell_change_event_t *) nav_buf;
> -is_cell_promising = !c->opt_trim ||
> dvdvideo_is_cell_promising(s, state->pgc, e_cell->cellN);
>
> -av_log(s, AV_LOG_DEBUG, "new cell: prev=%d new=%d
> promising=%d\n",
> -state->celln, e_cell->cellN,
> is_cell_promising);
> +av_log(s, AV_LOG_DEBUG, "new cell: prev=%d new=%d\n",
> state->celln, e_cell->cellN);
>
> if (!state->in_ps && !state->in_pgc) {
> if (cur_title == c->opt_title&&
> (c->opt_pgc || cur_ptt == c->opt_chapter_start) &&
> cur_pgcn == state->pgcn &&
> -cur_pgn == state->entry_pgn &&
> -is_cell_promising) {
> +cur_pgn == state->entry_pgn) {
>
> state->in_pgc = 1;
> }
> -
> -if (c->opt_trim && !is_cell_promising)
> -av_log(s, AV_LOG_INFO, "Skipping padding cell
> #%d\n", e_cell->cellN);
> } else if (state->celln >= e_cell->cellN || state->pgn >
> cur_pgn) {
> return AVERROR_EOF;
> }
> @@ -766,6 +759,13 @@ static int dvdvideo_play_next_ps_block(AVFormatContext
> *s, DVDVideoPlaybackState
> e_pci->pci_gi.nv_pck_lbn, state->vobu_duration,
> state->nav_pts);
>
> if (!state->in_ps) {
> +if (c->opt_trim && !dvdvideo_is_cell_promising(s,
> state->pgc, state->celln)) {
> +av_log(s, AV_LOG_INFO, "Skipping padding cell
> #%d\n", state->celln);
nit++: possibly add some more information to aid debugging, such as
"Trimming enabled, skipping padding cell for non promising cell ..."
[...]
Looks good to me anyway, 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] avformat/dvdvideodec: Remove redundant ret initializations
On date Tuesday 2024-07-02 01:03:29 -0500, Marth64 wrote: > Remove initializing ret = 0, in areas where ret is > only used to hold an error value, immediately returned, > and the function would otherwise return a literal 0. > > Signed-off-by: Marth64 > --- > libavformat/dvdvideodec.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) LGTM. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/dvdvideodec: Don't add chapter markers for empty/dummy PTTs
On date Tuesday 2024-07-02 00:41:39 -0500, Marth64 wrote:
> Some discs (usually same ones with padding cells), also have empty
> padding PTTs / chapters to accompany them. This results, for example,
> in an extra chapter marker that starts and ends at 0 (no duration).
>
> Don't add these empty chapter markers.
>
> Signed-off-by: Marth64
> ---
> libavformat/dvdvideodec.c | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/dvdvideodec.c b/libavformat/dvdvideodec.c
> index f5b7dd33e5..0bf1a82ef9 100644
> --- a/libavformat/dvdvideodec.c
> +++ b/libavformat/dvdvideodec.c
> @@ -874,6 +874,9 @@ static int dvdvideo_chapters_setup_simple(AVFormatContext
> *s)
> for (int i = chapter_start - 1; i < chapter_end; i++) {
> uint64_t time_effective = c->play_state.pgc_pg_times_est[i] -
> c->play_state.nav_pts;
>
> +if (time_effective - time_prev == 0)
> +continue;
> +
> if (chapter_start != chapter_end &&
> !avpriv_new_chapter(s, i, DVDVIDEO_TIME_BASE_Q, time_prev,
> time_effective, NULL)) {
>
> @@ -934,13 +937,16 @@ static int
> dvdvideo_chapters_setup_preindex(AVFormatContext *s)
> continue;
> }
>
> -if (!avpriv_new_chapter(s, nb_chapters, DVDVIDEO_TIME_BASE_Q,
> cur_chapter_offset,
> -cur_chapter_offset + cur_chapter_duration,
> NULL)) {
> -ret = AVERROR(ENOMEM);
> -goto end_close;
> +if (cur_chapter_duration > 0) {
> +if (!avpriv_new_chapter(s, nb_chapters, DVDVIDEO_TIME_BASE_Q,
> cur_chapter_offset,
> +cur_chapter_offset +
> cur_chapter_duration, NULL)) {
> +ret = AVERROR(ENOMEM);
> +goto end_close;
> +}
> +
> +nb_chapters++;
> }
LGTM.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] lavfi: add perlin noise generator
On date Saturday 2024-07-06 12:40:55 +0200, Paul B Mahol wrote: > On Sat, Jul 6, 2024 at 11:19 AM Stefano Sabatini wrote: [...] > Not interested in your marginal toys or project. Good, so the next time avoid the rant and save even more time. ___ 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/3] lavu/opt: Rename AV_OPT_FLAG_RUNTIME_PARAM to ...POST_INIT_SETTABLE_PARAM
On date Tuesday 2024-07-02 10:08:38 +0100, Andrew Sayers wrote: > 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(-) Looks good to me, but will wait to let the discussion about the "little benefit renaming" settle. ___ 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] lavu/opt: Rename AV_OPT_FLAG_RUNTIME_PARAM to ...POST_INIT_SETTABLE_PARAM
On date Tuesday 2024-07-02 11:58:09 +0800, Zhao Zhili wrote: > > > 在 2024年7月2日，上午6:33，Stefano Sabatini 写道： > > > > On date Sunday 2024-06-16 18:08:29 +0100, Andrew Sayers wrote: > >> 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. > > Let’s not bother the user again and again for little benefit. Add > doc is fine, please don’t rename the flag. The point is that name is the documentation, having a better flag name guarantees simpler/better use of the API, as opposed to a once-time mechanical name change for a flag which is probably never used in external applications code. Aslo, by following this rule, we should never rename flags for "little benefit", sticking to misnamed constants (as this is the case) which require much more effort to double check the API (since the name was misleading, you need to double check with the API). If you think in terms of developer time, it is easy to measure the time required to check and replace the code (a few minutes?) while it is much harder to assess the overall time spent checking/writing/validating the code due to bad naming, but my suspect is that even for a trivial change like this one, it is a much bigger time that you are saving to the user. ___ 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 date Tuesday 2024-07-02 10:08:37 +0100, Andrew Sayers wrote: > 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. I agree the rename is a net improvement in the long term, giving that this will remove the developer cognitive load and possibly mistakes due to the misleading name, while the effort required to make the mechanical rename should be rather small - also there is no evidence that users are really using this API in their application code so probably the overall effort is close to zero in this case and it is only benefits the developers. 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. ___ 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] lavfi/perlin: Fix out of bounds stack buffer write
On date Tuesday 2024-07-02 20:38:00 +0200, Marvin Scholz wrote:
> An incorrect calculation in ff_perlin_init causes a write to the
> stack array at index 256, which is out of bounds.
>
> Fixes: CID1608711
> ---
> libavfilter/perlin.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavfilter/perlin.c b/libavfilter/perlin.c
> index 09bae7ad33..ffad8c1e4e 100644
> --- a/libavfilter/perlin.c
> +++ b/libavfilter/perlin.c
> @@ -129,7 +129,7 @@ int ff_perlin_init(FFPerlin *perlin, double period, int
> octaves, double persiste
> for (i = 0; i < 256; i++) {
> unsigned int random_idx = av_lfg_get(&lfg) % (256-i);
> uint8_t random_val = random_permutations[random_idx];
> -random_permutations[random_idx] = random_permutations[256-i];
> +random_permutations[random_idx] = random_permutations[255-i];
>
> perlin->permutations[i] = perlin->permutations[i+256] =
> random_val;
> }
Looks good, 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] lavfi: add perlin noise generator
On date Thursday 2024-07-04 17:29:16 +0200, Michael Koch wrote: > Tested and working fine. It would be nice if you could add the default > values to the documentation. Will do, 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] lavfi: add perlin noise generator
On date Tuesday 2024-07-02 10:17:26 +0200, Paul B Mahol wrote: > No need for query function. > This implementation is so blatantly slow and inefficient that is not useful. > Source filters should use .activate instead. > Many more other issues... It was pending reviews for more than two weeks. About the lack of optimizations, I'm aware of it but wanted to keep the initial commit as close as possible to the original implementation, and didn't had time to work on it yet. Also, patches are welcome of course. ___ 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] lavu/opt: Rename AV_OPT_FLAG_RUNTIME_PARAM to ...POST_INIT_SETTABLE_PARAM
On date Sunday 2024-06-16 18:08:29 +0100, Andrew Sayers wrote:
> 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)
This needs to be dropped at the next bump.
1. define in libavutil/version.h
#define FF_API_OPT_FLAG_RUNTIME_PARAM (LIBAVUTIL_VERSION_MAJOR < 60)
2. ifdef to automatically drop this at the next bump
#if FF_API_OPT_FLAG_RUNTIME_PARAM
/**
* 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)
#endif
3. add a note to doc/APIchanges for the new symbol.
___
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 0/3] s/RUNTIME/POST_INIT_SETTABLE/
On date Sunday 2024-06-16 18:08:28 +0100, Andrew Sayers wrote: > 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. Correct, but we can deprecate and keep the old symbols until the next major bump. [...] ___ 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 date Sunday 2024-06-16 19:02:51 +0100, Andrew Sayers wrote: > 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
Re: [FFmpeg-devel] [PATCH] avfilter/af_afade: fix opt_type for nb_samples/ns
On date Saturday 2024-06-22 15:20:52 +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?
LGTM as well, will apply after dropping the second sentence in the
commit log.
___
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 date Monday 2024-06-24 16:44:33 +0200, Vittorio Giovara wrote: > On Sun, Jun 23, 2024 at 7:57 PM 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. > > > > by that reasoning we should ban all subtitles from all the libraries > > av is a shorthand of multimedia and many people in the industry refer to > ffmpeg libs as "libav*" so it feels a bit odd to push for preserving an > alternative name > > > > 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's still 4 libraries vs 2... and swr/avr is shrouded in bad history that > is not worth bringing up > > I'd understand opposing a rename just for the sake of renaming, but this is > essentially a new library, i see no value in preserving the old naming > scheme, if not making downstream life worse :x +1 av_ is a kind of prefix used by most FFmpeg libraries, so it's not only about Audio/Video. Since the rewrite will probably break API anyhow, we should move in the direction of making this more consistent with the other 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/dvdvideodec: Fix duration logic with 1 chapter and validate chapter range
On date Monday 2024-07-01 22:49:42 +0200, Stefano Sabatini wrote: > On date Wednesday 2024-06-26 22:05:46 -0500, Marth64 wrote: > > Chapters and duration are calculated together in dvdvideo demuxer. > > Previous chapter calculation logic treated extraction of 1 chapter > > using chapter_start and chapter_end switches incorrectly, returning > > the duration of the entire title instead of just the segment. > > > > > Fix the logic so that it calculates and returns the duration of the > > chapter segment instead. Additionally, validate that chapter_end > > exceeds chapter_start (except in the special case of 0). > > What is the meaning of the 0 special case? > > > > > Signed-off-by: Marth64 > > --- > > libavformat/dvdvideodec.c | 15 --- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > [...] > > Looks good to me. Both patches applied. ___ 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] avformat/dvdvideodec: Fix duration logic with 1 chapter and validate chapter range
On date Wednesday 2024-06-26 22:05:46 -0500, Marth64 wrote: > Chapters and duration are calculated together in dvdvideo demuxer. > Previous chapter calculation logic treated extraction of 1 chapter > using chapter_start and chapter_end switches incorrectly, returning > the duration of the entire title instead of just the segment. > > Fix the logic so that it calculates and returns the duration of the > chapter segment instead. Additionally, validate that chapter_end > exceeds chapter_start (except in the special case of 0). What is the meaning of the 0 special case? > > Signed-off-by: Marth64 > --- > libavformat/dvdvideodec.c | 15 --- > 1 file changed, 12 insertions(+), 3 deletions(-) [...] Looks good to 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] [PATCH] avformat/dvdvideodec: Do not EOF on WAIT events
On date Wednesday 2024-06-26 20:34:35 -0500, Marth64 wrote: > A DVDNAV_WAIT event by itself should not warrant an > EOF when navigating the program stream. Some discs > have WAIT events in the middle of a title, causing > playback to end prematurely prior to this fix. > > Signed-off-by: Marth64 > --- > libavformat/dvdvideodec.c | 17 - > 1 file changed, 8 insertions(+), 9 deletions(-) LGTM. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 5/5] avfilter/vf_showinfo: don't use sizeof(AVSphericalMapping)
On date Wednesday 2024-06-26 11:10:14 -0300, James Almer wrote: > It's not part of the libavutil ABI. > > Signed-off-by: James Almer > --- > libavfilter/vf_showinfo.c | 5 - > 1 file changed, 5 deletions(-) All patches look good to me, 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] lavfi: add perlin noise generator
On date Sunday 2024-06-16 17:37:09 +0200, Stefano Sabatini wrote: > On date Thursday 2024-06-13 16:45:48 +0100, Andrew Sayers wrote: [...] > Applied other fixes as well, thanks. > From 7d9ffbdb8e419ca01e60604038d5534a3ca8ae0e 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 Applied. ___ 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: Discuss AV_OPT_FLAG_RUNTIME_PARAM more explicitly
On date Thursday 2024-06-06 17:02:06 +0100, Andrew Sayers wrote:
> 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)
I'm fine with changing the description, but then I wonder if we should
also rename the flag accordingly (by adding a new alias and
deprecating the old one):
AV_OPT_FLAG_POST_INIT_SETTABLE_PARAM
??
> /**
> @@ -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.
> + *
drop this note and the following ones, this is assumed by the flags so
there is no need to repeat this all over
___
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
On date Thursday 2024-06-13 16:45:48 +0100, Andrew Sayers wrote:
> 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
[...]
> > +@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.
kept current form since the following is a complete sentence
> > +@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 gray level to red-only
>
> I initially thought this was a typo for "read-only" - maybe s/red/blue/?
I prefer to keep red, this is a fire approximation
[...]
>
> > +{ "random_mode", "set random mode used to compute initial pattern",
> > OFFSET(random_mode), AV_OPT_TYPE_INT, {.i64=FF_PERLIN_RANDOM_MODE_RANDOM},
> > 0, FF_PERLIN_RANDOM_MODE_NB-1, FLAGS, .unit = "random_mode" },
> > +{ "random", "compute and use random seed", 0, AV_OPT_TYPE_CONST,
> > {.i64=FF_PERLIN_RANDOM_MODE_RANDOM}, 0, 0, FLAGS, .unit = "random_mode" },
> > +{ "ken", "use the predefined initial pattern defined by Ken Perlin in
> > the original article", 0, AV_OPT_TYPE_CONST,
> > {.i64=FF_PERLIN_RANDOM_MODE_KEN}, 0, 0, FLAGS, .unit = "random_mode" },
> > +{ "seed", "use the value specified by random_seed", 0,
> > AV_OPT_TYPE_CONST, {.i64=FF_PERLIN_RANDOM_MODE_SEED}, 0, 0, FLAGS, .unit =
> > "random_mode" },
> > +
> > +{ "random_seed", "set the seed for filling the initial grid randomly",
> > OFFSET(random_seed), AV_OPT_TYPE_UINT, {.i64=0}, 0, UINT_MAX, FLAGS },
> > +{ "seed","set the seed for filling the initial grid randomly",
> > OFFSET(random_seed), AV_OPT_TYPE_UINT, {.i64=0}, 0, UINT_MAX, FLAGS },
>
> Nit: "set the seed for filling the initial grid randomly" is ambiguous.
> Do you mean:
>
> a) Set the seed for the RNG to a specified value (value is the seed number)
> b) Pick a random number to fill the grid with (value is a boolean yes/no)
>
> I settled on the former after several reads, but would appreciate clearer
> wording.
Changed to:
set the seed for filling the initial pattern
[...]
Applied other fixes as well, thanks.
>From 7d9ffbdb8e419ca01e60604038d5534a3ca8ae0e 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 1
Re: [FFmpeg-devel] [PATCH v6 1/4] doc: Explain what "context" means
On date Thursday 2024-06-13 15:20:38 +0100, Andrew Sayers wrote: > On Wed, Jun 12, 2024 at 10:52:00PM +0200, Stefano Sabatini wrote: [...] > > > +@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. The context is just another way to call a struct used to keep an entity state operated by several functions (that is in other words an object and its methods), it's mostly about the specific jargon used by FFmpeg (and used by other C projects as well). In addition to this we provide some generic utilities (logging+avoptions) which can be used through AVClass employment. Giving a longer explanation is making this appear something more complicated than actually is. My point is that providing more information than actually needed provides the long-wall-of-text effect (I need to read through all this to understand it - nah I'd rather give-up), thus discouraging readers. > > > > > > + > > > +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. I mean, if you read for the first time: | [the context] term's meaning is guaranteed by *the project's | community*, not the languaguage it's written for. | That means guarantees may be more flexible and change more over time. it's very hard to figure out what these guarantees are about, and this might apply to every specific language and to every specific term, that's why I consider this "vague". [...] > > > +Some functions fit awkwardly within FFmpeg's context idiom, so they send > > > mixed > > > +signals. For example, av_ambient_viewing_environment_create_side_data() > > > creates > > > +an AVAmbientViewingEnvironment context, then adds it to the side-data of > > > an > > > +AVFrame context. So its name hints at one context, its parameter hints > > > at > > > +another, and its documentation is silent on the issue. You might prefer > > > to > > > +think of such functions as not having a context, or as “receiving” one > > > context > > > +and “producing” another. > > > > I'd skip this paragraph. In fact, I think that API makes perfect > > sense, OOP languages adopt such constructs all the time, for example > > this could be a static module/class constructor. In other words, we > > are not telling anywhere that all functions should take a "context" as > > its first argument, and the documentation specify exactly how this > > works, if you feel this is not clear or silent probably this is a sign > > that that function documentation should be extended. > >
Re: [FFmpeg-devel] [PATCH v6 1/4] doc: Explain what "context" means
On date Tuesday 2024-06-04 15:47:21 +0100, Andrew Sayers wrote:
> 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.
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.
> +
> +@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
Re: [FFmpeg-devel] [PATCH] doc/faq: Provide information about git send-email and gmail
On date Wednesday 2024-06-12 19:46:59 +0200, Michael Niedermayer wrote:
> The 2 links are the clearest i found.
>
> Signed-off-by: Michael Niedermayer
> ---
> doc/faq.texi | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/doc/faq.texi b/doc/faq.texi
> index 477cc60533a..d07ed533dd7 100644
> --- a/doc/faq.texi
> +++ b/doc/faq.texi
> @@ -683,4 +683,9 @@ Do you happen to have a @code{~} character in the samples
> path to indicate a
> home directory? The value is used in ways where the shell cannot expand it,
> causing FATE to not find files. Just replace @code{~} by the full path.
>
> +@section How to setup git send-email?
> +
> +Please see @url{https://git-send-email.io/}.
> +For gmail additionally see
> @url{https://shallowsky.com/blog/tech/email/gmail-app-passwds.html}.
> +
Should be good but I think it's more useful to put this close to
developers.texi - Submitting patches
___
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: add sdlvsink for video display
On date Tuesday 2024-06-11 21:13:48 +0800, Shiqi Zhu wrote: > On Fri, 7 Jun 2024 at 19:55, Rémi Denis-Courmont wrote: > > Le 7 juin 2024 12:53:51 GMT+03:00, Michael Niedermayer > > a écrit : > > >We can require anything from an API that we are able to change and extend > > >Of course we can decide not to allow such requirment even if optional > > >but we surely _could_ add such a feature if we choose to do so > > > > Sure. You can also require infinite memory or an oracle be provided. That's > > just not going to happen though. And having libraries depend on the main > > thread is a well-documented malpractice. > > > > I don't think we should add maintenance burden with code that can't be used > > safely. > > Thank you all for your attention to this patch; I greatly appreciate it. > > I'd like to provide a brief recap of the issue we've been discussing, > with the following points: > > 1. Addition of sink type in the filter: > This enhancement is primarily based on the existing avfilter mechanism > and serves as a strengthening module. Using SDL as the sink doesn't > seem to be a good fit, as I'll attempt to rectify in the following > patch. > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240611130310.1131755-1-hiccup...@gmail.com/ > > 2. Utilizing SDL as an implementation for the sink: > Before submitting the patch, I hadn't considered many aspects. During > the intense discussions, I retested the patch on different operating > systems, with the following results, hoping it may assist those > interested in this issue: > > Command: ./ffmpeg -lavfi > "testsrc2=size=300x200:rate=25:duration=500,format=yuv420p,sdlvsink" > -f null /dev/null In addition to this, I wonder if adding a vsink for each different output device is the correct way. We have a movie source which can be used to read from libavformat/libavdevice, probablhy we should have a movie sink to be used to write to libavformat/libavdevice, meaning that a single sink would enable access to all the supported libavformat/libavdevice outputs. I started having a look in that direction a looot of time ago. This was never finalized because I was not sure about ways to pass options to encoders and muxers, and about dealing with a variable number of outputs, I'm attaching this very old proof-of-concept patch for reference. This approach would be possibly much more complex, but should provide a single bridge in place of having a different sink for every output device or muxer. >From dc88e3f19ad0e481e0adc5e192ad3e2b4b249f55 Mon Sep 17 00:00:00 2001 From: Stefano Sabatini Date: Wed, 11 Apr 2012 15:10:14 +0200 Subject: [PATCH] lavfi: add moviesink and amoviesink sinks --- configure| 2 + libavfilter/Makefile | 4 + libavfilter/allfilters.c | 2 + libavfilter/sink_moviesink.c | 365 +++ 4 files changed, 373 insertions(+) create mode 100644 libavfilter/sink_moviesink.c diff --git a/configure b/configure index 81c7cc18a2..5e08fbbf6c 100755 --- a/configure +++ b/configure @@ -1674,6 +1674,7 @@ udp_protocol_deps="network" # filters aconvert_filter_deps="swresample" amovie_filter_deps="avcodec avformat" +amoviesink_filter_deps="avcodec avformat" aresample_filter_deps="swresample" ass_filter_deps="libass" asyncts_filter_deps="avresample" @@ -1690,6 +1691,7 @@ frei0r_src_filter_deps="frei0r dlopen" frei0r_src_filter_extralibs='$ldl' hqdn3d_filter_deps="gpl" movie_filter_deps="avcodec avformat" +moviesink_filter_deps="avcodec avformat" mp_filter_deps="gpl avcodec swscale postproc" mptestsrc_filter_deps="gpl" negate_filter_deps="lut_filter" diff --git a/libavfilter/Makefile b/libavfilter/Makefile index 29345fc15e..a2bd404304 100644 --- a/libavfilter/Makefile +++ b/libavfilter/Makefile @@ -7,8 +7,10 @@ FFLIBS-$(CONFIG_RESAMPLE_FILTER) += avresample FFLIBS-$(CONFIG_ACONVERT_FILTER) += swresample FFLIBS-$(CONFIG_AMOVIE_FILTER) += avformat avcodec +FFLIBS-$(CONFIG_AMOVIESINK_FILTER) += avformat avcodec FFLIBS-$(CONFIG_ARESAMPLE_FILTER)+= swresample FFLIBS-$(CONFIG_MOVIE_FILTER)+= avformat avcodec +FFLIBS-$(CONFIG_MOVIESINK_FILTER)+= avformat avcodec FFLIBS-$(CONFIG_PAN_FILTER) += swresample FFLIBS-$(CONFIG_REMOVELOGO_FILTER) += avformat avcodec FFLIBS-$(CONFIG_MP_FILTER) += avcodec postproc @@ -65,6 +67,7 @@ OBJS-$(CONFIG_AMOVIE_FILTER) += src_movie.o OBJS-$(CONFIG_ANULLSRC_FILTER) += asrc_anullsrc.o OBJS-$(CONFIG_ABUFFERSINK_FILTER)+= sin
Re: [FFmpeg-devel] lavfi: add perlin noise generator
On date Monday 2024-06-03 11:41:59 +0200, Stefano Sabatini wrote:
> On date Tuesday 2024-05-28 18:58:28 +0200, Stefano Sabatini wrote:
> > On date Monday 2024-05-27 23:37:33 +0200, Stefano Sabatini wrote:
> > > Hi,
> > >
> > > still missing documentation and might be optimized (and maybe extended
> > > to support gray16 - this should be simple), comments are welcome.
> >
> > Updated with documentation.
>
> > From 607459e7a184ab2d111b65f5017fb7f76e3bd58d 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
>
> Anyone interested with reviewing this?
>
> Otherwise I'll probably push in a week or so, further improvements can
> be done on top of that.
Updated patch with minor doc fixes, will push it in a few days unless
I see comments.
>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).
+
+@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
+
+@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
+
+@subsection Examples
+@itemize
+@item
+Generate single component:
+@example
+perlin
+@end example
+
+@item
+Use Perlin noise with 7 components, each one with a halved contribute
+to total amplitude:
+@example
+perlin=octaves=7:persistence=0.5
+@end example
+
+@item
+Chain Perlin noise with the
Re: [FFmpeg-devel] [PATCH v6] avcodec: add farbfeld encoder
On date Wednesday 2024-06-05 13:14:01 +0200, Andreas Rheinhardt wrote: > Stefano Sabatini: [...] > >> One does not need two checks as long as int is 32 bits (because then one > >> can just perform the addition in 64bits). > > > > sizeof(int) is not defined by the C standard, so you cannot assume it > > is 32 bits (even if on most platforms/compilers it will be) > > > > Did you even read the following? It handles the case where simply using > 64bits is not enough. Yes, but there is no need to mix types, introduce ifdeffery and make more assumptions when there is a simpler solution, please let's stick at that. > >> Just use the following (#if > >> has been used because compilers have a tendency to emit warnings if a > >> particular check is tautologically false): > >> > >> #if INT_MAX > INT64_MAX - HEADER_SIZE > >> if (raw_img_size > INT64_MAX - HEADER_SIZE) > >> return AVERROR(ERANGE); > >> #endif ___ 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] avcodec: add farbfeld encoder
On date Wednesday 2024-06-05 12:02:08 +0200, Andreas Rheinhardt wrote:
> Stefano Sabatini:
> > On date Tuesday 2024-06-04 17:28:35 -0500, Marcus B Spencer wrote:
[...]
> >> +#define HEADER_SIZE 16
> >> +
> >> +static int farbfeld_encode_frame(AVCodecContext *ctx, AVPacket *pkt,
> >> + const AVFrame *p, int *got_packet)
> >> +{
> >
> >> +int raw_img_size = av_image_get_buffer_size(
> >> +p->format,
> >> +p->width,
> >> +p->height,
> >> +1
> >> +);
> >
> >> +int64_t buf_size = (int64_t)raw_img_size + HEADER_SIZE;
> >
> > not yet, this might change the sign for a negative raw_img_size, you
> > need two separate checks (one is not enough), as in the following:
> >
> > int raw_img_size = av_image_get_buffer_size(p->format, p->width,p->height,
> > 1);
> >
> > if (raw_image_size < 0)
> > return raw_image_size;
> >
> > int buf_size = raw_img_size + HEADER_SIZE;
> > if (buf_size < 0)
> > return AVERROR(EINVAL);
>
> This is absolutely wrong: raw_img_size is nonnegative here as is
> HEADER_SIZE and the addition will be performed as an int, so that
> overflow would be UB which implies that the compiler can optimize this
> check away.
Correct, the following should avoid the UB if I'm not mistaken again:
if (HEADER_SIZE > (INT_MAX - raw_img_size))
return AVERROR(EINVAL);
int buf_size = raw_img_size + HEADER_SIZE;
...
> One does not need two checks as long as int is 32 bits (because then one
> can just perform the addition in 64bits).
sizeof(int) is not defined by the C standard, so you cannot assume it
is 32 bits (even if on most platforms/compilers it will be)
> Just use the following (#if
> has been used because compilers have a tendency to emit warnings if a
> particular check is tautologically false):
>
> #if INT_MAX > INT64_MAX - HEADER_SIZE
> if (raw_img_size > INT64_MAX - HEADER_SIZE)
> return AVERROR(ERANGE);
> #endif
___
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 date Tuesday 2024-06-04 15:47:22 +0100, Andrew Sayers wrote:
> ---
> 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
Generic logging and options handling facilities.
> + *
> + * Logging and AVOptions functions expect to be passed structs
Logging and options handling functions ...
> + * 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".
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.
> + *
> + * @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
... 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
[...]
___
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/trim: flag trim filter as metadata only
On date Tuesday 2024-06-04 23:41:05 +0530, Gyan Doshi wrote:
> Similar to select filter for video - it can only pass through or drop frames
> ---
> libavfilter/trim.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libavfilter/trim.c b/libavfilter/trim.c
> index 4c1a2b4f48..4afc4c74bb 100644
> --- a/libavfilter/trim.c
> +++ b/libavfilter/trim.c
> @@ -364,6 +364,7 @@ const AVFilter ff_vf_trim = {
> .activate= activate,
> .priv_size = sizeof(TrimContext),
> .priv_class = &trim_class,
> +.flags = AVFILTER_FLAG_METADATA_ONLY,
> FILTER_INPUTS(trim_inputs),
> FILTER_OUTPUTS(ff_video_default_filterpad),
> };
> --
> 2.44.0
Should be good.
___
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
On date Tuesday 2024-05-28 18:58:28 +0200, Stefano Sabatini wrote: > On date Monday 2024-05-27 23:37:33 +0200, Stefano Sabatini wrote: > > Hi, > > > > still missing documentation and might be optimized (and maybe extended > > to support gray16 - this should be simple), comments are welcome. > > Updated with documentation. > From 607459e7a184ab2d111b65f5017fb7f76e3bd58d 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 Anyone interested with reviewing this? Otherwise I'll probably push in a week or so, further improvements can be done on top of that. ___ 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 date Wednesday 2024-05-29 11:50:40 +0100, Andrew Sayers wrote:
> 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.
It impacts the user as this changes the API. I guess when AVClass was
removed from AVFrame that was done with a major bump to advertise an
API break.
> 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.
This looks redundant to me, this is true for all the opaque structus,
and we don't want to repeat this again and again (this is a feature of
the C language, not of the API itself).
> + *
> + * @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.
Better to treat this as a separate patch anyway.
> *Are AVOptions just command-line options?*
No, in fact this is not about comman-line options at all.
>
> 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.
AVOptions (or better the av_opt API) is a system to set
fields/properties/options on an enabled struct. This is a high level
API since it allows multiple options setting, validation, setting
from a dictionary etc., and enables to abstract the struct field name
(since the option names might be different from the field name).
>
> 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"?
It is not about CLI options, although you can use introspection to get
the supported options and build an interface to set them.
___
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 date Sunday 2024-05-26 13:06:52 +0100, Andrew Sayers wrote: > 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. 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 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. > 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 langu
[FFmpeg-devel] lavfi: add perlin noise generator
On date Monday 2024-05-27 23:37:33 +0200, Stefano Sabatini wrote:
> Hi,
>
> still missing documentation and might be optimized (and maybe extended
> to support gray16 - this should be simple), comments are welcome.
Updated with documentation.
>From 607459e7a184ab2d111b65f5017fb7f76e3bd58d 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 | 100 +
libavfilter/vsrc_perlin.c | 169
7 files changed, 596 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 12770e4296..1670038d00 100644
--- a/Changelog
+++ b/Changelog
@@ -11,6 +11,7 @@ version :
- vf_scale2ref deprecated
- qsv_params option added for QSV encoders
- VVC decoder compatible with DVB test content
+- perlin source
version 7.0:
diff --git a/doc/filters.texi b/doc/filters.texi
index f5bf475d13..baed8c5530 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
@@ -29280,6 +29283,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).
+
+@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
+
+@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
+
+@subsection Examples
+@itemize
+@item
+Generate single component:
+@example
+perlin
+@end example
+
+@item
+Use Perlin noise with 7 components, each one with a halved contribute
+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 gray level to red-only
+signal:
+@example
+perlin=octaves=7:tscale=0.4:yscale=0.3,lutrgb=r=val:b=0:g=0
+@end example
+@end itemize
+
@section qrencodesrc
Generate a QR code using the libqrencode library (see
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 5992fd161f..63088e9286 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -603,6 +603,7 @@ OBJS-$(CONFIG_NULLSRC_FILTER)+= vsrc_testsrc.o
OBJS-$(CONFIG_OPENCLSRC_FILT
[FFmpeg-devel] [POC] lavfi: add perlin noise generator
Hi,
still missing documentation and might be optimized (and maybe extended
to support gray16 - this should be simple), comments are welcome.
>From 5018f5b887ef84c19cbf3f76c6607b49a6c99f2a 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
---
libavfilter/Makefile | 1 +
libavfilter/allfilters.c | 1 +
libavfilter/perlin.c | 221 ++
libavfilter/perlin.h | 52 +
libavfilter/vsrc_perlin.c | 170 +
5 files changed, 445 insertions(+)
create mode 100644 libavfilter/perlin.c
create mode 100644 libavfilter/perlin.h
create mode 100644 libavfilter/vsrc_perlin.c
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 5992fd161f..63088e9286 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -603,6 +603,7 @@ OBJS-$(CONFIG_NULLSRC_FILTER)+= vsrc_testsrc.o
OBJS-$(CONFIG_OPENCLSRC_FILTER) += vf_program_opencl.o opencl.o
OBJS-$(CONFIG_PAL75BARS_FILTER) += vsrc_testsrc.o
OBJS-$(CONFIG_PAL100BARS_FILTER) += vsrc_testsrc.o
+OBJS-$(CONFIG_PERLIN_FILTER) += vsrc_perlin.o perlin.o
OBJS-$(CONFIG_QRENCODE_FILTER) += qrencode.o textutils.o
OBJS-$(CONFIG_QRENCODESRC_FILTER)+= qrencode.o textutils.o
OBJS-$(CONFIG_RGBTESTSRC_FILTER) += vsrc_testsrc.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index c532682fc2..63600e9b58 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -569,6 +569,7 @@ extern const AVFilter ff_vsrc_openclsrc;
extern const AVFilter ff_vsrc_qrencodesrc;
extern const AVFilter ff_vsrc_pal75bars;
extern const AVFilter ff_vsrc_pal100bars;
+extern const AVFilter ff_vsrc_perlin;
extern const AVFilter ff_vsrc_rgbtestsrc;
extern const AVFilter ff_vsrc_sierpinski;
extern const AVFilter ff_vsrc_smptebars;
diff --git a/libavfilter/perlin.c b/libavfilter/perlin.c
new file mode 100644
index 00..07a7cc18ea
--- /dev/null
+++ b/libavfilter/perlin.c
@@ -0,0 +1,221 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/**
+ * @file
+ * Perlin Noise generator, based on code from:
+ * https://adrianb.io/2014/08/09/perlinnoise.html
+ *
+ * Original article from Ken Perlin:
+ * http://mrl.nyu.edu/~perlin/paper445.pdf
+ */
+
+#include
+
+#include "libavutil/lfg.h"
+#include "libavutil/random_seed.h"
+#include "perlin.h"
+
+static inline int inc(int num, int period)
+{
+num++;
+if (period > 0)
+num %= period;
+return num;
+}
+
+static inline double grad(int hash, double x, double y, double z)
+{
+// Take the hashed value and take the first 4 bits of it (15 == 0b)
+int h = hash & 15;
+// If the most significant bit (MSB) of the hash is 0 then set u = x. Otherwise y.
+double u = h < 8 /* 0b1000 */ ? x : y;
+double v;
+
+// In Ken Perlin's original implementation this was another
+// conditional operator (?:), then expanded for readability.
+if (h < 4 /* 0b0100 */)
+// If the first and second significant bits are 0 set v = y
+v = y;
+// If the first and second significant bits are 1 set v = x
+else if (h == 12 /* 0b1100 */ || h == 14 /* 0b1110*/)
+v = x;
+else
+// If the first and second significant bits are not equal (0/1, 1/0) set v = z
+v = z;
+
+// Use the last 2 bits to decide if u and v are positive or negative. Then return their addition.
+return ((h&1) == 0 ? u : -u)+((h&2) == 0 ? v : -v);
+}
+
+static inline double fade(double t)
+{
+// Fade function as defined by Ken Perlin. This eases coordinate values
+// so that they will "ease" towards integral values. This ends up smoothing
+// the final output.
+return t * t * t * (t * (t * 6 - 15) + 10); // 6t^5 - 15t^4 + 10t^3
+}
+
+static double lerp(double a, double b, double x)
+{
+return a + x * (b - a);
+}
+
+// Hash lookup table as defined by Ken Perlin. This is a randomly
+// arranged array of all numbers from 0-255 inclusive.
+static uint8_t ken_permutations[] = {
+151, 160, 137, 91, 90,
Re: [FFmpeg-devel] [PATCH v5 1/4] doc: Explain what "context" means
On date Thursday 2024-05-23 21:00:40 +0100, Andrew Sayers wrote:
> 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.
nit++: better to use simple "" quoting to help people with simple
ascii keyboard layouts.
nit++: "Context" is a name employed for a widely-used programming idiom.
since "context" is not the idiom itself.
> +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):
> +...
> +```
nit: as noted, I'd skip this example altogether
> +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 );
> +
> +}
> +```
Not convinced this is a good example, especially given that a struct
with a single field might be used directly so this looks a bit like
obfuscation.
Rather than this, maybe something as:
struct AVBikeShedContext {
uint8_t color_r;
uint8_t color_g;
uint8_t color_b;
char *color;
int (* set_default_color)(const char *color);
}
AVBikeShedContext *av_bikeshed_context_alloc();
intav_bikeshed_context_set_color(AVBikeShedContext *bikeshed,
const char *color);
const char*av_bikeshed_context_get_color(AVBikeShedContext *bikeshed);
void av_bikeshed_context_get_rgb_color(AVBikeShedContext
*bikeshed, uint8_t *color_r, uint8_t *color_g, uint8_t *color_b);
Then you can use:
int set_bikeshed_pink_color(AVBikeShedContext *bikeshed, const char *color) {
return av_bikeshed_context_set_color(bikeshed, "pink");
}
bikeshed->set_default_color = set_bikeshed_pink_color;
to provide a callback example
> +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 *f
Re: [FFmpeg-devel] [PATCH v5 2/4] lavu: Clarify relationship between AVClass, AVOption and context
On date Thursday 2024-05-23 21:00:41 +0100, Andrew Sayers wrote:
> ---
> 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
Looks mostly good to me but now I wonder if we are really confusing
introspection with AVOptions (AVOptions adopt introspection but it's
mostly about AVOptions themselves).
So maybe we should replace "introspection" with something more
concrete, such as:
Generic logging and options 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
ditto, more concrete with:
Generic options facilities ...
[...]
___
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 date Wednesday 2024-05-22 17:07:51 +0100, Andrew Sayers wrote:
[...]
> > > 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.
I think it's safe to assume UTF-8 as the default nowadays, and every
other encoding broken in same way. Latin-1 is assumed probably only by
a minor group of FFmpeg users.
> >
> > > 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
>
But again, this is redundnat if you know Python, otherwise is not very
helpful if you don't. In this case probably it's good just to keep the
mention to self/this, without a specific example.
> >
> > > +
> > > +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 earlier paragraph acknowledged that contexts resemble OOP (telling the OOP
> audience we get it), then this paragraph adds "but they're not the same"
> (telling the OOP audience we disagree). To be more useful to non-OOP folk,
> how about:
>
> Contexts can be a fundamental building block of OOP, but can also be used
> in
> procedural projects like FFmpeg.
What perplexes me is that "context" is not part of the standard OOP
jargon, so this is probably adding more to the confusion. I think the
target here is to clarify what context is meant for in FFmpeg, and
this can be simply defined without any reference to OOP:
|A context is a structure used to contain data (allocated memory,
|configuration, and/or state) for operations working on the same
|entity.
As such a cont
Re: [FFmpeg-devel] [PATCH v3 1/3] doc: Explain what "context" means
On date Wednesday 2024-05-22 13:47:36 +0100, Andrew Sayers wrote: > 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). >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? av_ambient_viewing_environment_ defines the namespace. In this case we are not using the av_frame_ API since we want to have all the ambient API defined in a single place. Alternatively we might have extended the av_frame API (e.g. av_frame_create_ambient_viewing_environment_side_data()), but we wanted to avoid to reference the ambient API in the minimal frame API, so this approach makes perfect sense to me and I cannot see major inconsistencies. You can think about it in terms of a static or class constructor function, where AVFrame is simply an argument of the contructor. What is the approach that you would have expected in this 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. I see, but I still see an excessive use of the context concept in place of the simpler and more natural use of parameters (in this case AVFrame is simply the input element), and this scheme is pretty common in C APIs (note that ambient is a simple structure, no need to use AVClass or options for this simple struct), so I see no need to tag this as an edge case. ___ 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 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".
>
> My current thinking is to propose separate patches renaming arguments
> to `ctx` whenever I find functions I can't parse. That's not as good
> as a simple rule like "the first argument is always the context", but
> better than adding a paragraph or two about how to read the docs.
There cannot be such rule, because it would be false in many cases.
> > Also, you are assuming that all the function should have a
> > context. That's not the case, as you don't always need to keep track
> > of a "context" when performing operations.
>
[...]
> > > av_channel_description_bprint() takes a `struct AVBPrint *` as its first
> > > argument, then `enum AVChannel`. Is the context AVBPrint, AVChannel,
> > > or both? Does it make sense for a function to have two contexts?
> >
> > Again, this should be clear from the doxy:
> > /**
> > * Get a human readable string describing a given channel.
> > *
> > * @param buf pre-allocated buffer where to put the generated string
> > * @param buf_size size in bytes of the buffer.
> > * @param channel the AVChannel whose description to get
> > * @return amount of bytes needed to hold the output string, or a negative
> > AVERROR
> > * on failure. If the returned value is bigger than buf_size, then
> > the
> > * string was truncated.
> > */
> > int av_channel_description(char *buf, size_t buf_size, enum AVChannel
> > channel);
> >
> > /**
> > * bprint variant of av_channel_description().
> > *
> > * @note the string will be appended to the bprint buffer.
> > */
> > void av_channel_description_bprint(struct AVBPrint *bp, enum AVChannel
> > channel_id);
>
> I think you're saying that I should look at which word appears more
> often in the doxy ("channel") rather than which word appears first in
> the argument list ("buf")? As above, the solution might be to rename
> the variable in a separate patch rather than teach people another
> special case.
This is more about the semantics described in English language by the
doxy (which is normative). Again, thinking in terms of "contexts" is
misleading in this case.
In this case you have two functions, av_channel_description writing a
string to a buffer with fixed size, the second modifying an "AVBPrint"
struct, which is a high-level buffer providing more flexibility (and
"bprint" is used as a verb in the doxy, which might be misleading).
Note that both signatures are mimicing the standard C library
convention:
memcpy(dst, dst_size, src)
which in turn is a mnemonics for:
dst = src
meaning that we are copying data from src to dst.
You might think that in fact you are operating on a context (the dst
buffer or the AVBPrint struct), but you don't need to introduce the
concept of context for these simple functions.
___
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 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.
___
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/4] lavu: Clarify relationship between AVClass, AVOption and context
On date Wednesday 2024-05-15 16:54:20 +0100, Andrew Sayers wrote:
> ---
> libavutil/log.h | 11 ---
> libavutil/opt.h | 7 ---
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/libavutil/log.h b/libavutil/log.h
> index ab7ceabe22..cfbf416679 100644
> --- a/libavutil/log.h
> +++ b/libavutil/log.h
> @@ -59,9 +59,14 @@ 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.).
> + * Metadata about an arbitrary data structure
The previous definition was circular, while this one is lacking
information (for example it is neglecting the logging facilities
provided by AVClass).
My take:
Provide access to generic logging and introspection facilities, used to
describe a class of structs.
An AVClass should be defined as the first element of the struct using
the AVClass facilities, so that functions operating on the AVClass
will work by providing the pointer to the structure.
Note: we might move the AVClass definition to its own file class.h.
> + *
> + * A @ref md_doc_2context "context struct" whose first member is a pointer
> + * to an AVClass object is called an "AVClass context structure"
> + * (e.g. AVCodecContext, AVFormatContext etc.).
> + *
> + * AVClass is often combined with @ref avoptions "AVOptions" to create
> + * "AVOptions-enabled structs" that can be easily configured by users.
> */
> typedef struct AVClass {
> /**
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 07e27a9208..e314e134c2 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -39,9 +39,10 @@
> * @defgroup avoptions AVOptions
> * @ingroup lavu_data
> * @{
> - * AVOptions provide a generic system to declare options on arbitrary structs
> - * ("objects").
This should be kept. In addition you can mention that it requires the
first element of the struct to be an AVClass (which is already
mentioned later in the text).
> An option can have a help text, a type and a range of possible
> - * values. Options may then be enumerated, read and written to.
> + * Builds on AVClass, adding a generic system to declare options.
> + *
> + * An option can have a help text, a type and a range of possible values.
> + * Options may then 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
[...]
___
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
Sorry for the slow reply.
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).
> 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.
> +
> +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.
> +For example, most callback functions can be understood to use contexts:
> +
> +```c
> +struct MyStruct {
> + int counter;
> +};
> +
> +void my_callback( void *my_var_ ) {
> + // my_var provides context for the callback function:
> + struct MyStruct *my_var = (struct MyStruct *)my_var_;
> + printf("Called %d time(s)", ++my_var->counter);
> +}
> +
> +void init() {
> + struct MyStruct my_var;
> + my_var.counter = 0;
> + register_callback( my_callback, &my_var );
style: fun(my_callback, ...) (so spaces around parentheses) here and
below
> +}
> +```
> +
> +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.
> +
> +## “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 CallbackContext {
> + int counter;
> +};
> +
> +void my_callback( void *ctx_ ) {
> + // ctx provides context for the callback function:
> + struct CallbackContext *ctx = (struct CallbackContext *)ctx_;
> + printf("Called %d time(s)", ++ctx->counter);
> +}
> +
> +void init() {
> + struct CallbackContext ctx;
> + ctx.counter = 0;
> + register_callback( my_callback, &ctx );
> +}
> +```
> +
> +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, imagine someone modified `MyStruct` in the earlier example
> +to count several unrelated events across the whole program. That would mean
> +it contained information about multiple entities, so was not a context.
> +But nobody ever *said* it was a context, so that isn't necessarily wrong.
> +However, proposing the same change to the `CallbackContext` in the later
> example
> +would violate a guarantee, and should be pointed out in a code review.
> +
I'm not very convinced by the callback example. The use of contexts in
the FFmpeg API is very much simpler, it is used to keep
Re: [FFmpeg-devel] [PATCH v3 1/3] doc: Explain what "context" means
On date Monday 2024-04-29 10:10:35 +0100, Andrew Sayers wrote:
> On Mon, Apr 22, 2024 at 07:05:12PM +0200, Stefano Sabatini wrote:
[...]
> > I don't have a strong opinion, but I'd probably focus on providing a
> > typical example of a common API (check doc/examples). Also I see here
> > there is a strong focus on OOP, this might be counter-productive in
> > case the reader is not familiar with OOP terminology.
> >
> > OTOH the content might be useful for readers coming from an OOP
> > background and terminology. I wonder if this content might be isolated
> > in a dedicated section, so that non-OOP readers can simply skip it.
> >
> > But this is not a strong objection, and can be possibly reworked in a
> > later step.
> >
>
> This is really a document for FFmpeg newbies, so we need to assume as
> little prior knowledge as possible. After a few days to think it
> over, I think we should avoid assuming...
>
> Knowledge of object-oriented programming. For example, this should be
> useful to a research mathematician with a project that involves codec
> algorithms. So the next draft should feel less like "FFmpeg for OOP
> devs" and more like "FFmpeg for newbies (with some optional OOP
> background reading)".
>
> Knowing that programming doesn't *have* to be object-oriented.
> OOP has become so ubiquitous nowadays, there are plenty of programmers
> who will insist everything is OOP if you just speak loudly and slowly.
> This is a harder problem in some ways, because someone who doesn't
> understand can always re-read until they do, while someone who jumps
> to the wrong conclusion will just keep reading and try to make things
> fit their assumption (e.g. my earlier messages in this thread!).
> So the "how it differs from OOP" stuff needs to stay fairly prominent.
>
> Knowing anything about FFmpeg (or multimedia in general). I like the
> idea of tweaking `doc/examples` to better introduce FFmpeg
> fundamentals, but explaining "context" is a steep enough learning
> curve on its own - introducing concepts like "stream" and "codec" at
> the same time seems like too much.
But even if you show the API that does not mean you need to explain
it entirely, you only need to highligth the structural relationships:
// create an instance context, whatever it is
c = avcodec_alloc_context3(codec);
if (!c) {
fprintf(stderr, "Could not allocate video codec context\n");
exit(1);
}
// set context parametres directly
c->bit_rate = 40;
/* resolution must be a multiple of two */
c->width = 352;
c->height = 288;
/* frames per second */
c->time_base = (AVRational){1, 25};
c->framerate = (AVRational){25, 1};
// use av_opt API to set the options?
...
// open the codec context provided a codec
ret = avcodec_open2(c, codec, NULL);
if (ret < 0) {
fprintf(stderr, "Could not open codec: %s\n", av_err2str(ret));
exit(1);
}
You might even replace avcodec_ with fooblargh_ and get the same
effect, with the addition that with avcodec_ you are already
familiarizing the user with the concrete API rather than with an
hypotetical one.
[...]
> 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);
Also, you are assuming that all the function should have a
context. That's not the case, as you don't always need to keep track
of a "context" when performing operations.
>
> av_register_bitstream_filter() (deprecated 4.0, removed 5.0) took an
> `AVBitStreamFilter *` as its first argument, but I don't think you'd say
> the argument provided "context" for the function. So would I be right in
> saying `AVBitStreamFilter *` is not a context, despite looking like one?
This was finally dropped so this is even missing. But again, it seems
you are assum
Re: [FFmpeg-devel] G729 encoding
On date Monday 2024-04-29 09:37:34 +, Marco Garbin wrote: > Hi to all, > when will be available the g729 encoding features ? As soon as someone is going to implement and submit it. As a start, you might create a ticket to track its status. Also, you might consider sponsoring a developer/company to provide 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".
Re: [FFmpeg-devel] [PATCH] fftools/ffprobe: Avoid overflow when calculating DAR
On date Friday 2024-05-03 17:36:23 +0100, Derek Buitenhuis wrote:
> Both the codecpar's width and height, and the SAR num and den are
> ints, which can overflow. Cast to int64_t, which is what av_reduce
> takes.
>
> Without this, occasionally, display_aspect_ratio can be negative in
> ffprobe's -show_stream output.
>
> Signed-off-by: Derek Buitenhuis
> ---
> fftools/ffprobe.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index 0d4cd0b048..5b40dad527 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -3324,8 +3324,8 @@ static int show_stream(WriterContext *w,
> AVFormatContext *fmt_ctx, int stream_id
> if (sar.num) {
> print_q("sample_aspect_ratio", sar, ':');
> av_reduce(&dar.num, &dar.den,
> - par->width * sar.num,
> - par->height * sar.den,
> + (int64_t) par->width * sar.num,
> + (int64_t) par->height * sar.den,
>1024*1024);
> print_q("display_aspect_ratio", dar, ':');
> } else {
LGTM, thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [REFUND-REQUEST] Vulkan F2F travel
On date Saturday 2024-04-27 14:16:36 +0200, Lynne wrote: > Hi, > > I'm requesting a reimbursement for attending the Khronos F2F > event in Brussels on 2024-04-26, where I gave a talk about the > current status of Vulkan Video integration into FFmpeg: > - Currently implemented decoding features > - What we would like to be improved (stability, speed and testing from all > vendors) > - Currently implemented WIP encoding features > - What we would need to implement in order to match nvenc in terms of > quality on most vendors > > Also, I collaborated on future extensions to the standard, > and discussed AV1 issues recently found in the specifications > and how we could fix them. > > 95 GBP: Eurostar to Bruxelle-Midi > 95 GBP: Eurostar from Bruxelle-Midi > Total: 190 GBP Looks good to me, thanks and sorry for the delay. ___ 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: Clarify that AVOptions is not indended for general use
Il mar 23 apr 2024, 13:18 Michael Niedermayer ha scritto: > On Tue, Apr 23, 2024 at 01:15:52PM +0200, Michael Niedermayer wrote: > > On Tue, Apr 23, 2024 at 11:10:43AM +0100, Andrew Sayers wrote: > > > On Tue, Apr 23, 2024 at 12:04:34PM +0200, Anton Khirnov wrote: > > > > Quoting Andrew Sayers (2024-04-23 11:51:00) > > > > > On Tue, Apr 23, 2024 at 11:21:27AM +0200, Anton Khirnov wrote: > > > > > > > lavu/opt: Clarify that AVOptions is not indended for general > use > > > > > > > > > > > > They _are_ intended for general use though. > > > > > > > > > > In that case I'm confused... > > > > > > [...] > > still you certainly can handle binary data (like a bitmap picture) > through > > AVOption > > And if you disagree, which you probably do :) > send a patch to improve AVOption to cover more general use > I think the real point is not that AVOptions/AVClass cannot be used in a generic application, but that using them is not the point of employing libav* libraries. In fact, if only part of your application is about multimedia, probably you will be using the encoding or muxing or filtering API but it's unlikely you will use AVOptions for generic non-multimedia code, and you will be already using some other generic toolkit for handling struct properties. This entails that practically AVOptions/AVClass is mostly used to develop FFmpeg internals. So even if the AVOptions API is generic, its use is not really the selling point of the FFmpeg libraries, and therefore the user is not really *expected* to use directly them to extend his generic structs, even if that might be possible. > ___ 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 date Monday 2024-04-22 16:56:48 +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 | 276 +
> 1 file changed, 276 insertions(+)
> create mode 100644 doc/context.md
>
> diff --git a/doc/context.md b/doc/context.md
> new file mode 100644
> index 00..73caacf54f
> --- /dev/null
> +++ b/doc/context.md
> @@ -0,0 +1,276 @@
> +# Context-oriented programming
> +
> +Like many C projects, FFmpeg has adopted the subset of object-oriented
> techniques
> +that help solve its problems. Object-like structures are called "contexts",
> +and this document provides a general introduction to how they work.
> +
> +Object-oriented programming usually focusses on *access* as a primary
> concern.
> +For example, members of an object are visibly designated "private",
> "constant"
> +etc. to control how they are accessed. *Reflection* is a secondary concern,
> +where it is provided at all. For example, C++ has no built-in way to get a
> +string containing the name of a variable.
> +
> +Reflection is extremely important for FFmpeg, because user-facing options are
> +implemented by reflecting state at runtime. Limiting access is a secondary
> +concern, mainly important for ensuring implementation details can change
> +between versions.
> +
> +An object-oriented programmer learning FFmpeg should be careful not to
> fixate on
> +FFmpeg's access control features, nor to overlook its reflection
> capabilities.
> +Both are present, but have been given the level of focus appropriate for the
> task.
> +
> +## Example: modify text then print it
> +
> +The example below shows a context structure that receives input strings,
> +modifies them in some context-dependant way, then prints them to a specified
> +filehandle.
> +
> +```c
> +/**
> + * Type information, accessible at runtime.
> + *
> + * Useful when configuring objects.
> + */
> +enum ModifyThenPrintDialect {
> +MODIFY_THEN_PRINT_PLAIN_TEXT = 0,
> +MODIFY_THEN_PRINT_REGEX = 1,
> +MODIFY_THEN_PRINT_REGEX_PCRE = 2
> +};
> +
> +/**
> + * User-facing information about types
> + *
> + * Useful for describing contexts to the user.
> + */
> +static const char* ModifyThenPrintDialectName[] = {
> +"plain text",
> +"regular expression",
> +"Perl-compatible regular expression"
> +};
> +
> +/**
> + * Context for functions that modify strings before printing them
> + */
> +struct ModifyThenPrintContext {
> +
> +/**
> + * Information about the type of this particular instance
> + *
> + * Object-oriented programs would probably represent this example
> + * as a sub-class, but some instance-specific functionality
> + * behaves more like a mixin.
> + */
> +enum ModifyThenPrintDialect dialect;
> +
> +/**
> + * Internal context
> + *
> + * Object-oriented programs would put private members in here,
> + * but could also use it to store a virtual function table
> + * or other "invisible" information.
> + */
> +void* priv_data;
style:
type *name;
here and below
> +
> +/**
> + * User-configurable options
> + *
> + * Best set through an API, but can be set directly if necessary
> + *
> + * Data from users needs to be validated before it's set, and the API
> + * might e.g. want to update some internal buffer when these change.
> + * Setting this directly is always less robust than using an API,
> + * but might be worthwhile if you're willing to spend the time checking
> + * for edge cases, and don't mind your code misbehaving if future
> + * versions change the API but not the structure.
> + *
> + * Object-oriented programs would likely make these protected members,
> + * initialised in a constructor and accessed with getters and setters.
> + * Making them user-configurable would be left to the programmer.
> + */
> +char *replace_this;
> +char *with_this;
> +
> +/**
> + * Programmer-configurable variable
> + *
> + * Object-oriented programs would represent this as a public member.
> + */
> +FILE *out;
> +
> +};
> +
> +/**
> + * Allocate and initialize a ModifyThenPrintContext
> + *
> + * This creates a new pointer, then fills in some sensible defaults.
> + *
> + * We can reasonably assume this function will initialise `priv_data`
> + * with a dialect-
Re: [FFmpeg-devel] [PATCH 8/6] doc/protocols: Fill in missing HTTP options
On date Monday 2024-04-22 15:26:24 +0100, Derek Buitenhuis wrote:
> On 4/16/2024 6:13 PM, Stefano Sabatini wrote:
> >> +@item metadata
> >> +An exported dictionary containing Icecast metadata from the bitstream, if
> >> present.
> >> +Only useful with the C API.
> >
> > Probably best to use impersonal verbal mode:
> > Set an exported ...
>
> This is not quite right. This is not a user-settable option, but exported
> data from http.c
> to be read by the user.
Correct, so this is "abusing" AVOption to export a property.
Possibly this might be changed to:
@item metadata (@emph{read only})
Contain a dictionary with Icecast metadata from the bitstream, if present.
...
[...]
I'm fine with it either way, 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 2/3] lavf/mkvtimestamp_v2: review implementation to match mkvextract behavior
On date Saturday 2024-04-20 18:47:58 +0200, Andreas Rheinhardt wrote:
> Stefano Sabatini:
[...]
> >> 1. This does not match mkvextract behaviour. mkvextract does not force a
> >> 1ms timebase.
> >
> > From your past comment:
> >> The accuracy of the timestamps output by mkvextract is determined by the
> >> TimestampScale of the file in question; it is most often 1ms when the
> >> file has video.
> >
>
> "most often" != "force"
[...]
> (I am not certain wrt MKVToolNix handling of fractional millisecond; old
> versions of mkvextract may really have simply rounded/truncated to
> milliseconds.)
It doesn't, at least with version:
$ mkvextract --version
mkvextract v65.0.0 ('Too Much') 64-bit
So far, mkvextract seems to output DTSs as in the original
implementation - therefore no need to change implementation, but for
the "timecode" => "timestamp" issue.
$ ./ffprobe -hide_banner slow.mkv -of csv=nk=1:p=0 -select_streams v:0
-show_entries packet=pts | head -n 20
[...]
0
1201
1235
1268
1368
1301
1335
1401
1435
1535
1468
1502
1602
1568
1735
1668
1635
1702
1768
1869
$ ./ffprobe -hide_banner slow.mkv -of csv=nk=1:p=0 -select_streams v:0
-show_entries packet=dts | head -n 20
N/A
N/A
0
1201
1235
1268
1301
1335
1368
1401
1435
1468
1502
1535
1568
1602
1635
1668
1702
1735
$ mkvextract slow.mkv timestamps_v2 0:slow.mkv.out
Progress: 100%
$ head -n 21 slow.mkv.out
# timestamp format v2
0
1201
1235
1268
1301
1335
1368
1401
1435
1468
1502
1535
1568
1602
1635
1668
1702
1735
1768
1802
___
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] avformat/httpauth.c Support RFC7616 [Style fixed]
On date Thursday 2024-04-18 12:49:53 +0200, Stefano Sabatini wrote:
> On date Monday 2024-04-15 19:56:48 +0200, Stefano Sabatini wrote:
> > On date Monday 2024-04-15 02:32:14 +, �� | Eugene wrote:
> > > Update digest authentication in httpauth.c
> > >
> > > - Refactor make_digest_auth() to support RFC 2617 and RFC 7617
> > > - Add support for SHA-256 and SHA-512/256 algorithms along with MD5
> > > - MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
> > > - Replace AVMD5 structure with AVHashContext for hash algorithm
> > > flexibility
> > > - Rename update_md5_strings() to update_hash_strings() for consistency
> > > - Address coding style feedback from reviewer:
> > >
> > > This is a feature update focused on digest authentication enhancements.
> > > Some lint issues in the existing code remain unaddressed in this patch.
> > >
> > > Signed-off-by: Eugene-bitsensing
> > > ---
> >
> > > libavformat/httpauth.c | 102 +
> > > 1 file changed, 53 insertions(+), 49 deletions(-)
> >
> > add entry to Changelog?
>
> Updated the patch with a few fixes, please check in attachment.
>
> [...]
> > nit++: to avoid semantic overlap I'd rather name this
> > selected_algorithm
> >
> > > +if (!*algorithm) {
> > > +algorithm = "MD5"; // if empty, use MD5 as Default
> > > +hashing_algorithm = "MD5";
> > > +} else if (av_stristr(algorithm, "MD5")) {
> > > +hashing_algorithm = "MD5";
> > > +} else if (av_stristr(algorithm, "sha256") || av_stristr(algorithm,
> > > "sha-256")) {
> > > +hashing_algorithm = "SHA256";
> > > +len_hash = 65; // SHA-2: 64 characters.
>
> > > +} else if (av_stristr(algorithm, "sha512-256") ||
> > > av_stristr(algorithm, "sha-512-256")) {
> > > +hashing_algorithm = "SHA512_256";
> > > +len_hash = 65; // SHA-2: 64 characters.
>
> I'm concerned by this, as it will never be reached because the "sha256"
> branch is always selected instead, that's why this should be made an
> exact match?
Ping to Eugene. I think it would be safe with the use of stricmp.
___
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: Clarify that AVOptions is not indended for general use
On date Monday 2024-04-22 13:09:25 +0100, Andrew Sayers wrote: > --- > libavutil/opt.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavutil/opt.h b/libavutil/opt.h > index e6013662f6..4c0e7d9223 100644 > --- a/libavutil/opt.h > +++ b/libavutil/opt.h > @@ -54,7 +54,10 @@ > * semantics of those fields without breaking API compatibility. > * > * @section avoptions_implement Implementing AVOptions > + * > * This section describes how to add AVOptions capabilities to a struct. > + * It is aimed at people adding new interfaces to internal FFmpeg > functionality, > + * but may also be of interest to programs that depend on FFmpeg. > * > * All AVOptions-related information is stored in an AVClass. Therefore > * the first member of the struct should be a pointer to an AVClass > describing it. > -- > 2.43.0 Looks good, thanks. Let's wait one day in case there are comments from other people. Note: I'll be offline for one week or so starting from tomorrow, someone else might need to push this - or I'll do it when I'll be back. ___ 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 date Sunday 2024-04-21 22:12:56 -0300, James Almer wrote: > On 4/17/2024 10:58 AM, Michael Niedermayer wrote: [...] > A full rewrite of ffserver, using only public API, and with modern streaming > in mind. It would give a lot of code in lavf some use. If this is going to happen, my advice is to use "ffstream" to stick to the ffVERB convention (with the exeption of ffmpeg, which might still be converted to ffconvert with some proper aliasing) and to avoid association with the old incompatible tool . ___ 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: Clarify that AVOptions is not indended for general use
On date Monday 2024-04-22 09:49:45 +0100, Andrew Sayers wrote: > --- > libavutil/opt.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/libavutil/opt.h b/libavutil/opt.h > index e6013662f6..795accb363 100644 > --- a/libavutil/opt.h > +++ b/libavutil/opt.h > @@ -54,7 +54,11 @@ > * semantics of those fields without breaking API compatibility. > * > * @section avoptions_implement Implementing AVOptions > + * > * This section describes how to add AVOptions capabilities to a struct. > + * It is intended for developers of FFmpeg itself - AVOptions can technically > + * be used as a more general toolkit, but is neither intended nor expected > + * to be good fit for other use cases. > * Nit: I'd make this statement somehow milder: Use of AVOptions is intended for development of FFmpeg itself, but use outside of FFmpeg is also possible. the rationale being that it is very difficult to know in advance the use case - in case the FFmpeg are the ones and only employed libraries (as in the case of the FFmpeg tools themselves) this might even be a good choice if you want to keep a small dependencies footprint and you don't plan to switch to a different multimedia library. Also: developers => development since an FFmpeg developer might work on other libraries as well, so "FFmpeg development" defines the scope more exactly. ___ 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/3] doc: Explain what "context" means
On date Saturday 2024-04-20 23:17:57 +0100, Andrew Sayers wrote: > On Sat, Apr 20, 2024 at 06:48:32PM +0200, Stefano Sabatini wrote: > > On date Saturday 2024-04-20 13:19:41 +0100, Andrew Sayers wrote: > > > Based largely on the explanation by Stefano Sabatini: > > > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325854.html > > > --- > > > doc/jargon.md | 169 ++ > > > 1 file changed, 169 insertions(+) > > > create mode 100644 doc/jargon.md > > > > > > diff --git a/doc/jargon.md b/doc/jargon.md > > > new file mode 100644 > > > index 00..f967b5c8bc > > > --- /dev/null > > > +++ b/doc/jargon.md > > > @@ -0,0 +1,169 @@ > > > +# Jargon > > > + > > > +Terms used throughout the code that developers may need to know. > > > + > > > +@anchor context > > > + > > > > > +## Context > > > + > > > > > +A design pattern that stores the context (e.g. configuration) for a > > > series > > > +of operations in a "context" structure, and moves other information with > > > +a longer or shorter lifetime elsewhere. > > > > I'd skip the mention of a design pattern since this is about the > > jargon. > > > > So a simplified variant would be: > > > > A "context" is a a structure used to store information > > (e.g. configuration and/or internal state) for a series of operations > > working on the same data. > > I think there's a pattern to the problems I'm having in this thread - > *anchoring effects*. > > If you ask someone "is 5 a big number?" then "is 5 thousand a big number?", > they'll probably say "yes" to the second question. But if you ask them > "is 5 billian a big number?" then "is 5 thousand a big number?", they'll > probably say "no". In each case, their concept of "bigness" has been > anchored by the first question you asked. > > When I originally tried to learn FFmpeg back in the day, I got nowhere with my > default OOP mindset. It wasn't until I thought to read the examples with a > procedural mindset that it started making any sense, and I think that has > *anchored* my mental model of FFmpeg to a mindset that made it hard to think > deeply about its object-oriented bits. > > Yesterday I would have agreed this was just one piece of jargon that needed > pinning down. But if other people have similarly mis-anchored themselves, > this question might need to be a bit easier for them to find. > [...] > > About the internal "private" context, this is mostly relevant for > > FFmpeg development, and not really useful for API users (basically > > they don't even need to know about the private data). > > > > For example all they need to know is that for AVCodecContext generic > > options they can set the fields in the context itself, or use > > AVOptions, but they can only use AVOptions for "private" options. > > > > We are not still enforcing the use of AVOption to set all options, > > although we might want in the future. > > I think you're saying that "context structure" is synonymous with "context", > and is FFmpeg's term for a common style of C structure; but that other > projects > might use a different word, or write that style of struct without naming it at > all? Correct, althought this style is pretty common in plain C and there are some commonly used conventions (e.g. the first parameter of the related functions is usually the "context") but there is no common jargon. Examples: https://github.com/freeswitch/sofia-sip/blob/master/libsofia-sip-ua/nua/nua_client.h https://github.com/GNOME/glib/blob/main/gio/gsettings.h https://code.videolan.org/videolan/x264/-/blob/master/x264.h?ref_type=heads The meaning of "context" in FFmpeg maps pretty well on the meaning of the English term (provides the context for a given operation needing to work on the same data and with a changing state). > If so, I'd argue it's important to give people a non-FFmpeg-specific > *anchor*, but that we should expand the later FFmpeg-specific example, so they > have an idea of how it's used around here. > > A quick grep of the source suggests that "private context" is an accepted > synonym for "internal context". And it sounds like it fulfils the same > purpose > as C++ "private" access. If both statements are true, then yes it doesn't > need > to go i
Re: [FFmpeg-devel] [PATCH 2/3] lavf/mkvtimestamp_v2: review implementation to match mkvextract behavior
On date Saturday 2024-04-20 18:47:58 +0200, Andreas Rheinhardt wrote: > Stefano Sabatini: > > On date Saturday 2024-04-20 15:18:39 +0200, Andreas Rheinhardt wrote: > >> Stefano Sabatini: > >>> Harmonize internal implementation with the mkvextract behavior: > >>> - print PTS in place of DTS values > >>> - ignore NOPTS values > >>> - sort PTS values > >>> --- > >>> libavformat/mkvtimestamp_v2.c | 69 +-- > >>> 1 file changed, 65 insertions(+), 4 deletions(-) [...] > >> 3. I still don't think that this muxer should exist at all. > > > > I tend to agree. But: > > > > We don't really know why this weird muxer was added, today we have > > better tools for that (we could use ffprobe, or even a bitstream > > filter similar to showinfo to get the same result), but if it was > > added probably there was a reason. So my plan is to make the format a > > bit more useful (DTS => PTS+sort), and possibly deprecate it and point > > to better available tools and drop this in two major releses. > > > > I don't think the point of the format was to really make the behavior > > exactly equal to mkvextract, the thing with TimeScale is probably not > > very important, at least for the original author that was not really > > an issue, he was probably only looking for some way to dump timestamps > > and took free inspiration from mkvtoolnix. > > > > This thing has been added in 1c5670dbb204369477ee1b5d967f9e8b4f4a33b8. I > can't find any discussion in the mailing list archives for this, but the > file description was "extract pts as timecode v2, as defined by > mkvtoolnix" at the time. Furthermore, its author is the one who started > the Matroska muxer. So I think its aim was really to mimic mkvextract. > (I am not certain wrt MKVToolNix handling of fractional millisecond; old > versions of mkvextract may really have simply rounded/truncated to > milliseconds.) > > > If this is true, we might point that this format is not exactly > > equivalent to timestamp_v2 in the doc. > > Which makes this thing even more pointless. I'm not against removing this muxer, but it is something we should do? Removing a working component (even if suboptimal) even without deprecation. Probably if there is agreement about this, especially given that there are better alternatives at this time. If not, I can fix the missing bits so we have a better implementation, but we might want to deprecated and drop later. ___ 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/3] doc: Explain what "context" means
On date Saturday 2024-04-20 13:19:41 +0100, Andrew Sayers wrote:
> Based largely on the explanation by Stefano Sabatini:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325854.html
> ---
> doc/jargon.md | 169 ++
> 1 file changed, 169 insertions(+)
> create mode 100644 doc/jargon.md
>
> diff --git a/doc/jargon.md b/doc/jargon.md
> new file mode 100644
> index 00..f967b5c8bc
> --- /dev/null
> +++ b/doc/jargon.md
> @@ -0,0 +1,169 @@
> +# Jargon
> +
> +Terms used throughout the code that developers may need to know.
> +
> +@anchor context
> +
> +## Context
> +
> +A design pattern that stores the context (e.g. configuration) for a series
> +of operations in a "context" structure, and moves other information with
> +a longer or shorter lifetime elsewhere.
I'd skip the mention of a design pattern since this is about the
jargon.
So a simplified variant would be:
A "context" is a a structure used to store information
(e.g. configuration and/or internal state) for a series of operations
working on the same data.
> +
> +Consider a code snippet to modify text then print it:
> +
> +```c
> +/**
> + * Contextual information about printing a series of messages
> + */
> +struct ModifyThenPrintContext {
> +
> +/**
> + * Members of the context usually are usually part of its public API...
> + */
> +FILE *out;
> +
> +/**
> + * ... but check the documentation just in case
> + */
> +[[deprecated]]
> +int no_longer_part_of_the_public_api;
> +
> +/**
> + * The "internal context" is private to the context itself.
> + *
> + * Unlike the members above, the private context is not guaranteed
> + * and can change arbitrarily between versions.
> + */
> +void* priv_data;
> +};
> +
> +/**
> + * Long-lifetime information, reused by many contexts
> + */
> +enum ModifyThenPrintDialect {
> +MODIFY_THEN_PRINT_PLAIN_TEXT,
> +MODIFY_THEN_PRINT_REGEX,
> +MODIFY_THEN_PRINT_REGEX_PCRE
> +};
> +
> +/**
> + * Short-lifetime information, used repeatedly in a single context
> + */
> +struct ModifyThenPrintMessage {
> +char *str;
> +char *replace_this;
> +char *with_this;
> +};
> +
> +/**
> + * Allocate and initialize a ModifyThenPrintContext
> + *
> + * This creates a new pointer, then fills in some sensible defaults.
> + *
> + * We can reasonably assume this function will initialise `priv_data`
> + * with a dialect-specific object, but shouldn't make any assumptions
> + * about what that object is.
> + *
> + */
> +int ModifyThenPrintContext_alloc_context(struct ModifyThenPrintContext **ctx,
> + FILE *out,
> + enum ModifyThenPrintDialect
> dialect);
> +
> +/**
> + * Uninitialize and deallocate a ModifyThenPrintContext
> + *
> + * This does any work required by the private context in `priv_data`
> + * (e.g. deallocating it), then deallocates the main context itself.
> + *
> + */
> +int ModifyThenPrintContext_free(struct ModifyThenPrintContext *ctx);
> +
> +/**
> + * Print a single message
> + */
> +int ModifyThenPrintContext_print(struct ModifyThenPrintContext *ctx,
> + struct ModifyThenPrintMessage *msg);
> +
> +int print_hello_world()
> +{
> +
> +int ret = 0;
> +
> +struct ModifyThenPrintContext *ctx;
> +
> +struct ModifyThenPrintMessage hello_world;
> +
> +if ( ModifyThenPrintContext_alloc_context( &ctx, stdout,
> MODIFY_THEN_PRINT_REGEX ) < 0 ) {
> +ret = -1;
> +goto EXIT_WITHOUT_CLEANUP;
> +}
> +
> +hello_world.replace_this = "Hi|Hullo";
> +hello_world.with_this= "Hello";
> +
> +hello_world.str = "Hi, world!\n";
> +if ( ModifyThenPrintContext_print( ctx, &hello_world ) < 0 ) {
> +ret = -1;
> +goto FINISH;
> +}
> +
> +hello_world.str = "Hullo, world!\n";
> +if ( ModifyThenPrintContext_print( ctx, &hello_world ) < 0 ) {
> +ret = -1;
> +goto FINISH;
> +}
> +
> +FINISH:
> +if ( ModifyThenPrintContext_free( ctx ) ) {
> +ret = -1;
> +goto EXIT_WITHOUT_CLEANUP;
> +}
> +
> +EXIT_WITHOUT_CLEANUP:
> +return ret;
> +
> +}
> +```
> +
> +In the example above, the `ModifyThenPrintContext` object contains
> information
> +that's needed for exactly the lifetime of the current job (i.e. how to modify
> +and where
Re: [FFmpeg-devel] [PATCH 1/3] doc: Explain what "context" means
On date Saturday 2024-04-20 13:18:29 +0100, Andrew Sayers wrote: > On 20/04/2024 08:25, Stefano Sabatini wrote: > > On date Thursday 2024-04-18 16:06:12 +0100, Andrew Sayers wrote: > > > Based largely on the explanation by Stefano Sabatini: > > > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325854.html > > > --- > > > doc/jargon.md | 96 +++ > > > 1 file changed, 96 insertions(+) > > > create mode 100644 doc/jargon.md > > > > > > diff --git a/doc/jargon.md b/doc/jargon.md > > > new file mode 100644 > > > index 00..3b78ffb61f > > > --- /dev/null > > > +++ b/doc/jargon.md > > We currently have a single .md file in doc (for historical reason we > > still stick to texinfo). Also how is this integrated into doxygen? > > Doxygen automatically renders all /*.md and /doc/*.md files to pages at [1] > which is the only place I'd know to look for this sort of thing. it seems > like texinfo is more for man pages etc., which would be hard to link from > doxygen? By the way, a file called "jargon" seemed like a better idea than > e.g. a "design_patterns" file or a section in AVClass. I've rewritten the > document completely based on your feedback - same markdown file for now, > but happy to move/reformat. If this is the case, probably we should move the files to a dedicated directory (doxygen?) to avoid to mix too many things in the same bundle (can be done as a separated patch though since we might think about what we really want to include in the doxygen docs). > The points below should be addressed by the new patch, so I'll let that > speak for itself. But there's a general issue that's worth mentioning... > > Technically, it sounds like a more accurate description would be "Context > is an FFmpeg convention that has become fairly rigorous over the years". > But IMHO readers would uncharitably read that as "Context is some weird > FFmpeg thing they're stuck with because they picked a pre-OOP language". > Arguing "Context is a design pattern that groups objects by lifespan" > emphasises the lessons a newbie can take from FFmpeg and use elsewhere, > so they get more value from the time they spent reading the document. > I've tried to write the document to start with the more useful argument, > then gradually ease in to the more accurate one. But, I don't think this is really something very rigourous, because it is used in different "contexts" with sligthly different features (e.g. you can have a structure named "context" but without an AVClass or options). And it's not really about lifespan, and not really specific for FFmpeg, in C programming this is used to provide a "context" for several "methods" operating on a given struct - so basically a light implementation of an object-oriented-based 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 2/3] lavf/mkvtimestamp_v2: review implementation to match mkvextract behavior
On date Saturday 2024-04-20 15:18:39 +0200, Andreas Rheinhardt wrote:
> Stefano Sabatini:
> > Harmonize internal implementation with the mkvextract behavior:
> > - print PTS in place of DTS values
> > - ignore NOPTS values
> > - sort PTS values
> > ---
> > libavformat/mkvtimestamp_v2.c | 69 +--
> > 1 file changed, 65 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/mkvtimestamp_v2.c b/libavformat/mkvtimestamp_v2.c
> > index 1eb2daf10a..c6446ed489 100644
> > --- a/libavformat/mkvtimestamp_v2.c
> > +++ b/libavformat/mkvtimestamp_v2.c
> > @@ -22,30 +22,91 @@
> > #include "avformat.h"
> > #include "internal.h"
> > #include "mux.h"
> > +#include "libavutil/qsort.h"
> > +
> > +#define PTSS_MAX_SIZE 128
> > +#define PTSS_HALF_SIZE (PTSS_MAX_SIZE >> 1)
> > +
> > +struct MkvTimestampContext {
> > +int64_t ptss[PTSS_MAX_SIZE];
> > +size_t ptss_size;
> > +};
> >
> > static int write_header(AVFormatContext *s)
> > {
> > -static const char *header = "# timecode format v2\n";
> > +static const char *header = "# timestamp format v2\n";
> > avio_write(s->pb, header, strlen(header));
> > avpriv_set_pts_info(s->streams[0], 64, 1, 1000);
> > +
> > return 0;
> > }
> >
> > +static int cmp_int64(const void *p1, const void *p2)
> > +{
> > +int64_t left = *(const int64_t *)p1;
> > +int64_t right = *(const int64_t *)p2;
> > +return FFDIFFSIGN(left, right);
> > +}
> > +
> > static int write_packet(AVFormatContext *s, AVPacket *pkt)
> > {
> > char buf[256];
> > +int i;
> > +struct MkvTimestampContext *m = s->priv_data;
> > +
> > if (pkt->stream_index)
> > av_log(s, AV_LOG_WARNING, "More than one stream unsupported\n");
> > -snprintf(buf, sizeof(buf), "%" PRId64 "\n", pkt->dts);
> > -avio_write(s->pb, buf, strlen(buf));
> > +
> > +if (pkt->pts == AV_NOPTS_VALUE) {
> > +av_log(s, AV_LOG_WARNING, "Found PTS with no value, ignored\n");
> > +return 0;
> > +}
> > +
> > +if (m->ptss_size > PTSS_MAX_SIZE) {
> > +// sort all PTSs
> > +AV_QSORT(m->ptss, PTSS_MAX_SIZE, int64_t, cmp_int64);
> > +
> > +// write only the first half and copy the second half to the
> > +// beginning of the array
> > +for (i = 0; i < PTSS_HALF_SIZE; i++) {
> > +snprintf(buf, sizeof(buf), "%" PRId64 "\n", m->ptss[i]);
> > +avio_write(s->pb, buf, strlen(buf));
> > +m->ptss[i] = m->ptss[i + PTSS_HALF_SIZE];
> > +}
> > +
> > +m->ptss_size = PTSS_HALF_SIZE;
> > +} else {
> > +m->ptss[m->ptss_size++] = pkt->pts;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int write_trailer(struct AVFormatContext *s)
> > +{
> > +struct MkvTimestampContext *m = s->priv_data;
> > +char buf[256];
> > +int i;
> > +
> > +// sort all PTSs
> > +AV_QSORT(m->ptss, m->ptss_size, int64_t, cmp_int64);
> > +
> > +/* flush remaining timestamps */
> > +for (i = 0; i < m->ptss_size; i++) {
> > +snprintf(buf, sizeof(buf), "%" PRId64 "\n", m->ptss[i]);
> > +avio_write(s->pb, buf, strlen(buf));
> > +}
> > +
> > return 0;
> > }
> >
> > const FFOutputFormat ff_mkvtimestamp_v2_muxer = {
> > .p.name= "mkvtimestamp_v2",
> > -.p.long_name = NULL_IF_CONFIG_SMALL("mkvtoolnix v2 timecode format"),
> > +.p.long_name = NULL_IF_CONFIG_SMALL("mkvtoolnix v2 timestamp
> > format"),
> > .p.audio_codec = AV_CODEC_ID_NONE,
> > .p.video_codec = AV_CODEC_ID_RAWVIDEO,
> > .write_header = write_header,
> > .write_packet = write_packet,
> > +.write_trailer = write_trailer,
> > +.priv_data_size = sizeof(struct MkvTimestampContext),
> > };
>
> 1. This does not match mkvextract behaviour. mkvextract does not force a
> 1ms timebase.
>From your past comment:
>The accuracy of the timestamps output by mkvextract is determined by the
>TimestampScale of the file in question; it is most often 1ms when the
>file has video.
Note
Re: [FFmpeg-devel] [PATCH 3/3] doc/muxers: add mkvtimestamp_v2
On date Saturday 2024-04-20 13:48:35 +0200, Stefano Sabatini wrote: > --- > doc/muxers.texi | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) Sorry, discard this in favor of the new patch. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] doc/muxers: add mkvtimestamp_v2
---
doc/muxers.texi | 8
1 file changed, 8 insertions(+)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index 6340c8e54d..e1d6a0e557 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -2933,6 +2933,14 @@ MicroDVD subtitle format muxer.
This muxer accepts a single @samp{microdvd} subtitles stream.
+@section mkvtimestamp_v2
+mkvtoolnix v2 timestamp format muxer.
+
+Write the PTS rawvideo frame to the output, as implemented by the
+@command{mkvextract} tool from the @command{mkvtoolnix} suite.
+
+This muxer accepts a single @samp{rawvideo} stream.
+
@section mmf
Synthetic music Mobile Application Format (SMAF) format muxer.
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] lavf/mkvtimestamp_v2: use name in place of description in long name
---
libavformat/mkvtimestamp_v2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/mkvtimestamp_v2.c b/libavformat/mkvtimestamp_v2.c
index dde431ab7d..1eb2daf10a 100644
--- a/libavformat/mkvtimestamp_v2.c
+++ b/libavformat/mkvtimestamp_v2.c
@@ -43,7 +43,7 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
const FFOutputFormat ff_mkvtimestamp_v2_muxer = {
.p.name= "mkvtimestamp_v2",
-.p.long_name = NULL_IF_CONFIG_SMALL("extract pts as timecode v2 format,
as defined by mkvtoolnix"),
+.p.long_name = NULL_IF_CONFIG_SMALL("mkvtoolnix v2 timecode format"),
.p.audio_codec = AV_CODEC_ID_NONE,
.p.video_codec = AV_CODEC_ID_RAWVIDEO,
.write_header = write_header,
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3] doc/muxers: add mkvtimestamp_v2
---
doc/muxers.texi | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index 6340c8e54d..4cd53a4449 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -2933,14 +2933,13 @@ MicroDVD subtitle format muxer.
This muxer accepts a single @samp{microdvd} subtitles stream.
-@section mmf
-Synthetic music Mobile Application Format (SMAF) format muxer.
+@section mkvtimestamp_v2
+mkvtoolnix v2 timestamp format muxer.
-SMAF is a music data format specified by Yamaha for portable
-electronic devices, such as mobile phones and personal digital
-assistants.
+Write the PTS rawvideo frame to the output, as implemented by the
+@command{mkvextract} tool from the @command{mkvtoolnix} suite.
-This muxer accepts a single @samp{adpcm_yamaha} audio stream.
+This muxer accepts a single @samp{rawvideo} stream.
@section mp3
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] lavf/mkvtimestamp_v2: review implementation to match mkvextract behavior
Harmonize internal implementation with the mkvextract behavior:
- print PTS in place of DTS values
- ignore NOPTS values
- sort PTS values
---
libavformat/mkvtimestamp_v2.c | 69 +--
1 file changed, 65 insertions(+), 4 deletions(-)
diff --git a/libavformat/mkvtimestamp_v2.c b/libavformat/mkvtimestamp_v2.c
index 1eb2daf10a..c6446ed489 100644
--- a/libavformat/mkvtimestamp_v2.c
+++ b/libavformat/mkvtimestamp_v2.c
@@ -22,30 +22,91 @@
#include "avformat.h"
#include "internal.h"
#include "mux.h"
+#include "libavutil/qsort.h"
+
+#define PTSS_MAX_SIZE 128
+#define PTSS_HALF_SIZE (PTSS_MAX_SIZE >> 1)
+
+struct MkvTimestampContext {
+int64_t ptss[PTSS_MAX_SIZE];
+size_t ptss_size;
+};
static int write_header(AVFormatContext *s)
{
-static const char *header = "# timecode format v2\n";
+static const char *header = "# timestamp format v2\n";
avio_write(s->pb, header, strlen(header));
avpriv_set_pts_info(s->streams[0], 64, 1, 1000);
+
return 0;
}
+static int cmp_int64(const void *p1, const void *p2)
+{
+int64_t left = *(const int64_t *)p1;
+int64_t right = *(const int64_t *)p2;
+return FFDIFFSIGN(left, right);
+}
+
static int write_packet(AVFormatContext *s, AVPacket *pkt)
{
char buf[256];
+int i;
+struct MkvTimestampContext *m = s->priv_data;
+
if (pkt->stream_index)
av_log(s, AV_LOG_WARNING, "More than one stream unsupported\n");
-snprintf(buf, sizeof(buf), "%" PRId64 "\n", pkt->dts);
-avio_write(s->pb, buf, strlen(buf));
+
+if (pkt->pts == AV_NOPTS_VALUE) {
+av_log(s, AV_LOG_WARNING, "Found PTS with no value, ignored\n");
+return 0;
+}
+
+if (m->ptss_size > PTSS_MAX_SIZE) {
+// sort all PTSs
+AV_QSORT(m->ptss, PTSS_MAX_SIZE, int64_t, cmp_int64);
+
+// write only the first half and copy the second half to the
+// beginning of the array
+for (i = 0; i < PTSS_HALF_SIZE; i++) {
+snprintf(buf, sizeof(buf), "%" PRId64 "\n", m->ptss[i]);
+avio_write(s->pb, buf, strlen(buf));
+m->ptss[i] = m->ptss[i + PTSS_HALF_SIZE];
+}
+
+m->ptss_size = PTSS_HALF_SIZE;
+} else {
+m->ptss[m->ptss_size++] = pkt->pts;
+}
+
+return 0;
+}
+
+static int write_trailer(struct AVFormatContext *s)
+{
+struct MkvTimestampContext *m = s->priv_data;
+char buf[256];
+int i;
+
+// sort all PTSs
+AV_QSORT(m->ptss, m->ptss_size, int64_t, cmp_int64);
+
+/* flush remaining timestamps */
+for (i = 0; i < m->ptss_size; i++) {
+snprintf(buf, sizeof(buf), "%" PRId64 "\n", m->ptss[i]);
+avio_write(s->pb, buf, strlen(buf));
+}
+
return 0;
}
const FFOutputFormat ff_mkvtimestamp_v2_muxer = {
.p.name= "mkvtimestamp_v2",
-.p.long_name = NULL_IF_CONFIG_SMALL("mkvtoolnix v2 timecode format"),
+.p.long_name = NULL_IF_CONFIG_SMALL("mkvtoolnix v2 timestamp format"),
.p.audio_codec = AV_CODEC_ID_NONE,
.p.video_codec = AV_CODEC_ID_RAWVIDEO,
.write_header = write_header,
.write_packet = write_packet,
+.write_trailer = write_trailer,
+.priv_data_size = sizeof(struct MkvTimestampContext),
};
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] lavf/mkvtimestamp_v2: review mkvtimestamp_v2 implementation, add documentation
[PATCH 1/3] lavf/mkvtimestamp_v2: use name in place of description in [PATCH 2/3] lavf/mkvtimestamp_v2: review implementation to match [PATCH 3/3] doc/muxers: add mkvtimestamp_v2 ___ 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] avformat/network: add ff_neterrno2() for cases where we already have an errno
On date Friday 2024-04-19 20:07:59 +0100, Andrew Sayers wrote: > For example, WSAStartup()'s documentation says: > > "A call to the WSAGetLastError function is not needed and should not be > used" > --- > libavformat/network.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) missing declaration in network.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] [PATCH] tools: add target_enc_fuzzer.c
On date Saturday 2024-04-20 03:10:37 +0200, Michael Niedermayer wrote:
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer
> ---
> Makefile | 3 +
> tools/Makefile| 3 +
> tools/target_enc_fuzzer.c | 213 ++
> 3 files changed, 219 insertions(+)
> create mode 100644 tools/target_enc_fuzzer.c
>
> diff --git a/Makefile b/Makefile
> index b309dbc4db9..de727cbe00e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -52,6 +52,9 @@ $(TOOLS): %$(EXESUF): %.o
> target_dec_%_fuzzer$(EXESUF): target_dec_%_fuzzer.o $(FF_DEP_LIBS)
> $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $^ $(ELIBS) $(FF_EXTRALIBS)
> $(LIBFUZZER_PATH)
>
> +target_enc_%_fuzzer$(EXESUF): target_enc_%_fuzzer.o $(FF_DEP_LIBS)
> + $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $^ $(ELIBS) $(FF_EXTRALIBS)
> $(LIBFUZZER_PATH)
> +
> tools/target_bsf_%_fuzzer$(EXESUF): tools/target_bsf_%_fuzzer.o
> $(FF_DEP_LIBS)
> $(LD) $(LDFLAGS) $(LDEXEFLAGS) $(LD_O) $^ $(ELIBS) $(FF_EXTRALIBS)
> $(LIBFUZZER_PATH)
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 72e8e709a8d..2a11fa0ae62 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -5,6 +5,9 @@ TOOLS-$(CONFIG_ZLIB) += cws2fws
> tools/target_dec_%_fuzzer.o: tools/target_dec_fuzzer.c
> $(COMPILE_C) -DFFMPEG_DECODER=$*
>
> +tools/target_enc_%_fuzzer.o: tools/target_enc_fuzzer.c
> + $(COMPILE_C) -DFFMPEG_ENCODER=$*
> +
> tools/target_bsf_%_fuzzer.o: tools/target_bsf_fuzzer.c
> $(COMPILE_C) -DFFMPEG_BSF=$*
>
> diff --git a/tools/target_enc_fuzzer.c b/tools/target_enc_fuzzer.c
> new file mode 100644
> index 000..bc9f98c1443
> --- /dev/null
> +++ b/tools/target_enc_fuzzer.c
> @@ -0,0 +1,213 @@
> +/*
> + * Copyright (c) 2024 Michael Niedermayer
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + *
> + * Based on target_dec_fuzzer
> + */
> +
> +#include "config.h"
> +#include "libavutil/avassert.h"
> +#include "libavutil/avstring.h"
> +#include "libavutil/cpu.h"
> +#include "libavutil/imgutils.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/mem.h"
> +
> +#include "libavcodec/avcodec.h"
> +#include "libavcodec/bytestream.h"
> +#include "libavcodec/codec_internal.h"
> +#include "libavformat/avformat.h"
> +
> +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
> +
> +extern const FFCodec * codec_list[];
> +
> +static void error(const char *err)
> +{
> +fprintf(stderr, "%s", err);
> +exit(1);
> +}
> +
> +static const FFCodec *c = NULL;
> +static const FFCodec *AVCodecInitialize(enum AVCodecID codec_id)
nit: snake_case, also the function is used once and the code can be
embedded in the code
[...]
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] doc: Explain what "context" means
On date Thursday 2024-04-18 16:06:12 +0100, Andrew Sayers wrote:
> Based largely on the explanation by Stefano Sabatini:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325854.html
> ---
> doc/jargon.md | 96 +++
> 1 file changed, 96 insertions(+)
> create mode 100644 doc/jargon.md
>
> diff --git a/doc/jargon.md b/doc/jargon.md
> new file mode 100644
> index 00..3b78ffb61f
> --- /dev/null
> +++ b/doc/jargon.md
We currently have a single .md file in doc (for historical reason we
still stick to texinfo). Also how is this integrated into doxygen?
> @@ -0,0 +1,96 @@
> +# Jargon
> +
> +Terms used throughout the code that developers may need to know.
> +
> +@anchor context
> +
> +## Context
> +
> +A design pattern that stores the context (e.g. configuration) for a series
> +of operations in a "context" structure, and moves other information
> elsewhere.
> +
> +Consider a trivial program to print uppercase text:
> +
> +```c
> +/*
> + * Contextual information about where to print a series of messages
> + */
Style:
/**
* Contextual information about where to print a series of messages
*/
> +struct UpperCasePrinterContext {
> +FILE* out;
here and below, use:
VAR *out;
for overall consistency with the project style.
> +};
> +
> +/*
> + * Extra information about messages to print.
> + * This could be used multiple times in a single context,
> + * or reused several times across multiple contexts.
> + */
> +struct PrintInfo {
> +char* str;
> +};
> +
> +void print(
> +struct UpperCasePrinterContext * ctx,
> +struct PrintInfo * info
> +) {
> +for ( char* c = info->str; *c; ++c ) {
> +char C = toupper(*c);
> +fwrite( &C, 1, 1, ctx->out );
> +}
> +}
> +
> +int main()
> +{
> +struct PrintInfo hello, world;
> +struct UpperCasePrinterContext ctx;
> +
> +hello.str = "hello, ";
> +world.str = "world!\n";
> +
> +ctx.out = stdout;
> +
> +print( &ctx, &hello );
> +print( &ctx, &world );
> +
> +return 0;
> +}
I'm not sure this is a fitting example. Usually the context is a
public structure and the internal context (which corresponds to the
PrintInfo struct) is private to the implementation. In this case the
API user is not interested at all at its implmentation.
You can think the context provides the "object"/instance where some
operations are done - this is alike in object oriented programming,
where the context corresponds to the self, so that you create/allocate
the object, initialize it, and invoke operations on the object.
So using this analogy, the example would be:
struct UpperCasePrinterContext {...};
// this corresponds to a "method" defined on the context/object
void uppercase_printer_print(UpperCasePrinterContext *ctx, const char *str);
Or maybe you want to define the property in the context itself, so you
do:
uppercase_ctx.str = "foobar";
then you have:
void uppercase_printer_print(UpperCasePrinterContext *ctx);
On a typical FFmpeg context you typically do (see
doc/examples/encode.c example):
// create the context
c = avcodec_alloc_context3(codec);
// set parameters, either in the context or by using the av_opt API
c->bit_rate = 40;
c->width = 352;
c->height = 288;
c->time_base = (AVRational){1, 25};
c->framerate = (AVRational){25, 1};
// ...
// invoke the methods defined in the context
ret = avcodec_send_frame(enc_ctx, frame);
if (ret < 0) {
fprintf(stderr, "Error sending a frame for encoding\n");
exit(1);
}
while (ret >= 0) {
ret = avcodec_receive_packet(enc_ctx, pkt);
if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
return;
else if (ret < 0) {
fprintf(stderr, "Error during encoding\n");
exit(1);
}
...
av_packet_unref(pkt);
}
[...]
Note also that "context" in the FFmpeg jargon is not very specific
because it might be different depending on the implementation, for
example it might not have an av_opt_* interface (for example
libavutil/hash.h).
> +```
> +
> +The `UpperCasePrinterContext` object contains the information that's about
> +the context of the current job (i.e. printing things to standard output).
I find this confusing, it is essentially saying that the context (the
"object") is the context of the current job.
> +Information with a lifetime different than that of the context is moved
> +to the `PrintInfo` object.
> +
> +FFmpeg's main context structures all happen to face some common problems:
Re: [FFmpeg-devel] [PATCH] avformat/httpauth.c Support RFC7616 [Style fixed]
On date Monday 2024-04-15 19:56:48 +0200, Stefano Sabatini wrote:
> On date Monday 2024-04-15 02:32:14 +, �� | Eugene wrote:
> > Update digest authentication in httpauth.c
> >
> > - Refactor make_digest_auth() to support RFC 2617 and RFC 7617
> > - Add support for SHA-256 and SHA-512/256 algorithms along with MD5
> > - MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
> > - Replace AVMD5 structure with AVHashContext for hash algorithm flexibility
> > - Rename update_md5_strings() to update_hash_strings() for consistency
> > - Address coding style feedback from reviewer:
> >
> > This is a feature update focused on digest authentication enhancements.
> > Some lint issues in the existing code remain unaddressed in this patch.
> >
> > Signed-off-by: Eugene-bitsensing
> > ---
>
> > libavformat/httpauth.c | 102 +
> > 1 file changed, 53 insertions(+), 49 deletions(-)
>
> add entry to Changelog?
Updated the patch with a few fixes, please check in attachment.
[...]
> nit++: to avoid semantic overlap I'd rather name this
> selected_algorithm
>
> > +if (!*algorithm) {
> > +algorithm = "MD5"; // if empty, use MD5 as Default
> > +hashing_algorithm = "MD5";
> > +} else if (av_stristr(algorithm, "MD5")) {
> > +hashing_algorithm = "MD5";
> > +} else if (av_stristr(algorithm, "sha256") || av_stristr(algorithm,
> > "sha-256")) {
> > +hashing_algorithm = "SHA256";
> > +len_hash = 65; // SHA-2: 64 characters.
> > +} else if (av_stristr(algorithm, "sha512-256") ||
> > av_stristr(algorithm, "sha-512-256")) {
> > +hashing_algorithm = "SHA512_256";
> > +len_hash = 65; // SHA-2: 64 characters.
I'm concerned by this, as it will never be reached because the "sha256"
branch is always selected instead, that's why this should be made an
exact match?
>From 2b88a2f64569bb83c0f519983385c35c217bbf9c Mon Sep 17 00:00:00 2001
From: "eug...@bitsensing.com"
Date: Mon, 15 Apr 2024 02:32:14 +
Subject: [PATCH] avformat/httpauth.c: support RFC7616 authentication
- Refactor make_digest_auth() to support RFC 2617 and RFC 7617
- Add support for SHA-256 and SHA-512/256 algorithms along with MD5
- MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
- Replace AVMD5 structure with AVHashContext for hash algorithm flexibility
- Rename update_md5_strings() to update_hash_strings() for consistency
Signed-off-by: Eugene-bitsensing
---
Changelog | 1 +
libavformat/httpauth.c | 104 +
2 files changed, 55 insertions(+), 50 deletions(-)
diff --git a/Changelog b/Changelog
index 8db14f02b4..5db501f8c4 100644
--- a/Changelog
+++ b/Changelog
@@ -7,6 +7,7 @@ version :
- ffmpeg CLI filtergraph chaining
- LC3/LC3plus demuxer and muxer
- pad_vaapi, drawbox_vaapi filters
+- HTTP protocol RFC7617 authentication
version 7.0:
diff --git a/libavformat/httpauth.c b/libavformat/httpauth.c
index 9048362509..90210e6179 100644
--- a/libavformat/httpauth.c
+++ b/libavformat/httpauth.c
@@ -25,7 +25,7 @@
#include "libavutil/mem.h"
#include "internal.h"
#include "libavutil/random_seed.h"
-#include "libavutil/md5.h"
+#include "libavutil/hash.h"
#include "urldecode.h"
static void handle_basic_params(HTTPAuthState *state, const char *key,
@@ -118,36 +118,36 @@ void ff_http_auth_handle_header(HTTPAuthState *state, const char *key,
}
}
-
-static void update_md5_strings(struct AVMD5 *md5ctx, ...)
+static void update_hash_strings(struct AVHashContext *hash_ctx, ...)
{
va_list vl;
-va_start(vl, md5ctx);
+va_start(vl, hash_ctx);
while (1) {
-const char* str = va_arg(vl, const char*);
+const char *str = va_arg(vl, const char*);
if (!str)
break;
-av_md5_update(md5ctx, str, strlen(str));
+av_hash_update(hash_ctx, (const uint8_t *)str, strlen(str));
}
va_end(vl);
}
-/* Generate a digest reply, according to RFC 2617. */
+/* Generate a digest reply, according to RFC 2617/7617 */
static char *make_digest_auth(HTTPAuthState *state, const char *username,
const char *password, const char *uri,
const char *method)
{
DigestParams *digest = &state->digest_params;
-int len;
+size_t len;
uint32_t cnonce_buf[2];
char cnonce[17];
char nc[9];
-int i;
-char A1hash[33], A2hash[33], response[33];
-struct AVMD5 *md5ctx;
-uint8_t hash[16];
+int i, ret;
Re: [FFmpeg-devel] [PATCH 4/6] doc/muxers: add mkvtimestamp_v2
On date Tuesday 2024-04-16 20:09:19 +0200, Andreas Rheinhardt wrote:
> Stefano Sabatini:
> > On date Tuesday 2024-04-16 12:50:19 +0200, Andreas Rheinhardt wrote:
> >> Stefano Sabatini:
> >>> ---
> >>> doc/muxers.texi | 8
> >>> 1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/doc/muxers.texi b/doc/muxers.texi
> >>> index f94513527d..490d5557bf 100644
> >>> --- a/doc/muxers.texi
> >>> +++ b/doc/muxers.texi
> >>> @@ -2933,6 +2933,14 @@ MicroDVD subtitle format muxer.
> >>>
> >>> This muxer accepts a single @samp{microdvd} subtitles stream.
> >>>
> >>> +@section mkvtimestamp_v2
> >>> +mkvtoolnix v2 timecode format muxer.
> >>> +
> >>> +Write the PTS rawvideo frame to the output, as supported by the
> >>> +@command{mkvextact} tool from the @command{mkvtoolnix} suite.
> >>> +
> >>> +This muxer accepts a single @samp{rawvideo} stream.
> >>> +
> >>> @section mp3
> >>>
> >>> The MP3 muxer writes a raw MP3 stream with the following optional
> >>> features:
> >>
> >
> >> This is wrong: MKVToolNix switched to "# timestamp format v2" a long
> >> time ago (we still write the old "# timecode format v2" header);
> >> furthermore, MKVToolNix actually uses pts (which it reorders to be
> >> ascending), not dts like our muxer. Furthermore MKVToolNix does not
> >> force a 1ms precision on timestamps.
> >
> > Correct.
> >
> > I compared the output of the muxer and of mkvtoolnix extract
> > timestamp_v2 and I'm not yet clear about the timestamp differences I'm
> > observing (the muxer output maps with the timestamps, the mkvtoolnix
> > timestamps differ by a few ms). But I think also mkvtoolnix use a 1ms
> > timebase.
>
> The accuracy of the timestamps output by mkvextract is determined by the
> TimestampScale of the file in question; it is most often 1ms when the
> file has video.
> You need to provide more details if you want these discrepancies to be
> analyzed.
Probably not worth the effort.
> > Also, IIRC there is no generic way to reorder PTSs, so this might
> > account for another difference which might be difficult to implement
> > generically.
>
> Write them into a buffer and reorder them at the end?
> (No, I have no intention to actually implement this. I am rather leaning
> to "this muxer should not exist".)
I also think we have better tools at this point (one being ffprobe
-show_packets) but we should not drop it before deprecating it.
Plan: av_tree to insert elements in a constant-size buffer or store in
a buffer sorted once at the end. We probably should skip PTS=NA
elements.
Dropping the doc patch as the implementation is broken.
Will apply the rest of the patchset soon.
___
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 date Wednesday 2024-04-17 15:58:32 +0200, Michael Niedermayer wrote: > Hi all > > The pace of inovation in FFmpeg has been slowing down. > Most work is concentarted nowadays on code refactoring, and adding > support for new codecs and formats. > > Should we > * make a list of longer term goals > * vote on them > * and then together work towards implementing them > ? > > (The idea here is to increase the success of larger efforts > than adding codecs and refactoring code) > It would then also not be possible for individuals to object > to a previously agreed goal. > And it would add ideas for which we can try to get funding/grants for > > (larger scale changes need consensus first that we as a whole want > them before we would be able to ask for funding/grants for them) > > Some ideas and why they would help FFmpeg: > [...] > * client side / in browser support > (expand towards webapps, webpages using ffmpeg client side in the browser) > bring in more users and developers, and it will be costly for us > if we let others take this area as its important and significant There are already several projects on github, the most prominent one: https://github.com/ffmpegwasm/ffmpeg.wasm/ In general it would be useful to provide libav* bindings to other languages, for example: https://github.com/PyAV-Org/PyAV https://github.com/zmwangx/rust-ffmpeg Not sure these should be really moved to FFmpeg though. One option I'm currenly exploring is having a python filter enabling to specify a custom filter implemented in python (needed for custom ad-hoc logic you don't really want to implement in C since it's not generic enough) and to enable using python modules when effiency is not an issue. ___ 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 date Wednesday 2024-04-17 19:21:39 -0700, Aidan wrote: > The best option is to figure stuff out. [...] > I use FFmpeg to download HLS streams from the internet or convert files > like probably most people do. FFmpeg is the ultimate way of doing this > because there is no better option. > > But there are issues: [...] > I submitted a patch for a TTML decoder because I thought it would be great. > It was completely ignored. Please ping the patch or send a new one. > If my patch was seriously bad, then fine. But seriously *no one cared*. I think contribution management is a serious issue here. What happens when you send a patch is that if you're lucky someone will be interested and put some effort to review and eventually get it pushed, which depending on several factors might require several interactions. Sometimes contributors are side-tracked or frustrated and the review process is interrupted. Sometimes the reviewer won't reply, and the review also might be stuck (in this case you might want to ping the patch). Sometimes there is no qualified or interested developer around, or maybe those ones are busy with other things (and it's easy to miss a patch, especially if you don't check emails since a few days and you got hundreds of backlog emails). In general, this is done on a best effort basis (read as: most developers are volunteers and they might have job/families/stuff to tend to), there is no guarantee that a patch might be reviewed in a timely fashion. This is not a problem specific with FFmpeg, but in general with most FLOSS projects. Probably we should find ways to fund such activites, so that a developer can spend more time on reviewing work, but this comes with other risks/issues (since managing money is also complex of potential tensions in a mostly volunteering-based project). It's also very difficult to track the sent patches, and that's why having a Pull-Request process a-la github has been proposed several times; we cannot switch to github for several reasons (licensing and affilitation issues with platform owner) and handling your own gitlab is costly and we lack volunteers at the moment. We are using patchwork to mitigate the tracking issue: https://patchwork.ffmpeg.org/project/ffmpeg/list/ but that's not really providing an effective workflow. Personally I find the status tracking confusing, e.g.: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=&submitter=&state=&q=TTML&archive=both&delegate= I cannot easily figure out what was integrated and what not. ___ 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 date Wednesday 2024-04-17 17:24:02 +0100, Andrew Sayers wrote:
> On 17/04/2024 14:58, Michael Niedermayer wrote:
[...]
> I've occasionally tried getting into ffmpeg for over a decade now, and have
> lurked here for the past few months as I put in the effort to grok it.
> On behalf of people who could contribute but don't, I'd like to suggest
> ffmpeg focus on *learnability*.
>
> Whenever I've tried to learn ffmpeg, I've always been rebuffed by
> documentation that seems needlessly hard to use. I understand some of
> these reflect deeper issues - for example there's a reason the words
> "ffmpeg" and "libav" are used ambiguously, even though it makes it
> hard to differentiate between the library and the command-line tool.
> But other issues seem like quick wins - for example I've lost count of
> all the times I typed two functions into Google, spent hours trying to
> make them work together, then finally realised I was looking at the
> documentation for 3.0 in one tab and 5.0 in the other. Surely you can
> just add a line to the top of the documentation like "click here to see
> the trunk version of this file"?
Functions are documented in doxygen, so they depend on the major.minor
version, while you seem to refer to the FFmpeg version. Also on the
website we usually only have the latest mainline documentation, so I
don't understand how can you have different versions in different tabs
(unless you didn't update that tab since months/years).
If you mean that we should ship documentatation for the latest
supported releases, in addition to latest mainline, I tend to agree.
> Here's a small example to demonstrate the larger issue -
> what does it mean for something to be a "context"?
> When I started learning how to write ffmpeg code, I read through the
> docs and saw various things calling themselves "context" structs, but
> never found a link to explain what that meant. If I was a young
> developer, I would probably have just assumed it was standard
> programming jargon I was too dumb to know, and walked away to find
> something more my speed. But I'm old and stubborn and have nothing
> better to do right now, so I kept going...
>
> I tried to learn by going through the examples, but the nearest thing
> to an explanation was e.g. `transcode.c` making up a new type and
> calling it a `FilteringContext`. I ignored the AVClass documentation
> for a long time because the name made me think it was some kind of
> GObject-style C-with-classes thing. It was only when I noticed that
> it says it describes "the class of an AVClass context structure" that
> I realised what I was looking at. And it was only when I convinced
> myself that the documentation for AVOptions was using
> "AVOptions-enabled struct" to mean the same thing as "AVClass context
> structure" that I felt able to disregard the `FilteringContext`. So
> my current opinion is that "AVOptions-enabled struct", "AVClass
> context structure" and "context structure" are different terms for the
> same thing - but now I've said that publicly, I will no doubt find an
> "SwrClass context structure" or something tomorrow.
In general, a "context" in the FFmpeg jargon is usually a data
structure providing the context/state/configuration for a given
operation, which can be muxing, demuxing, decoding, encoding,
filtering etc.
You need to fill the "context" with the configuration parameters and
with the data needed for the specific operation.
In general, when setting up a context, you also want some facilities
to avoid to repeat logic again and again:
- you want to provide means to send log messages
- you want an interface to query, set, and get options
- you want a "private" internal context, with options/parameters
specific for a particular instance of a generic context. For example
you might want to set specific options which only apply to a given
encoder (these are so-called "private" options)
This is done through the AVClass structure, which being generic is
used in various parts of FFmpeg.
The AVOption interface wass added later, so depending on your usage,
you might be directly setting a field in the context of set an option
through the AVOption programming interface.
Currently this is the documentation for AVCodecContext:
/**
* main external API structure.
* New fields can be added to the end with minor version bumps.
* Removal, reordering and changes to existing fields require a major
* version bump.
* You can use AVOptions (av_opt* / av_set/get*()) to access these fields from
user
* applications.
* The name string for AVOptions options matches the associated command line
* parameter name and can be found in libavcodec/options_table.h
* 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*.
*/
typedef struct AVCodecContext {
I agree this might be improved/updated, "main external API st
Re: [FFmpeg-devel] [PATCH 4/6] doc/muxers: add mkvtimestamp_v2
On date Tuesday 2024-04-16 12:50:19 +0200, Andreas Rheinhardt wrote:
> Stefano Sabatini:
> > ---
> > doc/muxers.texi | 8
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/doc/muxers.texi b/doc/muxers.texi
> > index f94513527d..490d5557bf 100644
> > --- a/doc/muxers.texi
> > +++ b/doc/muxers.texi
> > @@ -2933,6 +2933,14 @@ MicroDVD subtitle format muxer.
> >
> > This muxer accepts a single @samp{microdvd} subtitles stream.
> >
> > +@section mkvtimestamp_v2
> > +mkvtoolnix v2 timecode format muxer.
> > +
> > +Write the PTS rawvideo frame to the output, as supported by the
> > +@command{mkvextact} tool from the @command{mkvtoolnix} suite.
> > +
> > +This muxer accepts a single @samp{rawvideo} stream.
> > +
> > @section mp3
> >
> > The MP3 muxer writes a raw MP3 stream with the following optional features:
>
> This is wrong: MKVToolNix switched to "# timestamp format v2" a long
> time ago (we still write the old "# timecode format v2" header);
> furthermore, MKVToolNix actually uses pts (which it reorders to be
> ascending), not dts like our muxer. Furthermore MKVToolNix does not
> force a 1ms precision on timestamps.
Correct.
I compared the output of the muxer and of mkvtoolnix extract
timestamp_v2 and I'm not yet clear about the timestamp differences I'm
observing (the muxer output maps with the timestamps, the mkvtoolnix
timestamps differ by a few ms). But I think also mkvtoolnix use a 1ms
timebase.
Also, IIRC there is no generic way to reorder PTSs, so this might
account for another difference which might be difficult to implement
generically.
___
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] avformat/lc3: Only allow AV_CODEC_ID_LC3 in muxer
On date Tuesday 2024-04-16 19:23:27 +0200, Andreas Rheinhardt wrote:
> Also check for the number of streams and the AVCodecID generically
> using FF_OFMT_FLAGs.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/lc3.c | 14 +++---
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/libavformat/lc3.c b/libavformat/lc3.c
> index 93ce720af3..16c12a98d7 100644
> --- a/libavformat/lc3.c
> +++ b/libavformat/lc3.c
> @@ -186,16 +186,6 @@ const FFInputFormat ff_lc3_demuxer = {
>
> #if CONFIG_LC3_MUXER
>
> -static av_cold int lc3_muxer_init(AVFormatContext *s)
> -{
> -if (s->nb_streams != 1) {
> -av_log(s, AV_LOG_ERROR, "This muxer only supports a single
> stream.\n");
> -return AVERROR(EINVAL);
> -}
> -
> -return 0;
> -}
> -
> static int lc3_write_header(AVFormatContext *s)
> {
> AVStream *st = s->streams[0];
> @@ -243,8 +233,10 @@ const FFOutputFormat ff_lc3_muxer = {
> .p.extensions = "lc3",
> .p.audio_codec = AV_CODEC_ID_LC3,
> .p.video_codec = AV_CODEC_ID_NONE,
> +.p.subtitle_codec = AV_CODEC_ID_NONE,
> .p.flags = AVFMT_NOTIMESTAMPS,
> -.init = lc3_muxer_init,
> +.flags_internal = FF_OFMT_FLAG_MAX_ONE_OF_EACH |
> +FF_OFMT_FLAG_ONLY_DEFAULT_CODECS,
> .write_header = lc3_write_header,
> .write_packet = lc3_write_packet,
> };
LGTM, thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 8/6] doc/protocols: Fill in missing HTTP options
On date Tuesday 2024-04-16 14:55:55 +0100, Derek Buitenhuis wrote:
> Signed-off-by: Derek Buitenhuis
> ---
> doc/protocols.texi | 30 ++
> 1 file changed, 30 insertions(+)
>
> diff --git a/doc/protocols.texi b/doc/protocols.texi
> index 5ce1ddc8f4..60c6d831dd 100644
> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -492,6 +492,10 @@ contains the last non-empty metadata packet sent by the
> server. It should be
> polled in regular intervals by applications interested in mid-stream metadata
> updates.
>
> +@item metadata
> +An exported dictionary containing Icecast metadata from the bitstream, if
> present.
> +Only useful with the C API.
Probably best to use impersonal verbal mode:
Set an exported ...
> +
> @item auth_type
>
> Set HTTP authentication type. No option for Digest, since this method
> requires
> @@ -519,6 +523,10 @@ Send an Expect: 100-continue header for POST. If set to
> 1 it will send, if set
> to 0 it won't, if set to -1 it will try to send if it is applicable. Default
> value is -1.
>
> +@item location
> +An exported dictionary containing the content location. Only useful with the
> C
> +API.
Ditto
> +
> @item offset
> Set initial byte offset.
>
> @@ -535,6 +543,9 @@ be given a Bad Request response.
> When unset the HTTP method is not checked for now. This will be replaced by
> autodetection in the future.
>
> +@item reconnect
> +Reconnect automatically when disconnected before EOF is hit.
> +
> @item reconnect_at_eof
> If set then eof is treated like an error and causes reconnection, this is
> useful
> for live / endless streams.
> @@ -552,6 +563,14 @@ If set then even streamed/non seekable streams will be
> reconnected on errors.
> @item reconnect_delay_max
> Sets the maximum delay in seconds after which to give up reconnecting
>
> +@item reconnect_max_retries
> +Sets the maximum number of times to retry a connection. Default unset.
Set the ...
> +
> +@item respect_retry_after
> +If enabled, and a Retry-After header is encountered, its requested
> reconnection
> +delay will be honored, rather than using exponential backoff. Useful for 429
> and
> +503 errors. Default enabled.
> +
> @item listen
> If set to 1 enables experimental HTTP server. This can be used to send data
> when
> used as an output option, or read data from a client with HTTP POST when
> used as
> @@ -578,6 +597,17 @@ ffmpeg -i somefile.ogg -chunked_post 0 -c copy -f ogg
> http://@var{server}:@var{p
> wget --post-file=somefile.ogg http://@var{server}:@var{port}
> @end example
>
> +@item resource
> +The resource requested by a client, when the experimental HTTP server is in
> use.
Set the ...
also this might be more explicit (what is a resource in this context?)
> +
> +@item reply_code
> +The HTTP code returned to the client, when the experimental HTTP server is
> in use.
> +
> +@item short_seek_size
> +The threshold, in bytes, for when a readahead should be prefered over a seek
> and
> +new HTTP request. This is useful, for example, to make sure the same
> connection
> +is used for reading large video packets with small audio packets in between.
Set the ...
for consistency reasons
[...]
Should be good 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 7/6] doc/protocols: Re-order HTTP options to match http.c order
On date Tuesday 2024-04-16 14:55:54 +0100, Derek Buitenhuis wrote: > This makes the list easier to maintain. > > Signed-off-by: Derek Buitenhuis > --- > doc/protocols.texi | 112 ++--- > 1 file changed, 56 insertions(+), 56 deletions(-) Sure, thank 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] [PATCH 6/6] doc/muxers: add mmf
---
doc/muxers.texi | 9 +
1 file changed, 9 insertions(+)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index 490d5557bf..43c4865292 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -2941,6 +2941,15 @@ Write the PTS rawvideo frame to the output, as supported
by the
This muxer accepts a single @samp{rawvideo} stream.
+@section mmf
+Synthetic music Mobile Application Format (SMAF) format muxer.
+
+SMAF is a music data format specified by Yamaha for portable
+electronic devices, such as cell phones and PDAs. It is common as
+ringtones for mobile phones with one of five sound chips.
+
+This muxer accepts a single @samp{adpcm_yamaha} audio stream.
+
@section mp3
The MP3 muxer writes a raw MP3 stream with the following optional features:
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 5/6] lavf/mkvtimestamp_v2: use name in place of description in long name
---
libavformat/mkvtimestamp_v2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/mkvtimestamp_v2.c b/libavformat/mkvtimestamp_v2.c
index dde431ab7d..1eb2daf10a 100644
--- a/libavformat/mkvtimestamp_v2.c
+++ b/libavformat/mkvtimestamp_v2.c
@@ -43,7 +43,7 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
const FFOutputFormat ff_mkvtimestamp_v2_muxer = {
.p.name= "mkvtimestamp_v2",
-.p.long_name = NULL_IF_CONFIG_SMALL("extract pts as timecode v2 format,
as defined by mkvtoolnix"),
+.p.long_name = NULL_IF_CONFIG_SMALL("mkvtoolnix v2 timecode format"),
.p.audio_codec = AV_CODEC_ID_NONE,
.p.video_codec = AV_CODEC_ID_RAWVIDEO,
.write_header = write_header,
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 4/6] doc/muxers: add mkvtimestamp_v2
---
doc/muxers.texi | 8
1 file changed, 8 insertions(+)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index f94513527d..490d5557bf 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -2933,6 +2933,14 @@ MicroDVD subtitle format muxer.
This muxer accepts a single @samp{microdvd} subtitles stream.
+@section mkvtimestamp_v2
+mkvtoolnix v2 timecode format muxer.
+
+Write the PTS rawvideo frame to the output, as supported by the
+@command{mkvextact} tool from the @command{mkvtoolnix} suite.
+
+This muxer accepts a single @samp{rawvideo} stream.
+
@section mp3
The MP3 muxer writes a raw MP3 stream with the following optional features:
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/6] doc/muxers/matroskaenc: add missing options, apply misc style fixes
---
doc/muxers.texi | 44 +---
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index a77472ef1b..a162ab4075 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -2846,8 +2846,44 @@ the initially reserved space turns out to be
insufficient.
This option is ignored if the output is unseekable.
+@item cluster_size_limit @var{size}
+Store at most the provided amount of bytes in a cluster.
+
+If not specified, the limit is set automatically to a sensible
+hardcoded fixed value.
+
+@item cluster_time_limit @var{duration}
+Store at most the provided number of milliseconds in a cluster.
+
+If not specified, the limit is set automatically to a sensible
+hardcoded fixed value.
+
+@item dash @var{bool}
+Create a WebM file conforming to WebM DASH specification. By default
+it is set to @code{false}.
+
+@item dash_track_number @var{index}
+Track number for the DASH stream. By default it is set to @code{1}.
+
+@item live @var{bool}
+Write files assuming it is a live stream. By default it is set to
+@code{false}.
+
+@item allow_raw_vfw @var{bool}
+Allow raw VFW mode. By default it is set to @code{false}.
+
+@item flipped_raw_rgb @var{bool}
+If set to @code{true}, store positive height for raw RGB bitmaps, which
indicates
+bitmap is stored bottom-up. Note that this option does not flip the bitmap
+which has to be done manually beforehand, e.g. by using the @samp{vflip}
filter.
+Default is @code{false} and indicates bitmap is stored top down.
+
+@item write_crc32 @var{bool}
+Write a CRC32 element inside every Level 1 element. By default it is
+set to @code{true}.
+
@item default_mode @var{mode}
-This option controls how the FlagDefault of the output tracks will be set.
+Control how the FlagDefault of the output tracks will be set.
It influences which tracks players should play by default. The default mode
is @samp{passthrough}.
@table @samp
@@ -2865,12 +2901,6 @@ disposition default exists, no subtitle track will be
marked as default.
In this mode the FlagDefault is set if and only if the AV_DISPOSITION_DEFAULT
flag is set in the disposition of the corresponding stream.
@end table
-
-@item flipped_raw_rgb @var{bool}
-If set to true, store positive height for raw RGB bitmaps, which indicates
-bitmap is stored bottom-up. Note that this option does not flip the bitmap
-which has to be done manually beforehand, e.g. by using the vflip filter.
-Default is @var{false} and indicates bitmap is stored top down.
@end table
@anchor{md5}
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] Apply misc doc/muxers fixes and extensions
Apply misc doc/muxers fixes and extensions. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/6] doc/muxers: add microdvd
---
doc/muxers.texi | 5 +
1 file changed, 5 insertions(+)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index 710fc27eec..f94513527d 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -2928,6 +2928,11 @@ ffmpeg -i INPUT -f md5 -
@end example
@end itemize
+@section microdvd
+MicroDVD subtitle format muxer.
+
+This muxer accepts a single @samp{microdvd} subtitles stream.
+
@section mp3
The MP3 muxer writes a raw MP3 stream with the following optional features:
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/6] doc/muxers/md5: apply misc consistency fixes
---
doc/muxers.texi | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index a162ab4075..710fc27eec 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -2905,26 +2905,28 @@ flag is set in the disposition of the corresponding
stream.
@anchor{md5}
@section md5
-
MD5 testing format.
This is a variant of the @ref{hash} muxer. Unlike that muxer, it
defaults to using the MD5 hash function.
-@subsection Examples
+See also the @ref{hash} and @ref{framemd5} muxers.
+@subsection Examples
+@itemize
+@item
To compute the MD5 hash of the input converted to raw
audio and video, and store it in the file @file{out.md5}:
@example
ffmpeg -i INPUT -f md5 out.md5
@end example
-You can print the MD5 to stdout with the command:
+@item
+To print the MD5 to stdout:
@example
ffmpeg -i INPUT -f md5 -
@end example
-
-See also the @ref{hash} and @ref{framemd5} muxers.
+@end itemize
@section mp3
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/httpauth.c Support RFC7616 [Style fixed]
On date Monday 2024-04-15 02:32:14 +, �� | Eugene wrote:
> Update digest authentication in httpauth.c
>
> - Refactor make_digest_auth() to support RFC 2617 and RFC 7617
> - Add support for SHA-256 and SHA-512/256 algorithms along with MD5
> - MD5 and SHA-256 tested, but SHA-512/256 untested due to lack of server
> - Replace AVMD5 structure with AVHashContext for hash algorithm flexibility
> - Rename update_md5_strings() to update_hash_strings() for consistency
> - Address coding style feedback from reviewer:
>
> This is a feature update focused on digest authentication enhancements.
> Some lint issues in the existing code remain unaddressed in this patch.
>
> Signed-off-by: Eugene-bitsensing
> ---
> libavformat/httpauth.c | 102 +
> 1 file changed, 53 insertions(+), 49 deletions(-)
add entry to Changelog?
>
> diff --git a/libavformat/httpauth.c b/libavformat/httpauth.c
> index 9780928357..2592140526 100644
> --- a/libavformat/httpauth.c
> +++ b/libavformat/httpauth.c
> @@ -24,7 +24,7 @@
> #include "libavutil/avstring.h"
> #include "internal.h"
> #include "libavutil/random_seed.h"
> -#include "libavutil/md5.h"
> +#include "libavutil/hash.h"
> #include "urldecode.h"
>
> static void handle_basic_params(HTTPAuthState *state, const char *key,
> @@ -117,35 +117,35 @@ void ff_http_auth_handle_header(HTTPAuthState *state,
> const char *key,
> }
> }
>
> -
> -static void update_md5_strings(struct AVMD5 *md5ctx, ...)
> +/* Generate hash string, updated to use AVHashContext to support other
> algorithms */
> +static void update_hash_strings(struct AVHashContext *hash_ctx, ...)
> {
> va_list vl;
>
> -va_start(vl, md5ctx);
> +va_start(vl, hash_ctx);
> while (1) {
> -const char* str = va_arg(vl, const char*);
> +const char *str = va_arg(vl, const char*);
> if (!str)
> break;
> -av_md5_update(md5ctx, str, strlen(str));
> +av_hash_update(hash_ctx, (const uint8_t *)str, strlen(str));
> }
> va_end(vl);
> }
>
> -/* Generate a digest reply, according to RFC 2617. */
> +/* Generate a digest reply, according to RFC 2617. Update to support RFC
> 7617*/
> static char *make_digest_auth(HTTPAuthState *state, const char *username,
>const char *password, const char *uri,
>const char *method)
> {
> DigestParams *digest = &state->digest_params;
> -int len;
> +size_t len;
> uint32_t cnonce_buf[2];
> char cnonce[17];
> char nc[9];
> int i;
> -char A1hash[33], A2hash[33], response[33];
> -struct AVMD5 *md5ctx;
> -uint8_t hash[16];
> +char a1_hash[65], a2_hash[65], response[65];
> +struct AVHashContext *hash_ctx = NULL; // use AVHashContext for other
> algorithm support
> +size_t len_hash = 33; // Modifiable hash length, MD5:32, SHA-2:64
> char *authstr;
>
> digest->nc++;
> @@ -156,52 +156,56 @@ static char *make_digest_auth(HTTPAuthState *state,
> const char *username,
> cnonce_buf[i] = av_get_random_seed();
> ff_data_to_hex(cnonce, (const uint8_t*) cnonce_buf, sizeof(cnonce_buf),
> 1);
>
> -md5ctx = av_md5_alloc();
> -if (!md5ctx)
> -return NULL;
> -
> -av_md5_init(md5ctx);
> -update_md5_strings(md5ctx, username, ":", state->realm, ":", password,
> NULL);
> -av_md5_final(md5ctx, hash);
> -ff_data_to_hex(A1hash, hash, 16, 1);
> -
> -if (!strcmp(digest->algorithm, "") || !strcmp(digest->algorithm, "MD5"))
> {
> -} else if (!strcmp(digest->algorithm, "MD5-sess")) {
> -av_md5_init(md5ctx);
> -update_md5_strings(md5ctx, A1hash, ":", digest->nonce, ":", cnonce,
> NULL);
> -av_md5_final(md5ctx, hash);
> -ff_data_to_hex(A1hash, hash, 16, 1);
> -} else {
> -/* Unsupported algorithm */
> -av_free(md5ctx);
> +/* Generate hash context by algorithm. */
> +const char *algorithm = digest->algorithm;
> +const char *hashing_algorithm;
nit++: to avoid semantic overlap I'd rather name this
selected_algorithm
> +if (!*algorithm) {
> +algorithm = "MD5"; // if empty, use MD5 as Default
> +hashing_algorithm = "MD5";
> +} else if (av_stristr(algorithm, "MD5")) {
> +hashing_algorithm = "MD5";
> +} else if (av_stristr(algorithm, "sha256") || av_stristr(algorithm,
> "sha-256")) {
> +hashing_algorithm = "SHA256";
> +len_hash = 65; // SHA-2: 64 characters.
> +} else if (av_stristr(algorithm, "sha512-256") || av_stristr(algorithm,
> "sha-512-256")) {
> +hashing_algorithm = "SHA512_256";
> +len_hash = 65; // SHA-2: 64 characters.
> +} else { // Unsupported algorithm
> return NULL;
> }
>
> -av_md5_init(md5ctx);
> -update_md5_strings(md5ctx, method, ":", uri, NULL);
> -av_md5_final(md5ctx, hash);
> -ff_data_to_hex(A2hash, hash, 16, 1);
> +
Re: [FFmpeg-devel] [PATCH 4/6 v2] avformat/http: Add support for Retry-After header
On date Monday 2024-04-15 17:49:33 +0100, Derek Buitenhuis wrote: > 429 and 503 codes can, and often do (e.g. all Google Cloud > Storage URLs can), return a Retry-After header with the error, > indicating how long to wait, in seconds, before retrying again. > If it is not respected by, for example, using our default backoff > stratetgy instead, chances of success are very unlikely. > > This adds an AVOption to respect that header. > > Signed-off-by: Derek Buitenhuis > --- > libavformat/http.c| 12 > libavformat/version.h | 2 +- > 2 files changed, 13 insertions(+), 1 deletion(-) missing doc/protocols.texi update ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] doc/utils/eval: clarify meaning of random* seed value
On date Thursday 2024-04-11 03:50:46 +0200, Michael Niedermayer wrote: > On Wed, Apr 10, 2024 at 03:47:42PM +0200, Stefano Sabatini wrote: > > Possible address trac issue: > > > http://trac.ffmpeg.org/ticket/10763 > > Some of the things in this ticket are specific to the buggy LCG > we no longer use > > > > --- > > doc/utils.texi | 12 > > 1 file changed, 12 insertions(+) > > sounds ok > > thx Applied, 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] avformat/lc3: Add file format for LC3/LC3plus transport
On date Saturday 2024-04-13 10:54:38 +0200, Stefano Sabatini wrote: > On date Friday 2024-04-12 16:46:29 -0700, ffmpeg-devel Mailing List wrote: > > Thanks. > > > > On Fri, Apr 12, 2024 at 6:05 AM Stefano Sabatini wrote: > [...] > > From 93c5628502a1f242043b39a18e83895d9067541e Mon Sep 17 00:00:00 2001 > > From: Antoine SOULIER > > Date: Thu, 4 Apr 2024 22:38:03 + > > Subject: [PATCH] avformat/lc3: Add file format for LC3/LC3plus transport > > > > A file format is described in Bluetooth SIG LC3 and ETSI TS 103 634, for > > test purpose. This is the format implemented here. > > --- > > Changelog| 1 + > > doc/muxers.texi | 6 + > > libavformat/Makefile | 2 + > > libavformat/allformats.c | 2 + > > libavformat/lc3.c| 252 +++ > > 5 files changed, 263 insertions(+) > > create mode 100644 libavformat/lc3.c > > Looks good to me. > > Andreas, Paul, please comment. If I see no comments I'll push this in > two/three days. Finally applied, 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] avformat/httpauth.c [support both RFC 2617 and RFC 7617]
On date Thursday 2024-04-11 07:48:14 +, �� | Eugene wrote:
> - Updated the make_digest_auth() function to support both RFC 2617 and RFC
> 7617 digest authentication.
> - Supports sha256 , sha512-256 along with the existing md5. MD5 and sha256
> were tested, but sha512-256 was not tested due to lack of testable server.
> - AVHashContext instead of AVMD5 structure.
> - update_md5_strings() -> update_hash_strings().
> - There are some lynt issues in the old code of make_digest_auth, but this is
> a feature update patch, so I didn't fix it.
> - modified the implementation of RFC7616 based on community feedback.
>
> Signed-off-by: Eugene-bitsensing
> ---
> libavformat/httpauth.c | 105 ++---
> 1 file changed, 57 insertions(+), 48 deletions(-)
>
> diff --git a/libavformat/httpauth.c b/libavformat/httpauth.c
> index 9780928357..ba2ebea3a4 100644
> --- a/libavformat/httpauth.c
> +++ b/libavformat/httpauth.c
> @@ -24,7 +24,7 @@
> #include "libavutil/avstring.h"
> #include "internal.h"
> #include "libavutil/random_seed.h"
> -#include "libavutil/md5.h"
> +#include "libavutil/hash.h"
> #include "urldecode.h"
>
> static void handle_basic_params(HTTPAuthState *state, const char *key,
> @@ -117,35 +117,35 @@ void ff_http_auth_handle_header(HTTPAuthState *state,
> const char *key,
> }
> }
>
> -
> -static void update_md5_strings(struct AVMD5 *md5ctx, ...)
> +/* Generate hash string, updated to use AVHashContext to support other
> algorithms */
> +static void update_hash_strings(struct AVHashContext* hash_ctx, ...)
> {
> va_list vl;
>
> -va_start(vl, md5ctx);
> +va_start(vl, hash_ctx);
> while (1) {
> const char* str = va_arg(vl, const char*);
> if (!str)
> break;
> -av_md5_update(md5ctx, str, strlen(str));
> +av_hash_update(hash_ctx, (const uint8_t*)str, strlen(str));
nit++: (const uint8_t *)str
note the space before the "*"
> }
> va_end(vl);
> }
>
> -/* Generate a digest reply, according to RFC 2617. */
> +/* Generate a digest reply, according to RFC 2617. Update to support RFC
> 7617*/
nit: according to RFC 2617/7617
> static char *make_digest_auth(HTTPAuthState *state, const char *username,
>const char *password, const char *uri,
>const char *method)
> {
> DigestParams *digest = &state->digest_params;
> -int len;
> +size_t len;// change int -> size_t
drop this comment
> uint32_t cnonce_buf[2];
> char cnonce[17];
> char nc[9];
> int i;
> -char A1hash[33], A2hash[33], response[33];
> -struct AVMD5 *md5ctx;
> -uint8_t hash[16];
> +char a1_hash[65], a2_hash[65], response[65]; // increase hash size for
> SHA-2
same here, drop the comment or this will be confusing without
reference to the diff
> +struct AVHashContext* hash_ctx = NULL; // use AVHashContext for other
> algorithm support
nit: here and below, use:
TYPE *VAR
rather than:
TYPE* VAR
style for consistency
> +size_t len_hash = 33; // Modifiable hash length, MD5:32, SHA-2:64
> char *authstr;
>
> digest->nc++;
> @@ -156,52 +156,61 @@ static char *make_digest_auth(HTTPAuthState *state,
> const char *username,
> cnonce_buf[i] = av_get_random_seed();
> ff_data_to_hex(cnonce, (const uint8_t*) cnonce_buf, sizeof(cnonce_buf),
> 1);
>
> -md5ctx = av_md5_alloc();
> -if (!md5ctx)
> -return NULL;
> -
> -av_md5_init(md5ctx);
> -update_md5_strings(md5ctx, username, ":", state->realm, ":", password,
> NULL);
> -av_md5_final(md5ctx, hash);
> -ff_data_to_hex(A1hash, hash, 16, 1);
> -
> -if (!strcmp(digest->algorithm, "") || !strcmp(digest->algorithm, "MD5"))
> {
> -} else if (!strcmp(digest->algorithm, "MD5-sess")) {
> -av_md5_init(md5ctx);
> -update_md5_strings(md5ctx, A1hash, ":", digest->nonce, ":", cnonce,
> NULL);
> -av_md5_final(md5ctx, hash);
> -ff_data_to_hex(A1hash, hash, 16, 1);
> -} else {
> -/* Unsupported algorithm */
> -av_free(md5ctx);
> +/* Generate hash context by algorithm. */
> +const char* algorithm = digest->algorithm;
> +const char* hashing_algorithm;
> +if (!*algorithm) {
> +algorithm = "MD5"; // if empty, use MD5 as Default
> +hashing_algorithm = "MD5";
> +}
> +else if (av_stristr(algorithm, "md5") || av_stristr(algorithm, "MD5")) {
this should be case-insensitive so you can skip one check
also I wonder if this should be a strcmp, for example can
it happen to have "foo-md5-bar" and should the algorithm
be tolerant in such cases?
style nit: here and below, put { and else on the same line
} else ... {
> +hashing_algorithm = "MD5";
> +}
> +else if (av_stristr(algorithm, "sha256") || av_stristr(algorithm,
> "sha-256")) {
> +hashing_algorithm = "SHA256";
> +len_hash = 65; /
Re: [FFmpeg-devel] [PATCH] doc/muxers.texi: Don't use confusing variable name
On date Saturday 2024-04-13 00:38:49 +0200, Andreas Rheinhardt wrote:
> reserve_index_space is a size, not an index.
> Also refer to the variable in the description.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> doc/muxers.texi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index 4b30970b78..489f22288b 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -2816,14 +2816,14 @@ ffmpeg -i sample_left_right_clip.mpg -an -c:v libvpx
> -metadata stereo_mode=left_
>
> @subsection Options
> @table @option
> -@item reserve_index_space @var{index}
> +@item reserve_index_space @var{size}
> By default, this muxer writes the index for seeking (called cues in Matroska
> terms) at the end of the file, because it cannot know in advance how much
> space
> to leave for the index at the beginning of the file. However for some use
> cases
> -- e.g. streaming where seeking is possible but slow -- it is useful to put
> the
> index at the beginning of the file.
>
> -If this option is set to a non-zero value, the muxer will reserve a given
> amount
> +If this option is set to a non-zero value, the muxer will reserve @var{size}
> bytes
> of space in the file header and then try to write the cues there when the
> muxing
> finishes. If the reserved space does not suffice, no Cues will be written,
> the
> file will be finalized and writing the trailer will return an error.
This is more correct, 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] avformat/lc3: Add file format for LC3/LC3plus transport
On date Friday 2024-04-12 16:46:29 -0700, ffmpeg-devel Mailing List wrote: > Thanks. > > On Fri, Apr 12, 2024 at 6:05 AM Stefano Sabatini wrote: [...] > From 93c5628502a1f242043b39a18e83895d9067541e Mon Sep 17 00:00:00 2001 > From: Antoine SOULIER > Date: Thu, 4 Apr 2024 22:38:03 + > Subject: [PATCH] avformat/lc3: Add file format for LC3/LC3plus transport > > A file format is described in Bluetooth SIG LC3 and ETSI TS 103 634, for > test purpose. This is the format implemented here. > --- > Changelog| 1 + > doc/muxers.texi | 6 + > libavformat/Makefile | 2 + > libavformat/allformats.c | 2 + > libavformat/lc3.c| 252 +++ > 5 files changed, 263 insertions(+) > create mode 100644 libavformat/lc3.c Looks good to me. Andreas, Paul, please comment. If I see no comments I'll push this in two/three days. 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] avformat/lc3: Add file format for LC3/LC3plus transport
On date Wednesday 2024-04-10 16:46:55 -0700, ffmpeg-devel Mailing List wrote:
> Sure, I thought these warnings were disabled while looking at codec2.c
> Sorry for the bad merge of the doc.
> From 975040408f32431efc3fae0a0b8c048f02159515 Mon Sep 17 00:00:00 2001
> From: Antoine SOULIER
> Date: Thu, 4 Apr 2024 22:38:03 +
> Subject: [PATCH] avformat/lc3: Add file format for LC3/LC3plus transport
>
> A file format is described in Bluetooth SIG LC3 and ETSI TS 103 634, for
> test purpose. This is the format implemented here.
> ---
> Changelog| 1 +
> doc/muxers.texi | 6 +
> libavformat/Makefile | 2 +
> libavformat/allformats.c | 2 +
> libavformat/lc3.c| 253 +++
> 5 files changed, 264 insertions(+)
> create mode 100644 libavformat/lc3.c
>
> diff --git a/Changelog b/Changelog
> index b7a1af4083..5c8f505211 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -5,6 +5,7 @@ version :
> - Raw Captions with Time (RCWT) closed caption demuxer
> - LC3/LC3plus decoding/encoding using external library liblc3
> - ffmpeg CLI filtergraph chaining
> +- LC3/LC3plus demuxer and muxer
>
>
> version 7.0:
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index 4b30970b78..4c14323d50 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -2725,6 +2725,12 @@ games such as "Real War", and "Real War: Rogue States".
>
> This muxer accepts a single @samp{adpcm_ima_ssi} audio stream.
>
> +@section lc3
> +Bluetooth SIG Low Complexity Communication Codec audio (LC3), or
> +ETSI TS 103 634 Low Complexity Communication Codec plus (LC3plus).
> +
> +This muxer accepts a single @code{lc3} audio stream.
nit++: @samp{lc3} for consistency
> +
> @section lrc
> LRC lyrics file format muxer.
>
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 9981799cc9..8efe26b6df 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -332,6 +332,8 @@ OBJS-$(CONFIG_KVAG_DEMUXER) += kvag.o
> OBJS-$(CONFIG_KVAG_MUXER)+= kvag.o rawenc.o
> OBJS-$(CONFIG_LAF_DEMUXER) += lafdec.o
> OBJS-$(CONFIG_LATM_MUXER)+= latmenc.o rawenc.o
> +OBJS-$(CONFIG_LC3_DEMUXER) += lc3.o
> +OBJS-$(CONFIG_LC3_MUXER) += lc3.o
> OBJS-$(CONFIG_LMLM4_DEMUXER) += lmlm4.o
> OBJS-$(CONFIG_LOAS_DEMUXER) += loasdec.o rawdec.o
> OBJS-$(CONFIG_LUODAT_DEMUXER)+= luodatdec.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index ae925dcf60..305fa46532 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -252,6 +252,8 @@ extern const FFInputFormat ff_kvag_demuxer;
> extern const FFOutputFormat ff_kvag_muxer;
> extern const FFInputFormat ff_laf_demuxer;
> extern const FFOutputFormat ff_latm_muxer;
> +extern const FFInputFormat ff_lc3_demuxer;
> +extern const FFOutputFormat ff_lc3_muxer;
> extern const FFInputFormat ff_lmlm4_demuxer;
> extern const FFInputFormat ff_loas_demuxer;
> extern const FFInputFormat ff_luodat_demuxer;
> diff --git a/libavformat/lc3.c b/libavformat/lc3.c
> new file mode 100644
> index 00..e27727145b
> --- /dev/null
> +++ b/libavformat/lc3.c
> @@ -0,0 +1,253 @@
> +/*
> + * LC3 demuxer
nit: LC3 demuxer and muxer
> + * Copyright (C) 2024 Antoine Soulier
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +/**
> + * @file
> + * Based on the file format specified by :
> + *
> + * - Bluetooth SIG - Low Complexity Communication Codec Test Suite
> + * https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=502301
> + * 3.2.8.2 Reference LC3 Codec Bitstream Format
> + *
> + * - ETSI TI 103 634 V1.4.1 - Low Complexity Communication Codec plus
> + *
> https://www.etsi.org/deliver/etsi_ts/103600_103699/103634/01.04.01_60/ts_103634v010401p.pdf
> + * LC3plus conformance script package
> + */
> +
> +#include "config_components.h"
> +
> +#include "libavcodec/packet.h"
> +#include "libavutil/intreadwrite.h"
> +
> +#include "avformat.h"
> +#include "avio.h"
> +#include "demux.h"
> +#include "internal.h"
> +#include "mux.h"
> +
> +static int check_frame_length(int srate_hz, int frame_us)
Re: [FFmpeg-devel] [PATCH] avformat/lc3: Add file format for LC3/LC3plus transport
On date Wednesday 2024-04-10 20:26:01 +0200, Andreas Rheinhardt wrote:
> Antoine Soulier via ffmpeg-devel:
[...]
> > +#if CONFIG_LC3_DEMUXER
> > +const FFInputFormat ff_lc3_demuxer = {
> > +.p.name = "lc3",
> > +.p.long_name= NULL_IF_CONFIG_SMALL("LC3 (Low Complexity
> > Communication Codec)"),
> > +.p.extensions = "lc3",
> > +.p.flags= AVFMT_GENERIC_INDEX,
> > +.priv_data_size = sizeof(LC3DemuxContext),
> > +.read_probe = lc3_read_probe,
> > +.read_header= lc3_read_header,
> > +.read_packet= lc3_read_packet,
> > +};
> > +#endif
> > +
> > +#if CONFIG_LC3_MUXER
> > +const FFOutputFormat ff_lc3_muxer = {
> > +.p.name= "lc3",
> > +.p.long_name = NULL_IF_CONFIG_SMALL("LC3 (Low Complexity
> > Communication Codec)"),
> > +.p.extensions = "lc3",
> > +.p.audio_codec = AV_CODEC_ID_LC3,
> > +.p.video_codec = AV_CODEC_ID_NONE,
> > +.p.flags = AVFMT_NOTIMESTAMPS,
> > +.init = lc3_muxer_init,
> > +.write_header = lc3_write_header,
> > +.write_packet = lc3_write_packet,
> > +};
> > +#endif
>
> You only put the muxer and demuxer inside #if guards. If only one of
> these two is enabled, the other's functions will not be used and lead to
> compiler warnings. This can be fixed by putting all the stuff for only
> the muxer/demuxer inside the #if (see argo_cvg.c for an example).
Note: I pointed to codec2.c, which seems to suffer from the same
issue, argo_cvg.c is indeed a better example.
___
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".
