GNOME Bugzilla – Bug 440389
[API 0.11] Add GST_LEVEL_FIXME
Last modified: 2009-04-16 23:25:12 UTC
GST_LEVEL_FIXME should be a new debug level between GST_LEVEL_WARN and GST_LEVEL_ERROR. It's to be used for things that are yet unimplemented for various reasons. Examples: - features of closed formats that haven't been reverse engineered (think various flags in demuxers) - new features of extensible formats (think new 4CCs in AVI) - known issues that are hard to implement. Think elements that only implement subsets of complex formats (think SVG or SMIL) - incomplete or prototype elements All of these should allow the pipeline to continue working fine. But they will probably cause the result to not be as expected. Therefore it's an important information for people, in particular developers, looking for the failure. It should be enabled by default in releases. Rationale for adding this as a seperate debug level and not reusing existing ones: - Since it should be enabled in releases, the other category for these messages would be ERROR. However, errors are to be used for invalid files. - It is clearer to people reading debug output that this is a missing feature. They don't get scared by a big "ERROR" or "WARNING" sign. - It is easy to grep sources for FIXMEs when one wants to improve an existing element. - It encourages and documents a sane way to handle missing features. Currently the behaviour of elements in these cases varies from nothing over various debugging messages to g_warnings.
If a decoder properly handles a broken file, I don't think it should be spitting out GST_ERROR()s. It can either do a GST_ELEMENT_ERROR() and bail out, or fix up the output and send a discontinuity. It should probably also send a message indicating that the stream is broken. As for GST_FIXME, I think the label "FIXME" is independent of the severity of the issue, so it shouldn't be a specific severity. For example, the message "This code is not yet complete and could cause segfaults" would be a GST_ERROR, but "Somebody should spend a week sometime and fix this corner case that is really mostly correct right now anyway" is not. Also, having a FIXME is a natural occurrence in code. I wouldn't want to spew crap to the terminal all the time. Perhaps a FIXME in the message string would be a better start.
(In reply to comment #1) > If a decoder properly handles a broken file, I don't think it should be > spitting out GST_ERROR()s. It can either do a GST_ELEMENT_ERROR() and bail > out, or fix up the output and send a discontinuity. It should probably also > send a message indicating that the stream is broken. > I'll construct an example. Because it works so well for these kinds of issues, I'll use ID3v2, see http://www.id3.org/id3v2.4.0-structure for the standard. And I'll mostly use the nice method of unsynchronization (see chapter 6). Let's assume the following cases in a decoder: - The header sets the unsync flag, but the ID3 tag contains a frame without the sync flag set. - A frame has the unsync flag set, but encounters 2 consecutive bytes with data 0xFF. - For whatever reason, you didn't implement unsync yet but encounter a frame that has the unsync flag set. - The current frame is encrypted. The encoding specified (see http://www.id3.org/id3v2.4.0-frames 4.25) however is not implemented by your demuxer. - You encounter a frame that isn't matched with a corresponding GStreamer tag. What would you suggest to do in each of them? > As for GST_FIXME, I think the label "FIXME" is independent of the severity of > the issue, so it shouldn't be a specific severity. For example, the message > "This code is not yet complete and could cause segfaults" would be a GST_ERROR, > but "Somebody should spend a week sometime and fix this corner case that is > really mostly correct right now anyway" is not. > The first would be a bug. No code should ever crash, no matter if it's complete or correct. The point is that you want to hint at the developer what to do if a user files a bug because a user's file doesn't play back. If there's a /* FIXME: What to do if this flag is set? */ in the code somewhere, you'll probably never figure out the problem unless you step through the code with gdb. If the debug log contains a "FIXME: foo_bar_decode (245): What to do if this flag is set?" it'll be very easy to figure out what the problem is. > Also, having a FIXME is a natural occurrence in code. I wouldn't want to spew > crap to the terminal all the time. > It's not for every FIXME. It's for issues that you want to be notified about when they occur in the wild. Things such as /* FIXME: ugly */ or /* FIXME: slow */ can stay comments.
commit 116c8be6bfaac6c16a32e6b59949ebb085f1321b Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Mon Apr 6 01:27:26 2009 +0100 API: add FIXME and DUMPMEM log levels and convenience macros Two new log levels to dump FIXMEs into the log and to log data in form of a hex dump (#578114). API: GST_CAT_FIXME_OBJECT API: GST_CAT_MEMDUMP_OBJECT API: GST_CAT_FIXME API: GST_CAT_MEMDUMP API: GST_FIXME_OBJECT API: GST_MEMDUMP_OBJECT API: GST_FIXME API: GST_MEMDUMP *** This bug has been marked as a duplicate of 578114 ***