[FFmpeg-devel] [PATCH]lavu/frame: Try to improve the documentation wording

[Guo, Yejun]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Sunday, January 20, 2019 6:39 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at 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 <ceffmpeg at 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.