[FFmpeg-devel] [PATCH 1/2] lavf/dvenc: improve error messaging

[Anton Khirnov]

Quoting Stefano Sabatini (2024-01-21 11:30:27)
> > > -    if (((c->n_ast > 1) && (c->sys->n_difchan < 2)) ||
> > > -        ((c->n_ast > 2) && (c->sys->n_difchan < 4))) {
> > > -        /* only 2 stereo pairs allowed in 50Mbps mode */
> > > -        goto bail_out;
> > > +    if ((c->n_ast > 1) && (c->sys->n_difchan < 2)) {
> > > +        av_log(s, AV_LOG_ERROR,
> > > +               "Invalid number of channels %d, only 1 stereo pairs is allowed in 25Mps mode.\n",
> > > +               c->n_ast);
> > > +        return AVERROR_INVALIDDATA;
> > > +    }
> > > +    if ((c->n_ast > 2) && (c->sys->n_difchan < 4)) {
> > > +        av_log(s, AV_LOG_ERROR,
> > > +               "Invalid number of channels %d, only 2 stereo pairs are allowed in 50Mps mode.\n",
> > > +               c->n_ast);
> > > +        return AVERROR_INVALIDDATA;
> > 
> 
> > Surely this can be done in one log statement.
> 
> Yes, but this would complicate the logic for small gain.

More complicated than duplicating 5 lines? I wouldn't say so, not to
mention the string also has to be duplicated in the binary.

Also, can the second case even trigger? Seems like the block above
ensures n_ast is never larger than 2.

> > > -                    "     video: 25fps or 29.97fps, audio: 2ch/48|44|32kHz/PCM\n"
> 
> > > +                    "     video: 25fps or 29.97fps, audio: 2ch/48000|44100|32000Hz/PCM\n"
> > 
> > This does not seem like an improvement.
> 
> 44kHz != 44100
> 
> I could use 44.1 but this is not the unit used when setting the
> option

It can be.

-- 
Anton Khirnov