Re: [FFmpeg-devel] [PATCH] liavcodec: add bit-rate support to RoQ video encoder

[Victor Luchitz <vluchits@gmail.com>]

I've just posted a new version of the patch that addresses most (if not
all) of the comments from previous reviews.

On Mon, Jan 22, 2024 at 10:10 PM Tomas Härdin <git@haerdin.se> wrote:

> mån 2024-01-22 klockan 21:32 +0300 skrev Victor Luchitz:
> > On Mon, Jan 22, 2024 at 4:57 PM Tomas Härdin <git@haerdin.se> wrote:
> >
> > > > >
> > > > > >  -    if (frame->quality)
> > > > > > -        enc->lambda = frame->quality - 1;
> > > > > > -    else
> > > > > > -        enc->lambda = 2*ROQ_LAMBDA_SCALE;
> > > > > > +    if (avctx->bit_rate <= ROQ_DEFAULT_MIN_BIT_RATE) {
> > > > > > +        /* no specific bit rate desired, use frame quality
> > > > > > */
> > > > > > +        if (frame->quality)
> > > > > > +            enc->lambda = frame->quality - 1;
> > > > > > +        else
> > > > > > +            enc->lambda = 2*ROQ_LAMBDA_SCALE;
> > > > > > +    }
> > > > >
> > > > > This looks like a bit of a janky way to switch between qscale
> > > > > and
> > > > > bitrate. Isn't there a way to detect whether an option has been
> > > > > set
> > > > > explicitly? At the very least this behavior should be
> > > > > documented in
> > > > > doc/encoders.texi
> > > > >
> > > >
> > > > Originally, the code just checked for bit_rate !=
> > > > AV_CODEC_DEFAULT_BITRATE,
> > > > which required including options_table.h, which in turn produced
> > > > a
> > > > bunch
> > > > of compilation warnings about certain fields being deprecated.
> > > > None
> > > > of the
> > > > other codecs include that file + many simply check the bit_rate
> > > > field
> > > > against
> > > > magic constants.
> > >
> > > grepping for 200000 didn't reveal anything like that. Do you have a
> > > specific example of an encoder that does this?
> > >
> >
> > ~/ffmpeg/libavcodec# grep -RI bit_rate . | grep 000 | grep -v 1000 |
> > wc -l
> > 38
>
> Most of these seem to revolve around sane defaults, picking various
> submodes or rejecting too high or too low bitrates
>
>
> > Most of the cases here are comparisons against magic constants
> > without
> > any comments about how they were chosen or what they do, none
> > whatsoever.
>
> I would suggest adding a different default as Martin suggests. If users
> really want shitty encodes I see no reason to deny them. I could see it
> being useful for modders.
>
> You might want an option for the tolerance stuff too
>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>


-- 
Best regards,
 Victor Luchitz
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".