Re: [FFmpeg-devel] [RFC]] swscale modernization proposal

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 5066 bytes --]

On Fri, Jul 05, 2024 at 08:31:17PM +0200, Niklas Haas wrote:
> On Wed, 03 Jul 2024 15:25:58 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> > On Tue, 02 Jul 2024 15:27:00 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> >  
> > > 1. Is this a good idea, or too confusing / complex to be worth the gain?
> > >    Specifically, I am worried about confusion arising due to differences
> > >    in behavior, and implemented options, between all of the above.
> > > 
> > >    That said, I think there is a big win to be had from unifying all of
> > >    the different scaling and/or conversion filters we have in e.g.
> > >    libavfilter, as well as making it trivial for users of this API to
> > >    try using e.g. GPU scaling instead of CPU scaling.
> > 
> > After prototyping this approach a bit (using an internal struct
> > AVScaleBackend), I think I like it. It specifically makes handling
> > unscaled special converters pretty straightforward, for example - the
> > "unscaled" backend can be separate from the generic/scaling backend.
> > 
> > We could also trivially plug in something like libyuv, or some other
> > limited-use-case fast path, without the user really noticing.
> 
> Small update: I decided to scrap the idea of separate user-visible
> "backends" for now, but preserved the internal API boundary between the
> avscale_* "front-end" and the actual back-end implementation, which
> I have called 'AVScaleGraph' for now.
> 
> The idea is that this will grow into a full colorspace <-> colorspace
> "solver", but for now it is just hooked up to sws_createContext().
> 

> Attached is my revised working draft of <avscale.h>.

I dont agree to the renaming of swscale, that is heading toward


[...]
> /**
>  * The exact interpretation of these quality presets depends on the backend
>  * used, but the backend-invariant common settings are derived as follows:
>  */
> enum AVScaleQuality {
>     AV_SCALE_ULTRAFAST = 1,  /* no dither,      nearest+nearest     */
>     AV_SCALE_SUPERFAST = 2,  /* no dither,      bilinear+nearest    */
>     AV_SCALE_VERYFAST  = 3,  /* no dither,      bilinear+bilinear   */
>     AV_SCALE_FASTER    = 4,  /* bayer dither,   bilinear+bilinear   */
>     AV_SCALE_FAST      = 5,  /* bayer dither,   bicubic+bilinear    */
>     AV_SCALE_MEDIUM    = 6,  /* bayer dither,   bicubic+bicubic     */
>     AV_SCALE_SLOW      = 7,  /* bayer dither,   lanczos+bicubic     */
>     AV_SCALE_SLOWER    = 8,  /* full dither,    lanczos+bicubic     */
>     AV_SCALE_VERYSLOW  = 9,  /* full dither,    lanczos+lanczos     */
>     AV_SCALE_PLACEBO   = 10, /* full dither,    lanczos+lanczos     */

I dont think its a good idea to hardcode dither and the "FIR" filter to the quality level in the API


[...]
> /**
>  * Like `avscale_frame`, but operates only on the (source) range from `ystart`
>  * to `height`.
>  *
>  * @param ctx   The scaling context.
>  * @param dst   The destination frame. The data buffers may either be already
>  *              allocated by the caller or left clear, in which case they will
>  *              be allocated by the scaler. The latter may have performance
>  *              advantages - e.g. in certain cases some (or all) output planes
>  *              may be references to input planes, rather than copies.
>  * @param src   The source frame. If the data buffers are set to NULL, then
>  *              this function behaves identically to `avscale_frame_setup`.
>  * @param slice_start   First row of slice, relative to `src`. Must be a
>  *                      multiple of avscale_slice_alignment(src).
>  * @param slice_height  Number of (source) rows in the slice. Must be a
>  *                      multiple of avscale_slice_alignment(src).
>  *
>  * @return 0 on success, a negative AVERROR code on failure.
>  */
> int avscale_frame_slice(AVScaleContext *ctx, AVFrame *dst,
>                         const AVFrame *src, int slice_start, int slice_height);
> 
> /**
>  * Like `avscale_frame`, but without actually scaling. It will instead merely
>  * initialize internal state that *would* be required to perform the operation,
>  * as well as returning the correct error code for unsupported frame
>  * combinations.
>  *
>  * @param ctx   The scaling context.
>  * @param dst   The destination frame to consider.
>  * @param src   The source frame to consider.
>  * @return 0 on success, a negative AVERROR code on failure.
>  */
> int avscale_frame_setup(AVScaleContext *ctx, const AVFrame *dst,
>                         const AVFrame *src);

somewhat off topic as this is public API but
the swscale filtering code could internally use libavutil/executor.h
having filters and slices interdepend and need to execute them "in order"
and parallel, maybe that API is usefull, not sure, just wanted to mention it

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".