[FFmpeg-devel] [PATCH v2 1/8] swscale: document SWS_FULL_CHR_H_* flags

[Niklas Haas]

On Thu, 04 Jul 2024 19:56:56 +0200 Niklas Haas <ffmpeg at haasn.xyz> wrote:
> On Thu, 04 Jul 2024 16:24:24 +0100 Andrew Sayers <ffmpeg-devel at pileofstuff.org> wrote:
> > On Thu, Jul 04, 2024 at 04:30:57PM +0200, Niklas Haas wrote:
> > > From: Niklas Haas <git at haasn.dev>
> > > 
> > > Based on my best understanding of what they do, given the source code.
> > > ---
> > >  libswscale/swscale.h | 28 ++++++++++++++++++++++++++--
> > >  1 file changed, 26 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/libswscale/swscale.h b/libswscale/swscale.h
> > > index 9d4612aaf3..e22931cab4 100644
> > > --- a/libswscale/swscale.h
> > > +++ b/libswscale/swscale.h
> > > @@ -82,11 +82,35 @@ const char *swscale_license(void);
> > >  #define SWS_PRINT_INFO              0x1000
> > >  
> > >  //the following 3 flags are not completely implemented
> > > -//internal chrominance subsampling info
> > > +
> > > +/**
> > > + * Perform full chroma upsampling when converting to RGB as part of scaling.
> > 
> > Nitpick: "as part of scaling" seems redundant - can it be removed?
> 
> I wrote it this way because, afaict, this flag does not affect unscaled
> special converters (yuv->rgba). But I can remove it if you still think
> it's unnecessary.

How about: "Perform full chroma upsampling when upscaling to RGB"?

> 
> > 
> > > + *
> > > + * For example, when converting 50x50 yuv420p to 100x100 rgba, setting this flag
> > > + * will scale the chroma plane from 25x25 to 100x100 (4:4:4), and then convert
> > > + * the 100x100 yuv444p image to rgba in the final output step.
> > > + *
> > > + * Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
> > > + * with a single chroma sample being re-used for both horizontally adjacent RGBA
> > > + * output pixels.
> > 
> > Nitpick: this would be more readable as "for both of the...".
> > 
> > Consider the following sentence:
> > 
> >     Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
> >     with a single chroma sample being re-used for both horizontally and vertically
> >     adjacent RGBA output pixels.
> > 
> > Using "both of the" would make it clear what "both" refers to before the reader
> > starts doing branch-prediction in their head.
> 
> Fixed.
> 
> > 
> > Otherwise, LGTM (by which I mean it's clear, not that I know whether it's
> > correct).
> > _______________________________________________
> > 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".