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

2019-01-20 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Carl Eugen Hoyos
> Sent: Monday, January 21, 2019 10:13 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH]lavu/frame: Try to improve the
> documentation wording
> 
> 2019-01-21 3:06 GMT+01:00, Guo, Yejun :
> >
> >
> >> -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  >> 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 
> >> > 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
> 
> Didn't I ask you to improve it?

I got your point only after seeing this patch. And your patch is better.

> 
> Please send a patch, Carl Eugen

For the unchanged funky parts, I did try hard to make the description clear,
but can hardly improve the wording as a native speaker in a short time.
Anyway, before see an improvement patch, I'll keep it in my mind and improve it 
once I get new ideas for the rewording.

> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2019-01-20 Thread Carl Eugen Hoyos
2019-01-21 3:06 GMT+01:00, Guo, Yejun :
>
>
>> -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 > 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 
>> > 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

Didn't I ask you to improve it?

Please send a patch, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2019-01-20 Thread Guo, Yejun


> -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  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 
> > 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


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

2019-01-19 Thread Michael Niedermayer
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 
> 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.

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.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel