[FFmpeg-devel] [PATCH v5] Added Turing codec interface for ffmpeg

[wm4]

On Fri, 10 Feb 2017 15:48:55 +0000
Saverio Blasi <Saverio.Blasi at bbc.co.uk> wrote:

> > You probably shouldn't update options_buffer_size before the reallocation actually succeeded. (Probably doesn't matter with the current code, but for robustness...)  
> This is now addressed.
> 
> > Still not sure why there's at least 1 redundant field (s which is redundant with buffer_filled).  
> Truly s can be inferred from buffer_filled, but for readability and debugging purposes it is convenient to maintain both.s helps in keeping track of the current position within the options buffer. 

Can't really agree with this - redundant fields have the tendency to
become inconsistent, and if you use those fields for debugging it will
get even harder.

> > Not sure why this isn't just done by add_option.  
> This cannot be done inside add_option, because at the time of adding each option it is difficult to know how many options are passed to command line and consequently allocate argv

You don't know how long the "concatenated" option string is either. And
"concatenating" the options just to parse them back into single options
in finalize_options seems more work than just growing the argv array.