Re: [FFmpeg-devel] [PATCH 2/8] lavu: new AVWriter API

[Nicolas George <george@nsup.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1679 bytes --]

Rodney Baker (12023-04-28):
> I'm not normally a reviewer, but I noticed a few minor grammatical things that 
> stood out - hope this is OK. 

Thanks, it is absolutely useful.

> Nit - s/compating/comparing/

Fixed.

> > +**Note:** AVWriter is 8-bit clean, the strings it manipulates can be
> Use a hyphen or a semicolon rather than a comma after "clean". 

After consideration, a semicolon would be too strong; a hyphen would be
strange, too literary.

> > +In mainstream C, a function that needs to return a string usually have two
> > +options: either they accept pointer to a buffer that they fill or they
> > +allocate the buffer themselves and return it. Both these options have
> > +drawbacks, which one is best depends on the circumstances of the caller.
> Semicolon instead of comma after "drawbacks".

Same here, I find a semicolon would be too strong.

> Drop comma after "circumstances". 

Done.

> > +AVWriter also makes the work of the called function easier by providing
> > +convenient functions to append to the string that completely wrap error
> > +checks. Note that it only works for strings created as streams; functions
> > +that need random access to the string already built still need to manage
> > +their own buffers; some AVWriter implementations can still help for that.
> Full stop after "buffers" (instead of semicolon - you've already used one 
> previously in the same sentence). 

Disagree on this one: the last part should still be in the sentence that
starts with “Note that”. There is no problem in having multiple
semicolons to separate parts on the same level.

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".