[FFmpeg-devel] [PATCH] Get rid of p

Michael Niedermayer michaelni
Sun Jun 8 22:09:52 CEST 2008


On Sun, Jun 08, 2008 at 08:57:21PM +0100, Ramiro Polla wrote:
> Michael Niedermayer wrote:
> > On Fri, Jun 06, 2008 at 05:09:27PM +0100, Ramiro Polla wrote:
> >> Hello,
> >>
> >> By the message subject you probably asked yourself "what 'p'?".
> >>
> >> Exactly, I ask myself the same question when reading the source.
> >>
> >> I find one-letter variable names hard to grep for and hard to understand. 
> >> Are patches to get rid of them or use more descriptive names welcome?
> >>
> >> Particular cases that come to my mind are:
> >>  - 's', used throughout FFmpeg to mean context. I suggest to change them to 
> >> ctx;
> >>  - 'p', used in video encoders for something that I still have no idea what 
> >> is doing there.
> >>
> >> Ramiro Polla
> > 
> >> Index: utils.c
> >> ===================================================================
> >> --- utils.c	(revision 13671)
> >> +++ utils.c	(working copy)
> >> @@ -2779,7 +2779,6 @@
> >>  
> >>  int64_t parse_date(const char *datestr, int duration)
> >>  {
> >> -    const char *p;
> >>      int64_t t;
> >>      struct tm dt;
> >>      int i;
> >> @@ -2808,12 +2807,11 @@
> >>  
> >>      memset(&dt, 0, sizeof(dt));
> >>  
> >> -    p = datestr;
> > 
> > int64_t parse_date(const char *p, int duration)
> > 
> > and your patch is much smaller
> 
> Hmmm... That goes against the whole purpose of my patch =)

The purpose being to annoy me? :)

It just factorizes the change into the unneeded variable copying removial
and the cosmetic renaming of datestr->p (which i probably wont accept)


> 
> > we can discuss the renaming then seperately ...
> > iam not sure if its good or not for the functions in your patch ...
> 
> I know my argumentation skills are close to NULL, but I'll try anyways...
> 
> Once you know what the function does, having small variable names might 
> be better, but if you're reading it for the first time to try and find 
> out what it does, having single-letter or non-descriptive names makes it 
> harder to understand. Or if you're debugging and stumble upon those 
> variables, it takes longer to find out what they actually are.
> 
> I use FFmpeg's source code as a basis to learn lots of things. I prefer 
> learning from well-written C code than textual descriptions like 
> textbooks or tutorials, and I'm probably not the only one.
> 

> Another reason is that searching for any of occurrence of some variable 
> becomes hard, since its name is part of many other variable names and 
> mostly anything.

get cscope


> 
> I found this specially painful when reading mpeg code for the mimic 
> encoder. Long functions with lots of s around...

would it really have helped if these where lots of ctx instead? If so
iam curious how?

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080608/b7679327/attachment.pgp>



More information about the ffmpeg-devel mailing list