[FFmpeg-devel] [PATCH] Optimization: support for libx264's mb_info

[Carotti, Elias]

On Thu, 2023-06-22 at 10:44 +0200, Anton Khirnov wrote:
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender
> and know the content is safe.
> 
> 
> 
> Quoting Carotti, Elias (2023-06-21 17:53:09)
> > +
> > +    /**
> > +     * Provide macro block encoder-specific hinting information
> > for the encoder
> > +     * processing.  It can be used to pass information about which
> > macroblock
> > +     * can be skipped because it hasn't changed from the
> > corresponding one in
> > +     * the previous frame. This is useful for applications which
> > know in
> > +     * advance this information to speed up real-time encoding. 
> > Currently only
> > +     * used by libx264.
> 
> I'd avoid any such claims here, because this comment will certainly
> not
> be kept up to date.


Agreed. It was more a statement than a claim, since I only implemented
that :-)

> 
> > +/**
> > + * Allocate memory for a vector of AVVideoRect in the given
> > AVFrame
> > + * {@code frame} as AVFrameSideData of type
> > AV_FRAME_DATA_VIDEO_HINT_INFO.
> > + * The side data contains a list of rectangles for the portions of
> > the frame
> > + * which changed from the last encoded one (and the remainder are
> > assumed to be
> > + * changed), or, alternately (depending on the type parameter) the
> > unchanged
> > + * ones (and the remanining ones are those which changed).
> > + * Macroblocks will thus be hinted either to be P_SKIP-ped or go
> > through the
> > + * regular encoding procedure.
> > + */
> > +AVVideoHint *av_video_hint_create_side_data(AVFrame *frame,
> > +                                            AVVideoRect *rects,
> > +                                            size_t num_rects,
> > +                                            AVVideoHintType type);
> > +
> > +AVVideoHint *av_video_hint_alloc(AVVideoRect *rects,
> > +                                 size_t nb_rects,
> > +                                 AVVideoHintType type,
> > +                                 size_t *out_size);
> 
> If AVVideoHint is extended in the future, you will have a weird
> situation where some fields are set by the allocation function, while
> others have to be set manually by the caller.
> 
> You're also assuming the caller has an explicit array of AVVideoRect,
> while they may actually want to fill them in through some other
> means.

I agree on the first issue, and yes, it would be wiser to split the
allocation function from a the setting function. 
Would a simple append_rectangles (the name may be different) API work
for you?

I am not clear on the second issue you raise though: the thing is that
this side information is only needed per frame and before encoding, so
I do not see a use case where you keep adding rectangles to this side
data. The use case I see is where you accumulate the rectangles and
then feed them to the encoding function and forget about them, however,
again, if we add an append_rectangles we could easily extend it to the
use case you're hinting at.

> 
> Finally, it still seems to me this is largely duplicating
> AVVideoEncParams and you could get your functionality by adding your
> AVVideoHintType there.
> 

I disagree on this last point. My first idea to avoid duplicating or
adding unnecessary code was indeed to extend AVVideoEncParams. However,
(please correct me if I am wrong,) to my undestanding the
AVVideoEncParams are currently only used at the *decoder* side to
convey the encoding parameters (extracted from the bitstream) used by
the encoder to produce a stream while here we want to work the other
way around: at the encoder's side to provide hints on how to encode a
frame but won't affect the bitstream (aside from inducing faster
P_SKIPs generation.) and won't be known at the decoder's side.

So it seems to me it's a different semantics for which it's better to
have an appropriate side information.

Best, 
Elias
  

> --
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".




NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico