[FFmpeg-devel] [RFC] Direct Stream Transfer (DST) decoder

[Alexander Strasser]

On 2014-10-10 10:33 +0200, wm4 wrote:
> On Fri, 10 Oct 2014 09:57:20 +0200
> Alexander Strasser <eclipse7 at gmx.net> wrote:
> 
> > On 2014-10-09 08:13 +0200, Reimar Döffinger wrote:
> > > On 7 October 2014 08:41:09 CEST, Peter Ross <pross at xvid.org> wrote:
> > [...]
> > > >> > +    if ((ret = init_get_bits8(&gb, avpkt->data, avpkt->size)) < 0)
> > > >> > +    if ((ret = read_map(&gb, &fsets, map_ch_to_felem,
> > > >avctx->channels)) < 0)
> > > >> > +        if ((ret = read_map(&gb, &probs, map_ch_to_pelem,
> > > >avctx->channels)) < 0)
> > > >> 
> > > >> I still think putting assignments into an if is really bad idea all
> > > >> around.
> > > >
> > > >No exception for simple error return codes?
> > > 
> > > As Micheal said it's used a lot so no reason for rejecting a patch.
> > > However we had loads of bugs due to it, it is hard to read especially for people not that used to C and it only saves a single line.
> > > To me personally that's still a completely unreasonable risk/benefit ratio.
> > > Normally when we see a feature that is a repeated cause of bugs we avoid it. For some reason people are too attached to this one specifically.
> > > I still argue against it because I still believe it makes no sense to use it.
> > > Just my opinion though.
> > 
> >   +1
> > 
> >   I agree with Reimar.
> > 
> >   Harder to read, easier to get wrong. Even worse: if you get it wrong
> > it may even go unnoticed for a long time supported by the fact that
> > it is harder to read and thus people reading the code will usually not
> > notice any errors in such lines.
> > 
> >   To be sure: What I wrote above are general assertions, I am not
> > commenting on this patch set.
> > 
> 
> Strange, I thought it was the preferred idiom in ffmpeg. (I don't like
> it either, but submitted some patches using this.)

  Right, that idiom is used a lot in FFmpeg. That alone does not mean it
is preferred. Maybe preferred by some developers and contributors. IMHO
we do not need to be that strict and official about it, though at least I
would prefer to see the expanded version more often in the future.

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