> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Michael Niedermayer > Sent: Sunday, January 20, 2019 6:39 AM > To: FFmpeg development discussions and patches <ffmpeg- > de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the > documentation wording > > On Fri, Jan 18, 2019 at 12:38:20PM +0100, Carl Eugen Hoyos wrote: > > Hi! > > > > Attached patch tries to improve the ROI documentation wording, C > > structs cannot return errors. > > > > Please comment, Carl Eugen > > > frame.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > 888a2470d43113b08b0ef3ddd03bda528f873ccc > > 0001-lavu-frame-Try-to-improve-the-documentation-wording.patch > > From 4abd545e7ab463c97bf816b270544eee25c4755b Mon Sep 17 00:00:00 > 2001 > > From: Carl Eugen Hoyos <ceffm...@gmail.com> > > Date: Fri, 18 Jan 2019 12:35:44 +0100 > > Subject: [PATCH] lavu/frame: Try to improve the documentation wording. > > > > C structs cannot return errors. > > --- > > libavutil/frame.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavutil/frame.h b/libavutil/frame.h index > > 8aa3e88..1990320 100644 > > --- a/libavutil/frame.h > > +++ b/libavutil/frame.h > > @@ -218,8 +218,8 @@ typedef struct AVFrameSideData { > > * if the codec requires an alignment. > > * If the regions overlap, the last value in the list will be used. > > * > > - * qoffset is quant offset, and base rule here: > > - * returns EINVAL if AVRational.den is zero. > > + * qoffset is quantization offset: > > + * Encoders will return EINVAL if qoffset.den is zero. > > * the value (num/den) range is [-1.0, 1.0], clamp to +-1.0 if out of > > range. > > * 0 means no picture quality change, > > * negative offset asks for better quality (and the best with value > > -1.0), > > The field specific documentation should be added to each of the fields using > correct doxygen syntax. > > About EINVAL, i would tend to document what is valid and what is not > instead of documenting what some code will do with invalid values. > The user has no reason to ever use invalid values so that information should > have no value. And just has some chance to be incorrect now or later > > Also the wording of the unchanged parts sound funky, some native english > speaker should check and reword more of this i think.
Sorry for the wording, hope it would be better. For the struct field doxygen syntax, I got the comment to be 'above the typedef', I probably mis-understood it. Will study doxygen syntax and be more careful about it in later patches. And don't plan to change this one since rewording is needed. > > thanks > > [...] > > > -- > Michael GnuPG fingerprint: > 9FF2128B147EF6730BADF133611EC787040B0FAB > > If you drop bombs on a foreign country and kill a hundred thousand innocent > people, expect your government to call the consequence "unprovoked > inhuman terrorist attacks" and use it to justify dropping more bombs and > killing more people. The technology changed, the idea is old. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel