From: Andrew Sayers <ffmpeg-devel@pileofstuff.org> To: ffmpeg-devel@ffmpeg.org Subject: [FFmpeg-devel] [RFC PATCH v2 0/1] avutil/error: Provide better feedback about unknown error codes Date: Tue, 16 Jul 2024 12:13:09 +0100 Message-ID: <20240716111543.2437488-1-ffmpeg-devel@pileofstuff.org> (raw) In-Reply-To: <9e16396a-4ef2-fdb3-fce8-94e19a141473@passwd.hu> I'm having trouble managing this conversation. On one hand, you've brought up several important details that would need to be included in a new patch. On the other hand, I'm pretty sure we're talking past each other on the big problems, and need to start over. So let's fork the discussion. # First, let's haggle over some details The patch below fixes a number of small issues brought up by your comments... Error numbers are always expressed in the code as either uppercase hex numbers or FourCCs (or ThreeCCs, but you get the point). This patch prints error codes as hex, which is no less unintelligible for ordinary users, might make problems easier to find on Google, and will sometimes make them easier to grep for. Having said that, this patch prints non-negative numbers in decimal, because all bets are off if that manages to happen. A developer could create an error code that just happens to be valid ASCII. In that situation, the previous patch would have printed something like "Unrecognised error code \"~!X\"" occurred", which is worse than the current behaviour. This patch includes both (hex) number and name in those messages. This patch adds "please report this bug" for all unknown error messages. I'll cover the reasoning below, but the relevant detail is that the previous patch just gave users a different heiroglyphic before abandoning them. # Second, let's talk about the big picture Consider the following scenario: 1. a new developer adds some code to FFmpeg that calls an existing function 2. it turns out that function sometimes calls another function that returns a variety of internal error codes (FFERROR_REDO among others) 3. their testing uncovers a situation that intermittently returns an unknown error number, but they don't notice there are two different numbers 4. they spend a lot of time tracking down an error message based on a random number, and eventually fix "the" bug (actually one of two intermittent bugs) 5. the review doesn't catch the other bug, and the new code goes live 6. a user trips over the other bug and sees "Error number <number> occurred" 7. the user wastes a lot of time trying to work out what they did wrong, badmouthing FFmpeg to anyone who will listen as they do 8. they eventually catch the attention of a developer 9. that developer spends a lot of time bisecting the bug 10. the new developer is expected to fix this patch, and feels like they're to blame for the whole situation An error message like "Unrecognised error code \"REDO\" occurred, please report this bug" would give the newbie a fighting chance to catch both bugs at step 3, would make step 4 much shorter, and would optimise steps 7-10 to almost nothing. Catching this in a fate test would involve checking for an unknown function returning an unknown number that gets reused in a context it's subtly inappropriate for. I have no idea where to begin with that, and anyway it wouldn't help a developer in the process of tracking down an intermittent bug. As mentioned above, the v2 patch adds "please report this bug" in a few places. Any negative error code can be valid, but returning a raw error number is always a bug, so it's all the same to users - if they see this message, they're not expected to fix it themselves, they're expected to let us know. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2024-07-16 11:16 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-07-15 15:13 [FFmpeg-devel] [PATCH] " Andrew Sayers 2024-07-15 15:45 ` Marton Balint 2024-07-15 16:13 ` Andrew Sayers 2024-07-15 23:01 ` Marton Balint 2024-07-16 11:13 ` Andrew Sayers [this message] 2024-07-16 11:13 ` [FFmpeg-devel] [RFC PATCH v2] " Andrew Sayers 2024-07-17 21:06 ` Michael Niedermayer 2024-07-18 9:26 ` [FFmpeg-devel] [RFC PATCH v2 0/1] " Marton Balint 2024-07-18 10:46 ` [FFmpeg-devel] [PATCH v3 0/3] Protect against undocumented " Andrew Sayers 2024-07-18 10:46 ` [FFmpeg-devel] [PATCH v3 1/3] avutil/utils: Allow "!" in FourCCs Andrew Sayers 2024-07-18 15:42 ` Paul B Mahol 2024-07-18 10:46 ` [FFmpeg-devel] [PATCH v3 2/3] avutil/error: Provide better feedback about unknown error codes Andrew Sayers 2024-07-18 10:46 ` [FFmpeg-devel] [PATCH v3 3/3] tests/fate/source-check: Check for AVERROR codes without error strings Andrew Sayers 2024-07-18 11:16 ` [FFmpeg-devel] [RFC PATCH v2 0/1] avutil/error: Provide better feedback about unknown error codes Zhao Zhili 2024-07-17 6:25 ` [FFmpeg-devel] [PATCH] " Anton Khirnov
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=20240716111543.2437488-1-ffmpeg-devel@pileofstuff.org \ --to=ffmpeg-devel@pileofstuff.org \ --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