[FFmpeg-devel] [PATCH] swscale: add API to convert AVFrames directly

Stefano Sabatini stefasab at gmail.com
Thu Oct 3 10:45:25 CEST 2013


On date Tuesday 2013-10-01 19:00:11 +0200, wm4 encoded:
> On Tue, 1 Oct 2013 13:34:28 +0200
> Stefano Sabatini <stefasab at gmail.com> wrote:
-> 
> > On date Tuesday 2013-10-01 01:10:59 +0200, wm4 encoded:
> > [...] 
> > > New patch attached. (I didn't run any extensive tests though - because
> > > it's a new API and nothing uses it.)
> > 
> > What about using it in doc/examples or in ff*.c code?
> 
> OK, patch for scaling_video.c included. Ideally, I'd like to make
> vf_scale.c use it, but that does weird things for supporting interlaced
> scaling - not sure if it's even possible to use sws_scale_frame here.
> 
> > > From 0dfefdaf1ccb9687a112ab45bb3a0a2bfff3102a Mon Sep 17 00:00:00 2001
> > > From: wm4 <nfxjfg at googlemail.com>
> > > Date: Sun, 29 Sep 2013 19:53:09 +0200
> > > Subject: [PATCH] swscale: add API to convert AVFrames directly
> > > 
> > > See sws_scale_frame() and the example in its doxygen comment.
> > > 
> > > This makes the libswscale API considerably easier to use. Various
> > > settings are automatically initialized from AVFrame settings, such as
> > > format, size, color space and color range parameters.
> > > ---
> > >  libswscale/swscale.h          |  83 ++++++++++++++++
> > >  libswscale/swscale_internal.h |  10 ++
> > >  libswscale/utils.c            | 213 +++++++++++++++++++++++++++++++++++++++++-
> > >  libswscale/version.h          |   2 +-
> > >  4 files changed, 306 insertions(+), 2 deletions(-)
> > 
> > Note to commmitter: add missing APIchanges entry
> 
> Added something.
> 
> [...]
> > > +int sws_set_src_filter(struct SwsContext *sws_context, SwsFilter *srcFilter);
> > > +
> > > +/**
> > > + * Set destination filter. This works only with sws_reinit_cached_context() and
> > > + * sws_scale_frame().
> > > + *
> > > + * @return zero or positive value on success, a negative value on
> > > + * error
> > > + */
> > > +int sws_set_dst_filter(struct SwsContext *sws_context, SwsFilter *dstFilter);
> > 
> > nit: weird mixing between camelStyle and snake_style.
> 
> sws_init_context() does this already, and I followed the existing
> conventions for consistency.
> 
> > > + *      AVFrame *src = ...;
> > > + *      SwsContext *sws = sws_alloc_context();
> > > + *      AVFrame *dst = av_frame_alloc();
> > > + *      if (!dst) goto error;
> > > + *      dst->format = AV_PIX_FMT_YUV420P;
> > > + *      dst->width = src->width * 2;
> > > + *      dst->height = src->height * 2;
> > > + *      if (av_frame_copy_props(dst, src) < 0) goto error; // don't change anything else
> > > + *      if (av_frame_get_buffer(dst, 32) < 0) goto error; // allocate image
> > > + *      if (sws_scale_frame(sws, dst, src) < 0) goto error;
> > > + *      // dst now contains the scaled image data
> > 
> > You should probably enclose this with @code ... @endcode.
> 
> OK. I forgot to address Clément's comments. He mentioned this too.
> 
> > > + * Warning: libswscale expects that dst is writable (see av_frame_is_writable()).
> > 
> > duplicated??
> 
> Where?

In the docs, I see av_frame_is_writable() mentioned already,

> 
> > > + *
> > > + * The SwsContext can be reused when converting the next frame. If the AVFrame
> > > + * parameters are different, libswscale is automatically reinitialized.
> > > + *
> > > + * @return zero or positive value on success, a negative value on error
> > > + */
> > > +int sws_scale_frame(struct SwsContext *sws_context, struct AVFrame *dst,
> > 
> > > +                    struct AVFrame *src);
> > 
> > const?
> 
> Added.
> 
> > > +
> > > +/**
> > >   * Free the swscaler context swsContext.
> > >   * If swsContext is NULL, then does nothing.
> > >   */
> > > @@ -303,6 +385,7 @@ SwsFilter *sws_getDefaultFilter(float lumaGBlur, float chromaGBlur,
> > >                                  float lumaSharpen, float chromaSharpen,
> > >                                  float chromaHShift, float chromaVShift,
> > >                                  int verbose);
> > > +SwsFilter *sws_cloneFilter(SwsFilter *filter);
> > 
> > please docs
> 
> Well, there wasn't any documentation for any of the SwsFilter
> related stuff... but added.
>  
> > >  void sws_freeFilter(SwsFilter *filter);
> > >  
> > >  /**
> > > diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
> > > index 33fdfc2..23f0966 100644
> > > --- a/libswscale/swscale_internal.h
> > > +++ b/libswscale/swscale_internal.h
> > > @@ -271,6 +271,16 @@ typedef struct SwsContext {
> > >      const AVClass *av_class;
> > >  
> > >      /**
> > > +     * All of these are used by sws_reinit_cached_context() only.
> > > +     */
> > > +    int force_reinit;
> > > +    struct SwsContext *previous_settings;
> > > +    SwsFilter *user_src_filter;
> > > +    SwsFilter *user_dst_filter;
> > 
> > > +    int user_srcRange;
> > > +    int user_dstRange;
> > 
> > nit: self-inconsistent style
> 
> Well, I'm not sure what to do. Sometimes libswscale uses camel case
> style, sometimes underscores. Changed user_src_filter to 
> user_srcFilter for now.

I'd prefer the other way around, user_srcRange -> user_src_range, or
userSrcRange. In general it would be better to get rid of camelStyle
for overall consistency. Anyway this is just a nit, so feel free to
ignore.

[...]
> > > +int sws_set_src_filter(struct SwsContext *sws_context, SwsFilter *srcFilter)
> > > +{
> > > +    SwsFilter *new_filter = sws_cloneFilter(srcFilter);
> > > +    if (srcFilter && !new_filter)
> > > +        return AVERROR(ENOMEM);
> > > +
> > > +    sws_freeFilter(sws_context->user_src_filter);
> > > +    sws_context->user_src_filter = new_filter;
> > > +
> > > +    sws_flush_cache(sws_context);
> > > +    return 0;
> > > +}
> > > +
> > > +int sws_set_dst_filter(struct SwsContext *sws_context, SwsFilter *dstFilter)
> > > +{
> > > +    SwsFilter *new_filter = sws_cloneFilter(dstFilter);
> > > +    if (dstFilter && !new_filter)
> > > +        return AVERROR(ENOMEM);
> > > +
> > > +    sws_freeFilter(sws_context->user_dst_filter);
> > > +    sws_context->user_dst_filter = new_filter;
> > > +
> > > +    sws_flush_cache(sws_context);
> > > +    return 0;
> > > +}
> > 
> > not happy about code duplication, what about a macro or a common
> > routine?
> 
> IMO a bit more awkward, but done.

Feel free to revert to the old version.

[...] 
> > > +static void sws_set_avframe_colorspace_table(int table[4],
> > > +                                             enum AVColorSpace colorspace)
> > > +{
> > > +    int i;
> > 
> > > +    int swscsp = colorspace; // this is libavfilter/vf_scale.c's dirty secret
> > > +    const int *sws_table = sws_getCoefficients(swscsp);
> > 
> > why the intermediary swscsp variable?
> > 
> > Also I'd prefer to avoid the reference to scale.
> 
> The first line converts AVColorSpace to libswscale colorspace
> constants, but they happen to use the same values - this is
> undocumented, but vf_scale.c relies on it, so...

Why don't you do:
sws_getCoefficients(colorspace)?

> 
> Anyway, replaced with a comment that doesn't mention vf_scale.c.
> 
> > > +    for (i = 0; i < 4; i++)
> > > +        table[i] = sws_table[i];
> > > +}
> > > +
> > 
> > > +static void sws_set_src_frame_options(struct SwsContext *c, struct AVFrame *src)
> > > +{
> > > +    c->srcFormat = src->format;
> > > +    c->srcW      = src->width;
> > > +    c->srcH      = src->height;
> > > +    c->srcRange  = src->color_range == AVCOL_RANGE_JPEG;
> > > +    sws_set_avframe_colorspace_table(c->srcColorspaceTable, src->colorspace);
> > > +}
> > > +
> > > +static void sws_set_dst_frame_options(struct SwsContext *c, struct AVFrame *dst)
> > > +{
> > > +    c->dstFormat = dst->format;
> > > +    c->dstW      = dst->width;
> > > +    c->dstH      = dst->height;
> > > +    c->dstRange  = dst->color_range == AVCOL_RANGE_JPEG;
> > > +    sws_set_avframe_colorspace_table(c->dstColorspaceTable, dst->colorspace);
> > > +}
> > 
> > Could be probably macroized/factored, and merged with sws_set_avframe_colorspace_table().
> 
> I'd rather not, because the result would be quite awkward. If the image
> parameters weren't a loose bunch of variables, this would be easily
> possible: e.g. libswscale would use AVFrame directly, or there would be
> a type that summarizes image format information. But it doesn't, so
> you would need a macro that appends "src" or "dst" to a variable name,
> and any attempt to factor out this code would be awkward. I'd rather
> duplicate 5 lines of code just one time.

OK.
 
> > [...]
> 
> New patch attached.

> From 8385653d5e9c02af4def70010dddde0be11cf4a5 Mon Sep 17 00:00:00 2001
> From: wm4 <nfxjfg at googlemail.com>
> Date: Sun, 29 Sep 2013 19:53:09 +0200
> Subject: [PATCH 1/2] swscale: add API to convert AVFrames directly
> 
> See sws_scale_frame() and the example in its doxygen comment.
> 
> This makes the libswscale API considerably easier to use. Various
> settings are automatically initialized from AVFrame settings, such as
> format, size, color space and color range parameters.
> ---
>  doc/APIchanges                |   4 +
>  libswscale/swscale.h          |  89 ++++++++++++++++++
>  libswscale/swscale_internal.h |  10 ++
>  libswscale/utils.c            | 209 +++++++++++++++++++++++++++++++++++++++++-
>  libswscale/version.h          |   2 +-
>  5 files changed, 312 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index ab932c3..31097d2 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,10 @@ libavutil:     2012-10-22
>  
>  API changes, most recent first:
>  
> +2013-10-xx - xxxxxxx - lsws 2.6.100 -
> +  Add the following functions: sws_reinit_cached_context(), sws_scale_frame(),
> +  sws_set_src_filter(), sws_set_dst_filter(), sws_cloneFilter().
> +
>  2013-08-xx - xxxxxxx - lavfi 3.11.0 - avfilter.h
>    Add AVFilterGraph.execute and AVFilterGraph.opaque for custom slice threading
>    implementations.
> diff --git a/libswscale/swscale.h b/libswscale/swscale.h
> index 42702b7..0fb5212 100644
> --- a/libswscale/swscale.h
> +++ b/libswscale/swscale.h
> @@ -39,6 +39,8 @@

>  #include "libavutil/pixfmt.h"
>  -#include "version.h"

spurious?

>  
> +struct AVFrame;
> +
>  /**
>   * Return the LIBSWSCALE_VERSION_INT constant.
>   */
> @@ -157,6 +159,8 @@ int sws_isSupportedEndiannessConversion(enum AVPixelFormat pix_fmt);
>   * Allocate an empty SwsContext. This must be filled and passed to
>   * sws_init_context(). For filling see AVOptions, options.c and
>   * sws_setColorspaceDetails().
> + *
> + * Note that sws_scale_frame() will fill the SwsContext itself.
>   */
>  struct SwsContext *sws_alloc_context(void);
>  
> @@ -169,6 +173,86 @@ struct SwsContext *sws_alloc_context(void);
>  int sws_init_context(struct SwsContext *sws_context, SwsFilter *srcFilter, SwsFilter *dstFilter);
>  
>  /**
> + * Set source filter. This works only with sws_reinit_cached_context() and
> + * sws_scale_frame().
> + *
> + * @return zero or positive value on success, a negative value on
> + * error
> + */
> +int sws_set_src_filter(struct SwsContext *sws_context, SwsFilter *srcFilter);
> +
> +/**
> + * Set destination filter. This works only with sws_reinit_cached_context() and
> + * sws_scale_frame().
> + *
> + * @return zero or positive value on success, a negative value on
> + * error
> + */
> +int sws_set_dst_filter(struct SwsContext *sws_context, SwsFilter *dstFilter);
> +
> +/**
> + * Check for parameter changes, and reinitialize the swscale context if needed.
> + * You should call this before each sws_scale() call, unless you know for sure
> + * you did not change any swscale parameters since the last sws_scale() call.
> + *
> + * Note that you don't need to call sws_init_context() with this function
> + * (although it is possible). sws_setColorspaceDetails() is optional as well.
> + *
> + * Some functions (like sws_setColorspaceDetails() and sws_set_src/dst_filter())
> + * are always considered parameter changes.
> + *
> + * @return zero or positive value on success, a negative value on
> + * error
> + */
> +int sws_reinit_cached_context(struct SwsContext *sws_context);
> +

> +/**
> + * Scale and convert image data in src to the dimension and image parameters
> + * in dst. You must initialize dst and allocate image data before calling this
> + * function. In particular, you must set the image format and image dimensions
> + * on dst prior to calling this function.
> + *
> + * @warning libswscale expects that dst is writable (see av_frame_is_writable()).
> + *          The reason this is not done automatically is that this would not
> + *          allow the user to provide a static destination buffer (as the
> + *          AVFrame API does not consider these writable).
> + *
> + * @warning this will transparently reinitialize the sws context, and overwrite
> + *          some swscale options according to AVFrame. This includes settings
> + *          like source/destination image dimensions, pixel format, color space,
> + *          color range, and possibly more. Should AVFrame be extended to carry
> + *          more image parameters in the future, this function might be updated
> + *          to include them as well. This is usually convenient and the right
> + *          thing to do, but if you do not want libswscale to overwrite your
> + *          settings, use the low-level API and not sws_scale_frame(). (You can
> + *          also just modify the AVFrame fields before passing them to this
> + *          function.)

I would personally integrate the warning in the main text, since they
give information about how the function works (in particular in case
src/dst changes between separate calls).

> + *
> + * Example how to scale a given "src" image to 2x the size and 8 bit 4:2:0 YCbCr:
> + *
> + * @code
> + * AVFrame *src = ...;
> + * SwsContext *sws = sws_alloc_context();
> + * AVFrame *dst = av_frame_alloc();
> + * if (!dst) goto error;
> + * dst->format = AV_PIX_FMT_YUV420P;
> + * dst->width = src->width * 2;
> + * dst->height = src->height * 2;
> + * if (av_frame_copy_props(dst, src) < 0) goto error; // don't change anything else
> + * if (av_frame_get_buffer(dst, 32) < 0) goto error; // allocate image
> + * if (sws_scale_frame(sws, dst, src) < 0) goto error;
> + * // dst now contains the scaled image data
> + * @endcode
> + *
> + * The SwsContext can be reused when converting the next frame. If the AVFrame
> + * parameters are different, libswscale is automatically reinitialized.
> + *
> + * @return zero or positive value on success, a negative value on error
> + */
> +int sws_scale_frame(struct SwsContext *sws_context, struct AVFrame *dst,
> +                    const struct AVFrame *src);
> +
> +/**
>   * Free the swscaler context swsContext.
>   * If swsContext is NULL, then does nothing.
>   */
> @@ -306,6 +390,11 @@ SwsFilter *sws_getDefaultFilter(float lumaGBlur, float chromaGBlur,
>  void sws_freeFilter(SwsFilter *filter);
>  
>  /**
> + * Allocate and return a deep copy of the given filter.
> + */
> +SwsFilter *sws_cloneFilter(SwsFilter *filter);
> +
> +/**
>   * Check if context can be reused, otherwise reallocate a new one.
>   *
>   * If context is NULL, just calls sws_getContext() to get a new
> diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
> index 33fdfc2..b3ea0b0 100644
> --- a/libswscale/swscale_internal.h
> +++ b/libswscale/swscale_internal.h
> @@ -271,6 +271,16 @@ typedef struct SwsContext {
>      const AVClass *av_class;
>  
>      /**
> +     * All of these are used by sws_reinit_cached_context() only.
> +     */
> +    int force_reinit;
> +    struct SwsContext *previous_settings;
> +    SwsFilter *user_srcFilter;
> +    SwsFilter *user_dstFilter;
> +    int user_srcRange;
> +    int user_dstRange;
> +
> +    /**
>       * Note that src, dst, srcStride, dstStride will be copied in the
>       * sws_scale() wrapper so they can be freely modified here.
>       */
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index a2e3ce1..ac94ac3 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -46,6 +46,7 @@
>  #include "libavutil/mathematics.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/frame.h"
>  #include "libavutil/ppc/cpu.h"
>  #include "libavutil/x86/asm.h"
>  #include "libavutil/x86/cpu.h"
> @@ -964,6 +965,9 @@ int sws_setColorspaceDetails(struct SwsContext *c, const int inv_table[4],
>  {
>      const AVPixFmtDescriptor *desc_dst;
>      const AVPixFmtDescriptor *desc_src;
> +
> +    c->force_reinit = 1;
> +
>      memmove(c->srcColorspaceTable, inv_table, sizeof(int) * 4);
>      memmove(c->dstColorspaceTable, table, sizeof(int) * 4);
>  
> @@ -1106,6 +1110,8 @@ av_cold int sws_init_context(SwsContext *c, SwsFilter *srcFilter,
>      const AVPixFmtDescriptor *desc_src;
>      const AVPixFmtDescriptor *desc_dst;
>  
> +    c->force_reinit = 1;
> +
>      cpu_flags = av_get_cpu_flags();
>      flags     = c->flags;
>      emms_c();
> @@ -1964,7 +1970,33 @@ void sws_freeFilter(SwsFilter *filter)
>      av_free(filter);
>  }
>  
> -void sws_freeContext(SwsContext *c)
> +SwsFilter *sws_cloneFilter(SwsFilter *filter)
> +{
> +    SwsFilter *new_filter;
> +    if (!filter)
> +        return NULL;
> +
> +    new_filter = av_malloc(sizeof(SwsFilter));
> +    if (!new_filter)
> +        return NULL;
> +
> +    new_filter->lumH = sws_cloneVec(filter->lumH);
> +    new_filter->lumV = sws_cloneVec(filter->lumV);
> +    new_filter->chrH = sws_cloneVec(filter->chrH);
> +    new_filter->chrV = sws_cloneVec(filter->chrV);
> +
> +    if (!new_filter->lumH || !new_filter->lumV ||
> +        !new_filter->chrH || !new_filter->chrV) {
> +        sws_freeFilter(new_filter);
> +        return NULL;
> +    }
> +
> +    return new_filter;
> +}
> +
> +// Free and reset almost everything in the SwsContext, but do not wipe user
> +// settings.
> +static void sws_uninit_context(SwsContext *c)
>  {
>      int i;
>      if (!c)
> @@ -2027,7 +2059,19 @@ void sws_freeContext(SwsContext *c)
>  
>      av_freep(&c->yuvTable);
>      av_freep(&c->formatConvBuffer);
> +}
>  
> +void sws_freeContext(SwsContext *c)
> +{
> +    if (!c)
> +        return;
> +
> +    sws_uninit_context(c);
> +    sws_freeFilter(c->user_srcFilter);
> +    sws_freeFilter(c->user_dstFilter);
> +    c->user_srcFilter = NULL;
> +    c->user_dstFilter = NULL;
> +    av_freep(&c->previous_settings);
>      av_free(c);
>  }
>  
> @@ -2078,3 +2122,166 @@ struct SwsContext *sws_getCachedContext(struct SwsContext *context, int srcW,
>      }
>      return context;
>  }
> +
> +static int set_filter(struct SwsContext *c, SwsFilter **var,  SwsFilter *filter)
> +{
> +    SwsFilter *newFilter = sws_cloneFilter(filter);
> +    if (filter && !newFilter)
> +        return AVERROR(ENOMEM);
> +
> +    sws_freeFilter(*var);
> +    *var = newFilter;
> +
> +    c->force_reinit = 1;
> +    return 0;
> +}
> +
> +int sws_set_src_filter(struct SwsContext *c, SwsFilter *srcFilter)
> +{
> +    return set_filter(c, &c->user_srcFilter, srcFilter);
> +}
> +
> +int sws_set_dst_filter(struct SwsContext *c, SwsFilter *dstFilter)
> +{
> +    return set_filter(c, &c->user_dstFilter, dstFilter);
> +}
> +

> +static int compare_colorspace_table(int table1[4], int table2[4])
> +{
> +    int i;
> +    for (i = 0; i < 4; i++) {
> +        if (table1[i] != table2[i])
> +            return 0;
> +    }
> +    return 1;
> +}

Could be this replaced by memcmp(table1, table2, sizeof(int)*4)?

> +
> +// Return 1 if nothing changed between new and old, and 0 if it did.
> +// This checks user settings only.
> +static int can_reuse(struct SwsContext *new, struct SwsContext *old)
> +{
> +    return new->flags           == old->flags &&         // swscale_options
> +           new->srcW            == old->srcW &&
> +           new->srcH            == old->srcH &&
> +           new->dstW            == old->dstW &&
> +           new->dstH            == old->dstH &&
> +           new->srcFormat       == old->srcFormat &&
> +           new->dstFormat       == old->dstFormat &&
> +           new->srcRange        == old->srcRange &&
> +           new->dstRange        == old->dstRange &&
> +           new->param[0]        == old->param[0] &&
> +           new->param[1]        == old->param[1] &&
> +           new->src_v_chr_pos   == old->src_v_chr_pos &&
> +           new->src_h_chr_pos   == old->src_h_chr_pos &&
> +           new->dst_v_chr_pos   == old->dst_v_chr_pos &&
> +           new->dst_h_chr_pos   == old->dst_h_chr_pos &&
> +           new->dither          == old->dither &&
> +           new->brightness      == old->brightness &&    // sws_setColorspaceDetails
> +           new->contrast        == old->contrast &&
> +           new->saturation      == old->saturation &&
> +           compare_colorspace_table(new->srcColorspaceTable, old->srcColorspaceTable) &&
> +           compare_colorspace_table(new->dstColorspaceTable, old->dstColorspaceTable);
> +}
> +
> +// Does a shallow copy of all fields that are user settings.
> +static void copy_settings(struct SwsContext *dst, struct SwsContext *src)
> +{
> +    dst->user_srcFilter  = src->user_srcFilter;
> +    dst->user_dstFilter  = src->user_dstFilter;
> +    dst->flags           = src->flags;
> +    dst->srcW            = src->srcW;
> +    dst->srcH            = src->srcH;
> +    dst->dstW            = src->dstW;
> +    dst->dstH            = src->dstH;
> +    dst->srcFormat       = src->srcFormat;
> +    dst->dstFormat       = src->dstFormat;
> +    dst->srcRange        = src->srcRange;
> +    dst->dstRange        = src->dstRange;
> +    dst->param[0]        = src->param[0];
> +    dst->param[1]        = src->param[1];
> +    dst->src_v_chr_pos   = src->src_v_chr_pos;
> +    dst->src_h_chr_pos   = src->src_h_chr_pos;
> +    dst->dst_v_chr_pos   = src->dst_v_chr_pos;
> +    dst->dst_h_chr_pos   = src->dst_h_chr_pos;
> +    dst->dither          = src->dither;
> +    dst->brightness      = src->brightness;
> +    dst->contrast        = src->contrast;
> +    dst->saturation      = src->saturation;
> +    memcpy(dst->srcColorspaceTable, src->srcColorspaceTable, sizeof(int) * 4);
> +    memcpy(dst->dstColorspaceTable, src->dstColorspaceTable, sizeof(int) * 4);
> +}
> +
> +int sws_reinit_cached_context(struct SwsContext *c)
> +{
> +    int ret = 0;
> +    if (!c->previous_settings) {
> +        c->force_reinit = 1;
> +        if (!(c->previous_settings = av_malloc(sizeof(struct SwsContext))))
> +            return AVERROR(ENOMEM);
> +    }
> +    if (!c->contrast && !c->saturation) {
> +        c->contrast = 1 << 16;
> +        c->saturation = 1 << 16;
> +    }
> +    if (c->force_reinit || !can_reuse(c, c->previous_settings)) {
> +        struct SwsContext *previous_settings;
> +        previous_settings = c->previous_settings;
> +        *previous_settings = *c;
> +        sws_uninit_context(c);
> +        memset(c, 0, sizeof(*c));
> +        c->av_class = &sws_context_class;
> +        copy_settings(c, previous_settings);
> +        c->previous_settings = previous_settings;
> +        sws_setColorspaceDetails(c, c->srcColorspaceTable, c->srcRange,
> +                                 c->dstColorspaceTable, c->dstRange,
> +                                 c->brightness, c->contrast, c->saturation);
> +        ret = sws_init_context(c, c->user_srcFilter, c->user_dstFilter);
> +        if (ret < 0)
> +            return ret;
> +        c->force_reinit = 0;
> +    }
> +    return ret;
> +}
> +
> +static void sws_set_avframe_colorspace_table(int table[4],
> +                                             enum AVColorSpace colorspace)
> +{
> +    int i;
> +    // Note: AVColorSpace and SWS_CS_... use the same values
> +    const int *sws_table = sws_getCoefficients(colorspace);
> +    for (i = 0; i < 4; i++)
> +        table[i] = sws_table[i];

memcpy? Or are you afraid the mapping could change?

> +}
> +
> +static void sws_set_src_frame_options(struct SwsContext *c,
> +                                      const struct AVFrame *src)
> +{
> +    c->srcFormat = src->format;
> +    c->srcW      = src->width;
> +    c->srcH      = src->height;
> +    c->srcRange  = src->color_range == AVCOL_RANGE_JPEG;
> +    sws_set_avframe_colorspace_table(c->srcColorspaceTable, src->colorspace);
> +}
> +
> +static void sws_set_dst_frame_options(struct SwsContext *c,
> +                                      const struct AVFrame *dst)
> +{
> +    c->dstFormat = dst->format;
> +    c->dstW      = dst->width;
> +    c->dstH      = dst->height;
> +    c->dstRange  = dst->color_range == AVCOL_RANGE_JPEG;
> +    sws_set_avframe_colorspace_table(c->dstColorspaceTable, dst->colorspace);
> +}
> +
> +int sws_scale_frame(struct SwsContext *sws_context, struct AVFrame *dst,
> +                    const struct AVFrame *src)
> +{
> +    int ret;

> +    sws_set_src_frame_options(sws_context, src);
> +    sws_set_dst_frame_options(sws_context, dst);

nit: you could probably inline the code

> +    if ((ret = sws_reinit_cached_context(sws_context)) < 0)
> +        return ret;
> +    ret = sws_scale(sws_context, (const uint8_t *const *)src->data,
> +                    src->linesize, 0, src->height, dst->data, dst->linesize);
> +    return ret;
> +}
> diff --git a/libswscale/version.h b/libswscale/version.h
> index 06f119a..72c6074 100644
> --- a/libswscale/version.h
> +++ b/libswscale/version.h
> @@ -27,7 +27,7 @@
>  #include "libavutil/avutil.h"
>  
>  #define LIBSWSCALE_VERSION_MAJOR 2
> -#define LIBSWSCALE_VERSION_MINOR 5
> +#define LIBSWSCALE_VERSION_MINOR 6
>  #define LIBSWSCALE_VERSION_MICRO 100
>  
>  #define LIBSWSCALE_VERSION_INT  AV_VERSION_INT(LIBSWSCALE_VERSION_MAJOR, \
> -- 
> 1.8.4.rc3
> 

> From 35a4296eae2caca92cbefe4a7681c808c60e979c Mon Sep 17 00:00:00 2001
> From: wm4 <nfxjfg at googlemail.com>
> Date: Tue, 1 Oct 2013 18:47:51 +0200
> Subject: [PATCH 2/2] examples: scaling_video: work with AVFrame, use
>  sws_scale_frame()
> 
> To reduce the general weirdness, don't allocate and scale to an
> unaligned image (which prints a libswscale warning about unaligned
> data), but instead write the output file line-wise.
> ---
>  doc/examples/scaling_video.c | 65 +++++++++++++++++++++++---------------------
>  1 file changed, 34 insertions(+), 31 deletions(-)
> 
> diff --git a/doc/examples/scaling_video.c b/doc/examples/scaling_video.c
> index be2c510..d4ea2e8 100644
> --- a/doc/examples/scaling_video.c
> +++ b/doc/examples/scaling_video.c
> @@ -26,8 +26,9 @@
>   * @example doc/examples/scaling_video.c
>   */
>  
> -#include <libavutil/imgutils.h>
> +#include <libavutil/pixdesc.h>
>  #include <libavutil/parseutils.h>
> +#include <libavutil/frame.h>
>  #include <libswscale/swscale.h>
>  
>  static void fill_yuv_image(uint8_t *data[4], int linesize[4],
> @@ -51,16 +52,14 @@ static void fill_yuv_image(uint8_t *data[4], int linesize[4],
>  
>  int main(int argc, char **argv)
>  {
> -    uint8_t *src_data[4], *dst_data[4];
> -    int src_linesize[4], dst_linesize[4];
>      int src_w = 320, src_h = 240, dst_w, dst_h;
>      enum AVPixelFormat src_pix_fmt = AV_PIX_FMT_YUV420P, dst_pix_fmt = AV_PIX_FMT_RGB24;
>      const char *dst_size = NULL;
>      const char *dst_filename = NULL;
>      FILE *dst_file;
> -    int dst_bufsize;
> -    struct SwsContext *sws_ctx;
> +    struct SwsContext *sws_ctx = NULL;
>      int i, ret;
> +    AVFrame *src = NULL, *dst = NULL;
>  
>      if (argc != 3) {
>          fprintf(stderr, "Usage: %s output_file output_size\n"
> @@ -86,56 +85,60 @@ int main(int argc, char **argv)
>          exit(1);
>      }
>  
> -    /* create scaling context */
> -    sws_ctx = sws_getContext(src_w, src_h, src_pix_fmt,
> -                             dst_w, dst_h, dst_pix_fmt,
> -                             SWS_BILINEAR, NULL, NULL, NULL);
> -    if (!sws_ctx) {
> -        fprintf(stderr,
> -                "Impossible to create scale context for the conversion "
> -                "fmt:%s s:%dx%d -> fmt:%s s:%dx%d\n",
> -                av_get_pix_fmt_name(src_pix_fmt), src_w, src_h,
> -                av_get_pix_fmt_name(dst_pix_fmt), dst_w, dst_h);
> -        ret = AVERROR(EINVAL);
> +    src = av_frame_alloc();
> +    src->format = src_pix_fmt;
> +    src->width = src_w;
> +    src->height = src_h;

> +    if ((ret = av_frame_get_buffer(src, 32)) < 0) {
> +        fprintf(stderr, "Could not allocate source image\n");

image -> frame

>          goto end;
>      }

Nit:
#define ALIGN 32

av_frame_get_buffer(src, ALIGN)

to decrease the magic.

>  
> -    /* allocate source and destination image buffers */
> -    if ((ret = av_image_alloc(src_data, src_linesize,
> -                              src_w, src_h, src_pix_fmt, 16)) < 0) {
> +    dst = av_frame_alloc();
> +    dst->format = dst_pix_fmt;
> +    dst->width = dst_w;
> +    dst->height = dst_h;
> +    if ((ret = av_frame_get_buffer(dst, 32)) < 0) {
>          fprintf(stderr, "Could not allocate source image\n");

source image -> destination frame

>          goto end;
>      }
>  
> -    /* buffer is going to be written to rawvideo file, no alignment */
> -    if ((ret = av_image_alloc(dst_data, dst_linesize,
> -                              dst_w, dst_h, dst_pix_fmt, 1)) < 0) {
> -        fprintf(stderr, "Could not allocate destination image\n");
> +    sws_ctx = sws_alloc_context();

> +    if (!sws_ctx) {
> +        fprintf(stderr, "Could not allocate SwsContext\n");

Could not allocate the scaler context.

>          goto end;
>      }
> -    dst_bufsize = ret;
>  
>      for (i = 0; i < 100; i++) {
> +        int y;
> +
>          /* generate synthetic video */
> -        fill_yuv_image(src_data, src_linesize, src_w, src_h, i);
> +        fill_yuv_image(src->data, src->linesize, src->width, src->height, i);
>  
>          /* convert to destination format */
> -        sws_scale(sws_ctx, (const uint8_t * const*)src_data,
> -                  src_linesize, 0, src_h, dst_data, dst_linesize);

> +        if ((ret = sws_scale_frame(sws_ctx, dst, src)) < 0) {
> +            fprintf(stderr, "Could not initialize scaler for the conversion "

Failed to perform the scale conversion ...

> +                    "fmt:%s s:%dx%d -> fmt:%s s:%dx%d\n",
> +                    av_get_pix_fmt_name(src_pix_fmt), src_w, src_h,
> +                    av_get_pix_fmt_name(dst_pix_fmt), dst_w, dst_h);
> +            goto end;
> +        }
>  

>          /* write scaled image to file */
> -        fwrite(dst_data[0], 1, dst_bufsize, dst_file);
> +        /* assumes non-planar format with 3 bytes per pixel */
> +        for (y = 0; y < dst->height; y++)
> +            fwrite(dst->data[0] + y * dst->linesize[0], dst->width * 3, 1, dst_file);
>      }
>  
>      fprintf(stderr, "Scaling succeeded. Play the output file with the command:\n"
> -           "ffplay -f rawvideo -pix_fmt %s -video_size %dx%d %s\n",
> +           "ffplay -f rawvideo -pixel_format %s -video_size %dx%d %s\n",

why this?

>             av_get_pix_fmt_name(dst_pix_fmt), dst_w, dst_h, dst_filename);
>  
>  end:
>      if (dst_file)
>          fclose(dst_file);
> -    av_freep(&src_data[0]);
> -    av_freep(&dst_data[0]);
> +    av_frame_free(&src);
> +    av_frame_free(&dst);
>      sws_freeContext(sws_ctx);
>      return ret < 0;
>  }

Overall, the example is useful when using the AVFrame API, but not the
low level API. I would like to hear other people opinions if we should
favor the AVFrame API in place of the low-level buffer-based one (or
maybe if we should keep both of them in separate examples).
-- 
FFmpeg = Fundamental Freak MultiPurpose EniGma


More information about the ffmpeg-devel mailing list