GNOME Bugzilla – Bug 737138
audioencoder: weird error handling code path
Last modified: 2014-09-25 21:15:59 UTC
/* advance sample view */ if (G_UNLIKELY (samples * ctx->info.bpf > priv->offset)) { if (G_LIKELY (!priv->force)) { /* no way we can let this pass */ g_assert_not_reached (); /* really no way */ goto overflow; } else { The g_assert_not_reached () ensures that no one ever reaches overflow: { GST_ELEMENT_ERROR (enc, STREAM, ENCODE, ("received more encoded samples %d than provided %d", samples, priv->offset / ctx->info.bpf), (NULL)); if (buf) gst_buffer_unref (buf); ret = GST_FLOW_ERROR; goto exit; } 1) It would be nice to actually log what is wrong 2) and if we use the log line from overflow: make it more descriptive - at least I can't understand the "received more encoded samples %d than provided %d" - all the samples the class "receives" are "provided", right?
Mark, as you wrote this code. I appreciate any insights. The assert is triggered when encoding flac in ogg.
Note that the g_assert_if_reached() will be compiled out for releases.
It might be more descriptive if it mentions "received more encoded samples than provided input samples" or something like that, linguistic taste might vary ;-) And there is something to say for having some debug log in addition to an assert. So, perhaps we could move the g_assert_not_reached () to just before the 'goto exit' (indeed only having an effect in non-release cases). In any case, it must still be ensured that the subclass is not be claiming that the encoded data it has produced covers more (raw input) samples than have been provided to it in the first place. Well, it might happen at EOS or so, in which case the 'force' part will allow for that. Not clear on why this would happen with flac(enc) now. flacenc has been working for a long time as a subclass. Seems then like something else/more/intricate is going on (and a audiotestsrc ! flacenc ! fakesink still tests fine here).
For some additional data, in my case it is not audiotestsrc num-buffers=xxx but a seek with start and stop position. Lte me verify that eos is going through. Thanks for the explanation - I'll reshuffle the code in the base class.
commit 5f0aad6f42015da5182eee3bbd95f50b9cc7e26f Author: Stefan Sauer <ensonic@users.sf.net> Date: Tue Sep 23 11:56:33 2014 +0200 audioencoder: reshuffle code in error handling Move the assert to the error handling block at the end of the function so the the logging is still triggered. Reword the logging slightly and add another comment to hint what went wrong. Fixes #737138
For the record the issues I am seeing in flac is due to seeking in paused to configure the stop position (as seek in ready does not work any more - bug 733031). This way the encodebin prerolls and then gets the data again.