On 1/12/16, Ganesh Ajjanagadde <gajja...@mit.edu> wrote: > On Tue, Jan 12, 2016 at 10:29 AM, Ronald S. Bultje <rsbul...@gmail.com> > wrote: >> Hi, >> >> On Tue, Jan 12, 2016 at 10:07 AM, Ganesh Ajjanagadde <gajja...@mit.edu> >> wrote: >> >>> On Tue, Jan 12, 2016 at 9:43 AM, Ronald S. Bultje <rsbul...@gmail.com> >>> wrote: >>> > Hi, >>> > >>> > On Tue, Jan 12, 2016 at 7:52 AM, Ganesh Ajjanagadde <gajja...@mit.edu> >>> > wrote: >>> > >>> >> On Tue, Jan 12, 2016 at 4:38 AM, wm4 <nfx...@googlemail.com> wrote: >>> >> > This makes no sense. Even if fclose() should fail for >>> >> > whatever obscure reasons there might be, reading already worked >>> >> > without errors, and thus closing failure has no meaning to use. >>> >> >>> >> Well, reading may not have worked, and the fclose may have been called >>> >> in a failure path. >>> > >>> > >>> > Then the error should be in the code path of fread(), not fclose(). >>> > Displaying an error (in whatever way) related to close while the actual >>> > problem was reading data is going to be confusing to the user. >>> >>> Read the rest of it; checking for every fread/fseek is not productive; >>> there are at least 3 of fread/fseek in the example I gave. Printing at >>> the time of closure is a natural means of doing things, again see: >>> https://www.gnu.org/ghm/2011/paris/slides/jim-meyering-goodbye-world.pdf >>> (particularly slide 7). >> >> >> He's very smart, but you still have to see his advice in context, also see >> [1]. > > The question of his smartness is irrelevant, as is the appeal to > authority definition here. > >> >> Most production uses of ffmpeg involve long-running processes in a >> multi-component pipeline, where fail-to-read will cause errors downstream. >> Reading the file from disk is only one small component. Let's say that I >> read a buffer of 10 bytes but I only got 5 because whatever went wrong. I >> parse the input buffer of 5 bytes, it's obviously corrupt, decoding stream >> goes wrong, and the decoder displays error ("ffvp9: corrupt input stream, >> failed to decode"). Note how this is the topmost error in stderr, >> therefore >> the user (logically) thinks: "FFmpeg's decoder sucks. FFmpeg sucks." >> That's >> bad. [2] > > If the fread is unchecked, that is a separate issue. At least for the > example I gave, the fread is checked. However, there is little (if > anything) that gets logged, since only the return value is set. The > question is where/what to log, if any. > > Instead of making such generic statements, please post comments to the > actual patches; the discussion will be more productive. > >> >> The first error should (ideally) be indicative of the actual problem. So, >> if the read is the error, the error should be associated with that read >> early on to help the user diagnose the actual source of the problem. > > Well, how about littering the code all over the place for every > read/seek/puts, etc so that your "ideal" can be met. > >> >> Ronald >> >> [1] https://en.wikipedia.org/wiki/Argument_from_authority >> [2] 10s or 1000s of bytes or frames failing to decode later, there's some >> little error saying the file descriptor failed to close. Did you ever look >> at line 1000 in your error log? > > There is something called grep. And ironically your proposed idea of > logging at the fread does not help with this either.
Why the stuff I post have no comments at all? But this marginal functionality is commented so much. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
