From: Stefano Sabatini <stefasab@gmail.com> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] [PATCH v3 1/3] doc: Explain what "context" means Date: Sun, 5 May 2024 09:29:10 +0200 Message-ID: <Zjc1Rh3LJQUNU8sP@mariano> (raw) In-Reply-To: <Zi9kCyTjhG-7yHYd@andrews-2024-laptop.sayers> 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 = 400000; /* 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 assuming that all the functions should take a context, which is not the case. In that case you had: av_register_bistream_filter(filter) which was registering the filter in the program global state. > av_buffersink_*() all take a `const AVFilterContext *` argument. > What does the difference between av_buffersink prefix and AVFilter type mean? None, probabily it should have been named avfilter_buffersink since this is a libavfilter API, seel below. > 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); > Related to the previous question, does `av_cmp_q()` count as a function > with two contexts? Or no contexts? And again, it looks like you are overgeneralizing and require that all the functions take a context. In general, the C language is procedural and so is the FFmpeg API. The fact that we are assuming some constructs which might mimic or resemble OOP does not mean that all the API was designed in that way. > > Finally, a general question - functions of the form "avfoo" seem like they > are more consistent than "av_foo". Does the underscore mean anything? This is due to the fact that despite the effort, implementing a consistent API is difficult, also due to different reviewers/contributors picking different conventions. In some case we prefer av_ (most of libavutil) for the generic API, while libavcodec/libavformat/libavfilter tend to use avLIB_, but there might be exceptions. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2024-05-05 7:29 UTC|newest] Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-04-18 15:06 [FFmpeg-devel] [PATCH " Andrew Sayers 2024-04-18 15:06 ` [FFmpeg-devel] [PATCH 2/3] lavu: Clarify relationship between AVClass, AVOption and context Andrew Sayers 2024-04-18 15:06 ` [FFmpeg-devel] [PATCH 3/3] all: Link to "context" from all contexts with documentation Andrew Sayers 2024-04-20 7:25 ` [FFmpeg-devel] [PATCH 1/3] doc: Explain what "context" means Stefano Sabatini 2024-04-20 12:18 ` Andrew Sayers 2024-04-20 16:13 ` Stefano Sabatini 2024-04-20 12:19 ` [FFmpeg-devel] [PATCH v2 " Andrew Sayers 2024-04-20 12:19 ` [FFmpeg-devel] [PATCH v2 2/3] lavu: Clarify relationship between AVClass, AVOption and context Andrew Sayers 2024-04-20 12:19 ` [FFmpeg-devel] [PATCH v2 3/3] all: Link to "context" from all contexts with documentation Andrew Sayers 2024-04-20 16:48 ` [FFmpeg-devel] [PATCH v2 1/3] doc: Explain what "context" means Stefano Sabatini 2024-04-20 22:17 ` Andrew Sayers 2024-04-22 8:02 ` Stefano Sabatini 2024-04-22 15:56 ` [FFmpeg-devel] [PATCH v3 0/3] all: Link to "context" from all contexts with documentation Andrew Sayers 2024-04-22 15:56 ` [FFmpeg-devel] [PATCH v3 1/3] doc: Explain what "context" means Andrew Sayers 2024-04-22 17:05 ` Stefano Sabatini 2024-04-29 9:10 ` Andrew Sayers 2024-05-02 10:03 ` Andrew Sayers 2024-05-05 7:29 ` Stefano Sabatini [this message] 2024-05-05 21:04 ` Andrew Sayers 2024-05-22 10:37 ` Stefano Sabatini 2024-05-22 12:47 ` Andrew Sayers 2024-05-25 9:00 ` Stefano Sabatini 2024-04-29 9:24 ` [FFmpeg-devel] [PATCH v4 1/4] " Andrew Sayers 2024-04-29 9:24 ` [FFmpeg-devel] [PATCH v4 2/4] lavu: Clarify relationship between AVClass, AVOption and context Andrew Sayers 2024-04-29 9:24 ` [FFmpeg-devel] [PATCH v4 3/4] all: Link to "context" from all contexts with documentation Andrew Sayers 2024-05-02 11:01 ` Lynne 2024-05-02 11:14 ` Andrew Sayers 2024-05-02 13:00 ` Zhao Zhili 2024-05-02 13:27 ` Andrew Sayers 2024-05-02 13:39 ` Zhao Zhili 2024-04-29 9:24 ` [FFmpeg-devel] [PATCH v4 4/4] lavf: Add documentation for private "Context" classes Andrew Sayers 2024-05-05 8:31 ` [FFmpeg-devel] [PATCH v4 1/4] doc: Explain what "context" means Andreas Rheinhardt 2024-05-05 10:34 ` Andrew Sayers 2024-04-22 15:56 ` [FFmpeg-devel] [PATCH v3 2/3] lavu: Clarify relationship between AVClass, AVOption and context Andrew Sayers 2024-04-22 15:56 ` [FFmpeg-devel] [PATCH v3 3/3] all: Link to "context" from all contexts with documentation Andrew Sayers 2024-05-15 15:54 ` [FFmpeg-devel] [PATCH v4 0/4] Explain what "context" means Andrew Sayers 2024-05-15 15:54 ` [FFmpeg-devel] [PATCH v4 1/4] doc: " Andrew Sayers 2024-05-22 9:31 ` Stefano Sabatini 2024-05-22 16:07 ` Andrew Sayers 2024-05-25 9:49 ` Stefano Sabatini 2024-05-26 12:06 ` Andrew Sayers 2024-05-28 17:24 ` Stefano Sabatini 2024-05-29 10:10 ` Andrew Sayers 2024-05-29 10:50 ` Andrew Sayers 2024-05-29 11:06 ` Paul B Mahol 2024-05-29 14:18 ` Andrew Sayers 2024-05-29 16:06 ` Stefano Sabatini 2024-05-23 20:00 ` [FFmpeg-devel] [PATCH v5 0/4] " Andrew Sayers 2024-05-23 20:00 ` [FFmpeg-devel] [PATCH v5 1/4] doc: " Andrew Sayers 2024-05-25 11:00 ` Stefano Sabatini 2024-05-23 20:00 ` [FFmpeg-devel] [PATCH v5 2/4] lavu: Clarify relationship between AVClass, AVOption and context Andrew Sayers 2024-05-25 9:57 ` Stefano Sabatini 2024-05-23 20:00 ` [FFmpeg-devel] [PATCH v5 3/4] all: Link to "context" from all public contexts with documentation Andrew Sayers 2024-05-23 20:00 ` [FFmpeg-devel] [PATCH v5 4/4] all: Rewrite documentation for contexts Andrew Sayers 2024-05-24 1:50 ` [FFmpeg-devel] [PATCH v5 0/4] Explain what "context" means Michael Niedermayer 2024-05-24 9:43 ` Andrew Sayers 2024-05-15 15:54 ` [FFmpeg-devel] [PATCH v4 2/4] lavu: Clarify relationship between AVClass, AVOption and context Andrew Sayers 2024-05-22 10:04 ` Stefano Sabatini 2024-05-15 15:54 ` [FFmpeg-devel] [PATCH v4 3/4] all: Link to "context" from all contexts with documentation Andrew Sayers 2024-05-15 16:46 ` Lynne via ffmpeg-devel 2024-05-16 11:25 ` Andrew Sayers 2024-05-15 15:54 ` [FFmpeg-devel] [PATCH v4 4/4] lavf: Add documentation for private "Context" classes Andrew Sayers 2024-05-22 10:08 ` Stefano Sabatini 2024-05-22 14:47 ` Andrew Sayers 2024-05-22 15:24 ` Andreas Rheinhardt 2024-05-22 16:54 ` Andrew Sayers 2024-06-04 14:47 ` [FFmpeg-devel] [PATCH v6 0/4] doc: Explain what "context" means Andrew Sayers 2024-06-04 14:47 ` [FFmpeg-devel] [PATCH v6 1/4] " Andrew Sayers 2024-06-05 8:15 ` Anton Khirnov 2024-06-12 20:52 ` Stefano Sabatini 2024-06-13 14:20 ` Andrew Sayers 2024-06-15 9:17 ` Stefano Sabatini 2024-06-16 18:02 ` [FFmpeg-devel] Development process for explaining contexts (was Re: [PATCH v6 1/4] doc: Explain what "context" means) Andrew Sayers 2024-06-16 21:20 ` Paul B Mahol 2024-07-01 22:16 ` Stefano Sabatini 2024-07-02 9:56 ` Andrew Sayers 2024-07-06 11:33 ` Stefano Sabatini 2024-06-04 14:47 ` [FFmpeg-devel] [PATCH v6 2/4] lavu: Clarify relationship between AVClass, AVOption and context Andrew Sayers 2024-06-05 10:34 ` Stefano Sabatini 2024-06-05 12:46 ` Andrew Sayers 2024-06-04 14:47 ` [FFmpeg-devel] [PATCH v6 3/4] all: Link to "context" from all public contexts with documentation Andrew Sayers 2024-06-05 8:12 ` Anton Khirnov 2024-06-05 12:51 ` Andrew Sayers 2024-06-04 14:47 ` [FFmpeg-devel] [PATCH v6 4/4] all: Rewrite documentation for contexts Andrew Sayers
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Zjc1Rh3LJQUNU8sP@mariano \ --to=stefasab@gmail.com \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git