[FFmpeg-devel] [PATCH] lavfi audio framework

Stefano Sabatini stefano.sabatini-lala
Wed Aug 11 18:11:10 CEST 2010


On date Wednesday 2010-08-11 06:54:59 -0700, S.N. Hemanth Meenakshisundaram encoded:
> Same lavfi audio patch, but now checking for ret == NULL here too before
> setting ret->type.
> 
> ---
>  doc/APIchanges         |    4 ++
>  libavfilter/avfilter.c |   56 ++++++++++++++++++++++++++++
>  libavfilter/avfilter.h |   93 +++++++++++++++++++++++++++++++++++++++++++---
>  libavfilter/defaults.c |   96 ++++++++++++++++++++++++++++++++++++++++++++++++
>  libavfilter/formats.c  |    3 +-
>  5 files changed, 245 insertions(+), 7 deletions(-)
> 
> 
> 

> diff --git a/doc/APIchanges b/doc/APIchanges
> index e6ec0b0..ee447b8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,10 @@ libavutil:   2009-03-08
>  
>  API changes, most recent first:
>  
> +2010-08-10 - r24734 - lavfi 1.32.0 - add audio framework support
> +  AVFilterBufferRefAudioProps for audio properties
> +  get_audio_buffer, filter_samples functions and related changes
> +
>  2010-08-09 - r24733 - lavfi 1.31.1 - AVFilterBufferRef
>    Move video specific properties to AVFilterBufferRefVideoProps
>  
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 17a2f7c..9e63969 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -21,6 +21,7 @@
>  
>  /* #define DEBUG */
>  
> +#include "libavcodec/audioconvert.h"
>  #include "libavcodec/imgconvert.h"
>  #include "libavutil/pixdesc.h"
>  #include "avfilter.h"
> @@ -52,6 +53,9 @@ AVFilterBufferRef *avfilter_ref_buffer(AVFilterBufferRef *ref, int pmask)
>      if(ref->type == AVMEDIA_TYPE_VIDEO) {
>          ret->video = av_malloc(sizeof(AVFilterBufferRefVideoProps));
>          *ret->video = *ref->video;

> +    } else if(ref->type == AVMEDIA_TYPE_AUDIO) {

nit if_(

> +        ret->audio = av_malloc(sizeof(AVFilterBufferRefAudioProps));
> +        *ret->audio = *ref->audio;

maybe we should add a check if ret->audio == NULL.

>      }
>      ret->perms &= pmask;
>      ret->buf->refcount ++;
> @@ -63,6 +67,7 @@ void avfilter_unref_buffer(AVFilterBufferRef *ref)
>      if(!(--ref->buf->refcount))
>          ref->buf->free(ref->buf);
>      av_free(ref->video);
> +    av_free(ref->audio);
>      av_free(ref);
>  }
>  
> @@ -219,6 +224,23 @@ AVFilterBufferRef *avfilter_get_video_buffer(AVFilterLink *link, int perms, int
>      return ret;
>  }
>  

> +AVFilterBufferRef *avfilter_get_audio_buffer(AVFilterLink *link, int perms, int size,
> +                                             int64_t channel_layout, enum SampleFormat sample_fmt, int planar)
> +{
> +    AVFilterBufferRef *ret = NULL;
> +
> +    if(link_dpad(link).get_audio_buffer)
> +        ret = link_dpad(link).get_audio_buffer(link, perms, size, channel_layout, sample_fmt, planar);

nit: if_( here and below

> +
> +    if(!ret)
> +        ret = avfilter_default_get_audio_buffer(link, perms, size, channel_layout, sample_fmt, planar);
> +
> +    if (ret)
> +        ret->type = AVMEDIA_TYPE_AUDIO;
> +
> +    return ret;
> +}
> +
>  int avfilter_request_frame(AVFilterLink *link)
>  {
>      FF_DPRINTF_START(NULL, request_frame); ff_dprintf_link(NULL, link, 1);
> @@ -339,6 +361,40 @@ void avfilter_draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
>      draw_slice(link, y, h, slice_dir);
>  }
>  
> +void avfilter_filter_samples(AVFilterLink *link, AVFilterBufferRef *samplesref)
> +{
> +    void (*filter_samples)(AVFilterLink *, AVFilterBufferRef *);
> +    AVFilterPad *dst = &link_dpad(link);
> +
> +    if (!(filter_samples = dst->filter_samples))
> +        filter_samples = avfilter_default_filter_samples;
> +
> +    /* prepare to copy the samples if the buffer has insufficient permissions */
> +    if ((dst->min_perms & samplesref->perms) != dst->min_perms ||
> +        dst->rej_perms & samplesref->perms) {
> +
> +        av_log(link->dst, AV_LOG_INFO,
> +               "Copying audio data in avfilter (have perms %x, need %x, reject %x)\n",
> +               samplesref->perms, link_dpad(link).min_perms, link_dpad(link).rej_perms);
> +
> +        link->cur_buf = avfilter_default_get_audio_buffer(link, dst->min_perms,
> +                                                          samplesref->audio->size,
> +                                                          samplesref->audio->channel_layout,
> +                                                          samplesref->format, samplesref->audio->planar);
> +        link->cur_buf->pts                = samplesref->pts;
> +        link->cur_buf->audio->sample_rate = samplesref->audio->sample_rate;
> +
> +        /* Copy actual data into new samples buffer */
> +        memcpy(link->cur_buf->data[0], samplesref->data[0], samplesref->audio->size);
> +
> +        avfilter_unref_buffer(samplesref);

> +    }
> +    else
> +        link->cur_buf = samplesref;

nit: } else

> +
> +    filter_samples(link, link->cur_buf);
> +}
> +
>  #define MAX_REGISTERED_AVFILTERS_NB 64
>  
>  static AVFilter *registered_avfilters[MAX_REGISTERED_AVFILTERS_NB + 1];
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 56455c5..87b9ae3 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -89,6 +89,21 @@ typedef struct AVFilterBuffer
>  #define AV_PERM_REUSE2   0x10   ///< can output the buffer multiple times, modified each time
>  
>  /**
> + * Audio specific properties in a reference to an AVFilterBuffer. Since
> + * AVFilterBufferRef is common to different media formats, audio specific
> + * per reference properties must be separated out.
> + */
> +
> +typedef struct AVFilterBufferRefAudioProps
> +{

Nit: skip the newline, "{" on the same line as "struct AVFilterBufferRefAudioProps".

> +    int64_t channel_layout;     ///< channel layout of audio buffer
> +    int samples_nb;             ///< number of audio samples
> +    int size;                   ///< audio buffer size
> +    uint32_t sample_rate;       ///< audio buffer sample rate
> +    int planar;                 ///< audio buffer - planar or packed
> +} AVFilterBufferRefAudioProps;
> +
> +/**
>   * Video specific properties in a reference to an AVFilterBuffer. Since
>   * AVFilterBufferRef is common to different media formats, video specific
>   * per reference properties must be separated out.
> @@ -114,7 +129,7 @@ typedef struct AVFilterBufferRefVideoProps
>  typedef struct AVFilterBufferRef
>  {
>      AVFilterBuffer *buf;                ///< the buffer that this is a reference to
> -    uint8_t *data[8];                   ///< picture data for each plane
> +    uint8_t *data[8];                   ///< picture/audio data for each plane/channel
>      int linesize[8];                    ///< number of bytes per line
>      int format;                         ///< media format
>  
> @@ -125,12 +140,12 @@ typedef struct AVFilterBufferRef
>  
>      enum AVMediaType type;              ///< media type of buffer data
>      AVFilterBufferRefVideoProps *video; ///< video buffer specific properties
> +    AVFilterBufferRefAudioProps *audio; ///< audio buffer specific properties
>  
>  } AVFilterBufferRef;
>  
>  /**

> - * Copy properties of src to dst, without copying the actual video
> - * data.
> + * Copy properties of src to dst, without copying the actual data

Nit: missing terminating ".".

>   */
>  static inline void avfilter_copy_buffer_ref_props(AVFilterBufferRef *dst, AVFilterBufferRef *src)
>  {
> @@ -142,6 +157,9 @@ static inline void avfilter_copy_buffer_ref_props(AVFilterBufferRef *dst, AVFilt
>      case AVMEDIA_TYPE_VIDEO:
>          *dst->video = *src->video;
>          break;
> +    case AVMEDIA_TYPE_AUDIO:
> +        *dst->audio = *src->audio;
> +        break;
>      }
>  }
>  
> @@ -340,7 +358,7 @@ struct AVFilterPad
>      void (*start_frame)(AVFilterLink *link, AVFilterBufferRef *picref);
>  
>      /**
> -     * Callback function to get a buffer. If NULL, the filter system will
> +     * Callback function to get a video buffer. If NULL, the filter system will
>       * use avfilter_default_get_video_buffer().
>       *
>       * Input video pads only.
> @@ -348,6 +366,16 @@ struct AVFilterPad
>      AVFilterBufferRef *(*get_video_buffer)(AVFilterLink *link, int perms, int w, int h);
>  
>      /**
> +     * Callback function to get an audio buffer. If NULL, the filter system will
> +     * use avfilter_default_get_audio_buffer().
> +     *
> +     * Input audio pads only.
> +     */
> +    AVFilterBufferRef *(*get_audio_buffer)(AVFilterLink *link, int perms,
> +                                           int size, int64_t channel_layout,
> +                                           enum SampleFormat sample_fmt, int planar);
> +
> +    /**
>       * Callback called after the slices of a frame are completely sent. If
>       * NULL, the filter layer will default to releasing the reference stored
>       * in the link structure during start_frame().
> @@ -365,6 +393,14 @@ struct AVFilterPad
>      void (*draw_slice)(AVFilterLink *link, int y, int height, int slice_dir);
>  
>      /**
> +     * Samples filtering callback. This is where a filter receives audio data
> +     * and should do its processing.
> +     *
> +     * Input audio pads only.
> +     */
> +    void (*filter_samples)(AVFilterLink *link, AVFilterBufferRef *samplesref);
> +
> +    /**
>       * Frame poll callback. This returns the number of immediately available
>       * frames. It should return a positive value if the next request_frame()
>       * is guaranteed to return one frame (with no delay).
> @@ -407,13 +443,20 @@ void avfilter_default_start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
>  void avfilter_default_draw_slice(AVFilterLink *link, int y, int h, int slice_dir);
>  /** default handler for end_frame() for video inputs */
>  void avfilter_default_end_frame(AVFilterLink *link);
> -/** default handler for config_props() for video outputs */
> +/** default handler for filter_samples() for audio inputs */
> +void avfilter_default_filter_samples(AVFilterLink *link, AVFilterBufferRef *samplesref);
> +/** default handler for config_props() for audio/video outputs */
>  int avfilter_default_config_output_link(AVFilterLink *link);
> -/** default handler for config_props() for video inputs */
> +/** default handler for config_props() for audio/video inputs */
>  int avfilter_default_config_input_link (AVFilterLink *link);
>  /** default handler for get_video_buffer() for video inputs */
>  AVFilterBufferRef *avfilter_default_get_video_buffer(AVFilterLink *link,
>                                                    int perms, int w, int h);
> +/** default handler for get_audio_buffer() for audio inputs */
> +AVFilterBufferRef *avfilter_default_get_audio_buffer(AVFilterLink *link, int perms,
> +                                                     int size, int64_t channel_layout,
> +                                                     enum SampleFormat sample_fmt, int planar);
> +
>  /**
>   * A helper for query_formats() which sets all links to the same list of
>   * formats. If there are no links hooked to this filter, the list of formats is
> @@ -432,10 +475,18 @@ void avfilter_null_draw_slice(AVFilterLink *link, int y, int h, int slice_dir);
>  /** end_frame() handler for filters which simply pass video along */
>  void avfilter_null_end_frame(AVFilterLink *link);
>  
> +/** filter_samples() handler for filters which simply pass audio along */
> +void avfilter_null_filter_samples(AVFilterLink *link, AVFilterBufferRef *samplesref);
> +
>  /** get_video_buffer() handler for filters which simply pass video along */
>  AVFilterBufferRef *avfilter_null_get_video_buffer(AVFilterLink *link,
>                                                    int perms, int w, int h);
>  

> +/** get_audio_buffer() handler for filters which simply pass audio along */
> +AVFilterBufferRef *avfilter_null_get_audio_buffer(AVFilterLink *link, int perms,
> +                                                  int size, int64_t channel_layout,
> +                                                  enum SampleFormat sample_fmt, int planar);
> +
>  /**
>   * Filter definition. This defines the pads a filter contains, and all the
>   * callback functions used to interact with the filter.
> @@ -525,8 +576,14 @@ struct AVFilterLink
>  
>      enum AVMediaType type;      ///< filter media type
>  
> +    /* These two parameters apply only to video */
>      int w;                      ///< agreed upon image width
>      int h;                      ///< agreed upon image height
> +    /* These three parameters apply only to audio */
> +    int samples_nb;             ///< number of samples in this buffer
> +    int64_t channel_layout;     ///< channel layout of current buffer (see avcodec.h)
> +    int64_t sample_rate;        ///< samples per second
> +
>      int format;                 ///< agreed upon media format
>  
>      /**
> @@ -582,6 +639,22 @@ AVFilterBufferRef *avfilter_get_video_buffer(AVFilterLink *link, int perms,
>                                            int w, int h);
>  
>  /**
> + * Request an audio samples buffer with a specific set of permissions.
> + *
> + * @param link           the output link to the filter from which the buffer will
> + *                       be requested
> + * @param perms          the required access permissions
> + * @param samples_nb     the number of samples in the buffer to allocate
> + * @param channel_layout the number and type of channels per sample in the buffer to allocate
> + * @param sample_fmt     the format of each sample in the buffer to allocate
> + * @return               A reference to the samples. This must be unreferenced with
> + *                       avfilter_unref_samples when you are finished with it.
> + */
> +AVFilterBufferRef *avfilter_get_audio_buffer(AVFilterLink *link, int perms,
> +                                             int size, int64_t channel_layout,
> +                                             enum SampleFormat sample_fmt, int planar);

Why is the format required by avfilter_default_get_audio_buffer() and
not by avfilter_default_get_video_buffer()?

Also I wonder if we shouldn't rather have:
AVFilterBufferRef *avfilter_default_get_audio_buffer(AVFilterLink *link, int perms, const AVFilterBufferRefAudioProps *props);

otherwise we're going to break API whenever a new field is added to
AVFilterBufferRefAudioProps.

That's in the case there are valid reason to believe that new fields
may be added, I cannot judge that right now. Michael?

> +/**
>   * Request an input frame from the filter at the other end of the link.
>   * @param link the input link
>   * @return     zero on success
> @@ -629,6 +702,14 @@ void avfilter_end_frame(AVFilterLink *link);
>   */
>  void avfilter_draw_slice(AVFilterLink *link, int y, int h, int slice_dir);
>  
> +/**
> + * Send a buffer of audio samples to the next filter.
> + *
> + * @param link   the output link over which the audio samples are being sent
> + * @param planar samples are packed if 0 or planar if 1
> + */
> +void avfilter_filter_samples(AVFilterLink *link, AVFilterBufferRef *samplesref);
> +
>  /** Initialize the filter system. Register all builtin filters. */
>  void avfilter_register_all(void);
>  
> diff --git a/libavfilter/defaults.c b/libavfilter/defaults.c
> index e275982..f682658 100644
> --- a/libavfilter/defaults.c
> +++ b/libavfilter/defaults.c
> @@ -20,6 +20,7 @@
>   */
>  
>  #include "libavcore/imgutils.h"
> +#include "libavcodec/audioconvert.h"
>  #include "avfilter.h"
>  
>  /* TODO: buffer pool.  see comment for avfilter_default_get_video_buffer() */
> @@ -66,6 +67,66 @@ AVFilterBufferRef *avfilter_default_get_video_buffer(AVFilterLink *link, int per
>      return ref;
>  }
>  

> +AVFilterBufferRef *avfilter_default_get_audio_buffer(AVFilterLink *link, int perms,
> +                                                     int size, int64_t channel_layout,
> +                                                     enum SampleFormat sample_fmt, int planar)
> +{
> +    AVFilterBuffer *buffer = av_mallocz(sizeof(AVFilterBuffer));
> +    AVFilterBufferRef *ref = av_mallocz(sizeof(AVFilterBufferRef));
> +    int i, sample_size, num_chans, bufsize, per_channel_size, step_size = 0;
> +    char *buf;
> +
> +    ref->buf                   = buffer;
> +    ref->format                = sample_fmt;

> +    ref->audio                 = av_mallocz(sizeof(AVFilterBufferRefAudioProps));
> +    ref->audio->channel_layout = channel_layout;
> +    ref->audio->size           = size;
> +    ref->audio->planar         = planar;

missing check?

> +
> +    /* make sure the buffer gets read permission or it's useless for output */
> +    ref->perms = perms | AV_PERM_READ;
> +
> +    buffer->refcount   = 1;
> +    buffer->free       = avfilter_default_free_buffer;
> +
> +    sample_size = av_get_bits_per_sample_format(sample_fmt) >>3;
> +    num_chans = avcodec_channel_layout_num_channels(channel_layout);
> +
> +    per_channel_size     = size/num_chans;
> +    ref->audio->samples_nb = per_channel_size/sample_size;
> +
> +    /* Set the number of bytes to traverse to reach next sample of a particular channel:
> +     * For planar, this is simply the sample size.
> +     * For packed, this is the number of samples * sample_size.
> +     */

> +    for (i = 0; i < num_chans; i++)
> +        buffer->linesize[i] = (planar > 0)?(per_channel_size):sample_size;

Nit:      buffer->linesize[i] =  planar > 0 ? per_channel_size : sample_size;

> +    memset(&buffer->linesize[num_chans], 0, (8-num_chans)*sizeof(buffer->linesize[0]));

Nit++: memset(&buffer->linesize[num_chans], 0, (8-num_chans) * sizeof(buffer->linesize[0]));
more readable to me.

> +    /* Calculate total buffer size, round to multiple of 16 to be SIMD friendly */
> +    bufsize = (size + 15)&~15;
> +    buf = av_malloc(bufsize);
> +

missing check

> +    /* For planar, set the start point of each channel's data within the buffer
> +     * For packed, set the start point of the entire buffer only
> +     */
> +    buffer->data[0] = buf;

> +    if (planar > 0) {
> +        for (i = 1; i < num_chans; i++) {
> +            step_size += per_channel_size;
> +            buffer->data[i] = buf + step_size;
> +        }
> +    } else

> +        memset(&buffer->data[1], (long)buf, (num_chans-1)*sizeof(buffer->data[0]));

why the (long) cast?

> +
> +    memset(&buffer->data[num_chans], 0, (8-num_chans)*sizeof(buffer->data[0]));

Same as above.

> +    memcpy(ref->data,     buffer->data,     sizeof(ref->data));
> +    memcpy(ref->linesize, buffer->linesize, sizeof(ref->linesize));
> +
> +    return ref;
> +}
> +
>  void avfilter_default_start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
>  {
>      AVFilterLink *out = NULL;
> @@ -110,6 +171,28 @@ void avfilter_default_end_frame(AVFilterLink *link)
>      }
>  }
>  
> +/* FIXME: samplesref is same as link->cur_buf. Need to consider removing the redundant parameter. */
> +void avfilter_default_filter_samples(AVFilterLink *link, AVFilterBufferRef *samplesref)
> +{
> +    AVFilterLink *out = NULL;

Please use more meaningful names, inlink and outlink should be fine.

[...]

Regards.
-- 
FFmpeg = Free and Fantastic Meaningful Pacific Enlightened Gigant



More information about the ffmpeg-devel mailing list