[FFmpeg-devel] [PATCH 1/3] Implement parse_preset_line() and use it in ffmpeg.c and ffserver.c.

[Michael Niedermayer]

On Mon, Nov 08, 2010 at 01:18:15PM +0100, Stefano Sabatini wrote:
> On date Sunday 2010-11-07 16:14:21 +0100, Michael Niedermayer encoded:
> > On Sun, Nov 07, 2010 at 10:48:38AM +0100, Stefano Sabatini wrote:
> > > On date Sunday 2010-11-07 01:06:36 +0100, Michael Niedermayer encoded:
> > > > On Sat, Nov 06, 2010 at 03:02:39PM +0100, Stefano Sabatini wrote:
> > > > > ---
> > > > >  cmdutils.c |   24 ++++++++++++++++++++++++
> > > > >  cmdutils.h |   15 +++++++++++++++
> > > > >  ffmpeg.c   |   14 ++++----------
> > > > >  ffserver.c |   12 ++----------
> > > > 
> > > > >  4 files changed, 45 insertions(+), 20 deletions(-)
> > > > 
> > > > if this is code factorization its making the code >2x larger instead of smaller
> > > 
> > > It's because of the docs and the function header overhead, also sscanf
> > 
> > that makes 15 lines, the patch is still worsening it by 50%
> > 
> > 
> > > doesn't allow to write in a string with given variable size, so I need
> > > to copy the string back to the user provided buffers.
> > > 
> > > But the point of the patch is not to reduce the line count, but
> > > avoiding code duplication (see also patch #2).
> > 
> > I understand that, it still doesnt look all that much like a improvment to me
> 
> bug_count/2, also I like to bury implementation details away from the
> application, that's why I like to move code from ff* tools to the lib
> or to the cmdutils.c file, as this reduces the exposed complexity when
> reading the application code.
> 
> When this is (maybe) increasing the overall complexity, it is still
> reducing the exposed complexity in the application code.

one hides a trivial thing behind a supposedly more trivial API
i dont think adding layers on layers is a good idea in this case
especially as you put a custom layer over 2 ANSI C sscanf(), later every
programmer will understand but noone will knoe what your custom function
does without finding and opening the file where that function resides

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101109/1ca3b175/attachment.pgp>