[FFmpeg-devel] [FFmpeg-cvslog] parser: Remove commented-out cruft

[Alexander Strasser]

On 2014-02-26 08:14 +0100, Clément Bœsch wrote:
> On Tue, Feb 25, 2014 at 07:08:59PM +0100, Diego Biurrun wrote:
> > ffmpeg | branch: master | Diego Biurrun <diego at biurrun.de> | Tue Feb 25 11:59:05 2014 +0100| [ed61f3ca8a0664a697782253b354055136c5d303] | committer: Diego Biurrun
> > 
> > parser: Remove commented-out cruft
> > 
> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=ed61f3ca8a0664a697782253b354055136c5d303
> > ---
> > 
> >  libavcodec/parser.c |   11 ++---------
> >  1 file changed, 2 insertions(+), 9 deletions(-)
> > 
> > diff --git a/libavcodec/parser.c b/libavcodec/parser.c
> > index 511f1f3..e6743eb 100644
> > --- a/libavcodec/parser.c
> > +++ b/libavcodec/parser.c
> > @@ -97,8 +97,7 @@ void ff_fetch_timestamp(AVCodecParserContext *s, int off, int remove){
> >          if (   s->cur_offset + off >= s->cur_frame_offset[i]
> >              && (s->frame_offset < s->cur_frame_offset[i] ||
> >                (!s->frame_offset && !s->next_frame_offset)) // first field/frame
> > -            // check disabled since MPEG-TS does not send complete PES packets
> > -            && /*s->next_frame_offset + off <*/  s->cur_frame_end[i]){
> > +            && s->cur_frame_end[i]) {
> 
> Wasn't this "cruft" actually informative?
> 
> >              s->dts= s->cur_frame_dts[i];
> >              s->pts= s->cur_frame_pts[i];
> >              s->pos= s->cur_frame_pos[i];
> [...]
> 
> Also, in the next commit (yet another K&R cosmetics crap), I see that the
> "first field/frame" comment was discreetly discarded.
> 
> I'm not a maintainer of that file so I can't tell whether those comments
> were justified or not, but I'd suggest that everyone who get its code
> "prettified" by one of these apparently innocent looking commits has a
> closer cautious look at every such diff. It's definitely not the first
> time this is happening.

  I am neither maintaining this code, but I fully agree with your
assertions.

  And before anyone starts screaming it is recorded in VCS history, stop!
It is of course there, but it is nearly not detectable without having a
closer look at the individual explicit diffs.

  I honestly see no essence in changes like that! If you can state why
the comment is cruft and not needed anymore. Then say it explicitly
in the commit message with an elaborate explanation in the body of the
commit message.

  Just generating history clutter and potentially hindering future
examination and fixing of problems seems not really justifiable.

  Alexander
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140301/d2b07f86/attachment.asc>