On Wed, 16 Apr 2014 09:00:15 +0200
Luca Barbato <[email protected]> wrote:
> ---
> libavresample/avresample.h | 54 +++++++++++++++++++++
> libavresample/utils.c | 118
> +++++++++++++++++++++++++++++++++++++++++++++
> libavutil/error.h | 2 +
> 3 files changed, 174 insertions(+)
>
> diff --git a/libavresample/avresample.h b/libavresample/avresample.h
> index 3358628..702a767 100644
> --- a/libavresample/avresample.h
> +++ b/libavresample/avresample.h
> @@ -96,6 +96,7 @@
> #include "libavutil/avutil.h"
> #include "libavutil/channel_layout.h"
> #include "libavutil/dict.h"
> +#include "libavutil/frame.h"
> #include "libavutil/log.h"
>
> #include "libavresample/version.h"
> @@ -410,6 +411,59 @@ int avresample_available(AVAudioResampleContext *avr);
> */
> int avresample_read(AVAudioResampleContext *avr, uint8_t **output, int
> nb_samples);
>
> +
> +
> +/**
> + * Convert the samples in the input AVFrame and write them to the output
> AVFrame.
> + *
> + * Input and output AVFrames must have channel_layout, sample_rate and
> format set.
> + *
> + * The upper bound on the number of output samples is given by
> + * avresample_available() + (avresample_get_delay() + number of input
> samples) *
> + * output sample rate / input sample rate.
This isn't an exact count and should specify rounding. If the result of
the division is not an integer, should it be rounded up or not? (If you
do defensive programming, you'd round it up always, but maybe it's
better not to rely on the user getting these things right.)
> + * If the output AVFrame does not have the data pointers allocated a the
> nb_samples
"a the" -> typo?
> + * will be set as described above and av_frame_get_buffer() will be called.
You should specify whether a memory pool is used or not - I imagine it
might matter for performance.
> + * The output AVFrame can be NULL or have fewer allocated samples than
> required.
> + *
> + * In this case, any remaining samples not written to the output will be
> added
> + * to an internal FIFO buffer, to be returned at the next call to this
> function
> + * or to avresample_convert() or to avresample_read().
> + *
> + * If converting sample rate, there may be data remaining in the internal
> + * resampling delay buffer. avresample_get_delay() tells the number of
> remaining
> + * samples. To get this data as output, call this function or
> avresample_convert()
> + * with NULL input.
> + *
> + * At the end of the conversion process, there may be data remaining in the
> + * internal FIFO buffer. avresample_available() tells the number of remaining
> + * samples. To get this data as output, either call this function or
> avresample_convert()
> + * with NULL input or call avresample_read().
> + *
> + * If the AVAudioResampleContext configuration does not match the output and
> input AVFrame
> + * settings the conversion does not take place and depending on which
> AVFrame is not
> + * matching AVERROR_OUTPUT_CHANGED, AVERROR_INPUT_CHANGED or
> + * AVERROR_OUTPUT_CHANGED|AVERROR_INPUT_CHANGED is returned.
You should also mention that the context needs to be initialized, and
_how_ (maybe just mentioning avresample_config() is enough).
IMO there should be a way to test whether the AVFrame configuration
changes, so that the user can do its own flushing.
As far as I understand, avresample_config() actually flushes
everything, so it's not enough. So maybe something like
int avresample_needs_config(AVFrame *in, AVFrame *out) should be added.
The user should be encouraged to actually do the checking, maybe even
provide example code for a proper conversion loop. Almost every API
user will need such a conversion loop, but it's probably better than
making the API trickier.
(Here's an alternative suggestion though: allow avresample_convert_frame
to accept partial input... then it could handle flushing itself easily,
though the user still needs to check whether the input frame was
consumed. It also would change the AVFrame data pointers to advance the
sample positions... well, maybe this suggestion is a bad idea.)
> + * @see avresample_available()
> + * @see avresample_convert()
> + * @see avresample_read()
> + * @see avresample_get_delay()
> + *
> + * @param avr audio resample context
> + * @param output output AVFrame
> + * @param input input AVFrame
> + * @return number of samples written to the output buffer,
> + * not including converted samples added to the
> internal
> + * output FIFO or an AVERROR.
As far as I can tell, it actually returns 0 on success, and sets
nb_samples on the output frame instead.
> + */
> +int avresample_convert_frame(AVAudioResampleContext *avr,
> + AVFrame *output, AVFrame *input);
> +
> +int avresample_config(AVAudioResampleContext *avr, AVFrame *out, AVFrame *in,
> + AVDictionary **opts);
Obviously missing doxygen. But it seems straightforward enough. I find
the opts parameter a bit strange. You could just set such options on
the avr context itself. I guess the question is, how do we handle if
certain parameters get moved to AVFrame: is the AVFrame value
preferred? Or the context options? Do the context options even indicate
always whether they were set by the user, or is it not possible to
distinguish set options from options that are just initialized to their
default value?
Depending on the answer, going with opts would be best, but then the
avr context AVOptions mechanism becomes at least partially
dangerous/useless: what happens if the user sets such an AVOption on
the context, but a later extension of AVFrame overrides it?
> +
> /**
> * @}
> */
> diff --git a/libavresample/utils.c b/libavresample/utils.c
> index 35bee42..37d9a63 100644
> --- a/libavresample/utils.c
> +++ b/libavresample/utils.c
> @@ -21,6 +21,7 @@
> #include "libavutil/common.h"
> #include "libavutil/dict.h"
> #include "libavutil/error.h"
> +#include "libavutil/frame.h"
> #include "libavutil/log.h"
> #include "libavutil/mem.h"
> #include "libavutil/opt.h"
> @@ -506,6 +507,123 @@ int attribute_align_arg
> avresample_convert(AVAudioResampleContext *avr,
> current_buffer);
> }
>
> +int avresample_config(AVAudioResampleContext *avr, AVFrame *out, AVFrame *in,
> + AVDictionary **opts)
> +{
> + int ret;
> +
> + if (avresample_is_open(avr)) {
> + avresample_close(avr);
> + }
This sure looks strange. Maybe avresample_close() should work even on a
closed context.
> + if (!out || !in)
> + return AVERROR(EINVAL);
> +
> + avr->in_channel_layout = in->channel_layout;
> + avr->out_channel_layout = out->channel_layout;
> + avr->in_sample_rate = in->sample_rate;
> + avr->out_sample_rate = out->sample_rate;
> + avr->in_sample_fmt = in->format;
> + avr->out_sample_fmt = out->format;
> +
> + if ((ret = av_opt_set_dict(avr, opts)) < 0)
> + return ret;
> +
> + return avresample_open(avr);
> +}
> +
> +static int config_changed(AVAudioResampleContext *avr,
> + AVFrame *out, AVFrame *in)
> +{
> + int ret = 0;
> +
> + if (in) {
> + if (avr->in_channel_layout != in->channel_layout ||
> + avr->in_sample_rate != in->sample_rate ||
> + avr->in_sample_fmt != in->format) {
> + ret |= AVERROR_INPUT_CHANGED;
> + }
> + }
> +
> + if (out) {
> + if (avr->out_channel_layout != out->channel_layout ||
> + avr->out_sample_rate != out->sample_rate ||
> + avr->out_sample_fmt != out->format) {
> + ret |= AVERROR_OUTPUT_CHANGED;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static inline int convert_frame(AVAudioResampleContext *avr,
> + AVFrame *out, AVFrame *in)
> +{
> + int ret;
> + uint8_t **out_data = NULL, **in_data = NULL;
> + int out_linesize = 0, in_linesize = 0;
> + int out_nb_samples = 0, in_nb_samples = 0;
> +
> + if (out) {
> + out_data = out->extended_data;
> + out_linesize = out->linesize[0];
> + out_nb_samples = out->nb_samples;
> + }
> +
> + if (in) {
> + in_data = in->extended_data;
> + in_linesize = in->linesize[0];
> + in_nb_samples = in->nb_samples;
> + }
> +
> + ret = avresample_convert(avr, out_data, out_linesize,
> + out_nb_samples,
> + in_data, in_linesize,
> + in_nb_samples);
> +
> + if (ret > 0) {
> + out->nb_samples = ret;
> + return 0;
> + }
> +
> + return ret;
> +}
> +
> +static inline int output_frame_samples(AVAudioResampleContext *avr,
> + AVFrame *out, AVFrame *in)
> +{
> + return avresample_available(avr) +
> + (avresample_get_delay(avr) + in->nb_samples) *
> + out->sample_rate / in->sample_rate;
> +}
> +
> +int avresample_convert_frame(AVAudioResampleContext *avr,
> + AVFrame *out, AVFrame *in)
> +{
> + int ret;
> +
> + if (!avresample_is_open(avr)) {
> + if ((ret = avresample_config(avr, out, in, NULL)) < 0)
> + return ret;
> + if ((ret = avresample_open(avr)) < 0)
> + return ret;
Doesn't avresample_config already open the context?
> + } else {
> + // return as is or reconfigure for input changes?
> + if ((ret = config_changed(avr, out, in)))
> + return ret;
> + }
> +
> + if (out && !out->linesize[0]) {
> + out->nb_samples = output_frame_samples(avr, out, in);
> + if ((ret = av_frame_get_buffer(out, 0)) < 0) {
> + avresample_close(avr);
> + return ret;
> + }
> + }
> +
> + return convert_frame(avr, out, in);
> +}
> +
> int avresample_get_matrix(AVAudioResampleContext *avr, double *matrix,
> int stride)
> {
> diff --git a/libavutil/error.h b/libavutil/error.h
> index 268a032..0a95d2e 100644
> --- a/libavutil/error.h
> +++ b/libavutil/error.h
> @@ -60,6 +60,8 @@
> #define AVERROR_BUG (-0x5fb8aabe) ///< Bug detected, please
> report the issue
> #define AVERROR_UNKNOWN (-0x31b4b1ab) ///< Unknown error,
> typically from an external library
> #define AVERROR_EXPERIMENTAL (-0x2bb2afa8) ///< Requested feature is
> flagged experimental. Set strict_std_compliance if you really want to use it.
> +#define AVERROR_INPUT_CHANGED (-0x636e6701) ///< Input changed between
> calls. Reconfiguration is required.
> +#define AVERROR_OUTPUT_CHANGED (-0x636e6702) ///< Output changed between
> calls. Reconfiguration is required.
It's very unusual that AVERROR codes can be bitwise or-ed, so maybe
this should be documented. What's with these random looking magic
numbers anyway?
> /**
> * Put a description of the AVERROR code errnum in errbuf.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
