[FFmpeg-devel] [RFC] Swscale refactor progress and feedback

[Niklas Haas]

On Sat, 28 Sep 2024 01:20:58 +0200 Michael Niedermayer <michael at niedermayer.cc> wrote:
> Hi
>
> On Fri, Sep 27, 2024 at 04:49:58PM +0200, Niklas Haas wrote:
> > Hi all,
> >
> > After a bit of a hiatus due to delays in negotioting the appropriate
> > contracts, I've finally been able to resume work on the swscale refactor
> > and have my current draft to demonstrate and gather critique on.
> >
> > Rather than the initial goal of introducing a new AVScale header, I have
> > updated my proposal to instead directly reuse the sws_* namespace. For now,
> > this unfortunately requires postfixing some colliding functions with a `2`
>
> > suffix, e.g. sws_alloc_context2, sws_scale_frame2 and so on.
>
> sws_context_alloc()
> sws_frame_scale()
> and you have no conflicts and no 2 postfix

I thought about something like this, but was worried that it would be too
confusing. In any case, if we have near-homonyms like this, we should absolutely
keep the types distict to ensure some degree of type safety.

If we decide to merge the two contexts into a single struct, this becomes
redundant anyways.

>
>
> [...]
> > So, all that being said, here are the biggest pain points I want feedback on:
> >
> > 1. How do we resolve the abiguity between SwsContext and SwsContext2?
>
> We arent using a AVCodecContext2 yet nor a AVFormatContext2, so i would
> expect given that SwsContext is not public that adding needed fields and
> deprecating and removing what becomes obsolete should work, unless the struct
> plays a different role in a new design.

The struct indeed plays a new role in the new design, namely that it is now
a fairly lightweight high-level context that is merely used to hold and
configure the underlying scaling graph.

While we can trivially merge the old and new contexts into a single struct,
it will not resolve the fundamental issue that there will be no support for
stateful functions like `sws_setColorspaceDetails` in the new design.

I'll continue thinking about it for now. We can easily change this sort of
superficial design decision before merging.

> [...]
> >
> > 2. How detailed / accurate do we want to preserve back compat with "legacy"
> >    swscale semantics? For example, swscale currently has some obscure flags
> >    and modes that I don't see as a high priority to maintain support for. But
> >    if we want the new API to be a strict improvement, we ought to maintain
> >    backwards compatibility in some form for all of these obscure modes.
> >    OTOH, now might be our biggest chance to revise what is actually needed.
> >
> >    For example, things I currently omit / imply, or could:
> >     - SWS_FULL_CHR_H_INP: I added a new flag SWS_FLAG_ALIAS which roughly
> >         corresponds to similar semantics, but in a more generalized fashion.
>
> >     - SWS_FULL_CHR_H_INT: turned on by default now, unless the user requests
> >         chroma point sampling. (Thought this does trigger a slow path in
> >         the underlying swscale legacy implementation for bgr8 etc)
>
> quality by default makes sense in 2024

Yes, I agree. I'll go ahead and make the new defaults sane then, at the cost
of needing to update a bunch of FATE tests. For what it's worth, I would also
like to take the opportunity to change the default scaler to a three tap
laczos, or at least a slightly sharper two tap spline. The current bicubic
default is very blurry and not used as a default in any of our major
competitors afaict.

>
> and flags like this should correspond to the implementation obviously. I mean
> if we have code that can do 2:1 downsampling by droping every 2nd sample
> (or something like that)
> then a flag to access this makes sense. If we dont have such code (anymore)
> then obviously such a flag should be deprecated.
> Such code should not be kept "because" of the flag. But it should be
> kept if it serves a purpose like being faster
>
>
> >     - Some of the more obscure scaling and dithering modes, such as
> >         "experimental", arithmatic dither, area scaling, etc.
>
> "experimental" can be droped unless its something that is used by a real group of
> real people.
> "area scaling" should just be different coefficients, so i dont see the benefit
> in droping it

Fair enough, I'll preserve all scalers for now, since they are nearly free
to keep around.

>
>
>
> >     - Support for setting custom yuv matrices or filter weights.
>
> why not ?
> if we have a well optimized codepath to apply such a matrix or filter,
> it should be accessable to the user for other purposes

Agree for custom scalers. For YUV matrices, I think the correct way to
implement this would require first defining a suitable set of AVFrame
properties for defining custom matrices. (For example the way Dolby Vision
currently handles it)

>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Breaking DRM is a little like attempting to break through a door even
> though the window is wide open and the only thing in the house is a bunch
> of things you dont want and which you would get tomorrow for free anyway
> _______________________________________________
> 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".