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

[wm4]

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.)