[FFmpeg-devel] [PATCH] swr: general doxy text about swr and example code.

Michael Niedermayer michaelni at gmx.at
Tue Nov 20 19:48:18 CET 2012


On Tue, Nov 20, 2012 at 07:16:28PM +0100, Clément Bœsch wrote:
> On Mon, Nov 19, 2012 at 11:55:54PM +0100, Michael Niedermayer wrote:
> > Based on doxy from avr
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libswresample/swresample.h |   73 ++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 71 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libswresample/swresample.h b/libswresample/swresample.h
> > index 2ce666e..4a92268 100644
> > --- a/libswresample/swresample.h
> > +++ b/libswresample/swresample.h
> > @@ -18,13 +18,78 @@
> >   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> >   */
> >  
> > +#ifndef SWRESAMPLE_SWRESAMPLE_H
> > +#define SWRESAMPLE_SWRESAMPLE_H
> > +
> >  /**
> >   * @file
> >   * libswresample public header
> >   */
> >  
> > -#ifndef SWRESAMPLE_SWRESAMPLE_H
> > -#define SWRESAMPLE_SWRESAMPLE_H
> > +/**
> > + * @defgroup lswr Libswresample
> > + * @{
> > + *
> > + * Libswresample (lswr) is a library that handles audio resampling, sample
> > + * format conversion and mixing.
> > + *
> > + * Interaction with lswr is done through SwrContext, which is
> > + * allocated with swr_alloc() or swr_alloc_set_opts(). It is opaque, so all parameters
> > + * must be set with the @ref avoptions API.
> > + *
> > + * For example the following code will setup conversion from planar float sample
> > + * format to interleaved signed 16-bit integer, downsampling from 48kHz to
> > + * 44.1kHz and downmixing from 5.1 channels to stereo (using the default mixing
> > + * matrix):
> > + * @code
> > + * SwrContext *swr = swr_alloc();
> > + * av_opt_set_int(swr, "in_channel_layout",  AV_CH_LAYOUT_5POINT1, 0);
> > + * av_opt_set_int(swr, "out_channel_layout", AV_CH_LAYOUT_STEREO,  0);
> > + * av_opt_set_int(swr, "in_sample_rate",     48000,                0);
> > + * av_opt_set_int(swr, "out_sample_rate",    44100,                0);
> > + * av_opt_set_int(swr, "in_sample_fmt",      AV_SAMPLE_FMT_FLTP,   0);
> > + * av_opt_set_int(swr, "out_sample_fmt,      AV_SAMPLE_FMT_S16,    0);
> > + * @endcode
> > + *
> > + * Once all values have been set, it must be initialized with swr_init(). If
> > + * you need to change the conversion parameters, you can change the parameters
> > + * as described above, then call swr_init() again.
> > + *
> 
> or calling swr_alloc_set_opts() again?

yes, added


> 
> can we reset to default all the options just like the state after a
> swr_alloc()? (using av_opt_set_defaults on swr I guess?)

should be possible but i didnt try.


> 
> > + * The conversion itself is done by repeatedly calling swr_convert().
> > + * Note that the samples may get buffered in swr if you provide insufficient
> > + * output space or if sample rate conversion is done, which requires "future"
> > + * samples. Samples that do not require future input can be retrieved at any
> > + * time by using swr_convert() (in_count can be set to 0).
> > + * At the end of conversion the resampling buffer can be flushed by calling
> > + * swr_convert() with NULL in and 0 in_count.
> > + *
> > + * The delay between input and output, can at any time be found by using
> > + * swr_get_delay().
> > + *
> > + * The following code demonstrates the conversion loop assuming the parameters
> > + * from above and caller-defined functions get_input() and handle_output():
> > + * @code
> > + * uint8_t **input;
> > + * int in_linesize, in_samples;
> > + *
> > + * while (get_input(&input, &in_linesize, &in_samples)) {
> > + *     uint8_t *output
> > + *     int out_linesize;
> > + *     int out_samples = av_rescale_rnd(swr_get_delay(swr, 48000) +
> > + *                                      in_samples, 44100, 48000, AV_ROUND_UP);
> > + *     av_samples_alloc(&output, &out_linesize, 2, out_samples,
> > + *                      AV_SAMPLE_FMT_S16, 0);
> 
> If out_linesize can be NULL, we can simplify that example (and remove as
> well in_linesize). AFAIK it was in the example because lavr actually needs
> that duplicated information. lswr looks smarter, and doesn't need it. If
> you think the user might want such information, I'd better mention
> av_get_bytes_per_sample() in the doxy.

hmm, removed the linesizes


> 
> OTOH, some clarifications about the out_samples computation would be
> welcome; for example I'm curious in what case you would want to get a
> delay with a parameter different than the output sample rate (though, that
> is quiet unrelated to this example).

hmm, ill try to improve the swr_get_delay() doxy
or do you think it should be explained elsewhere ?


> 
> > + *     out_samples = swr_convert(swr, &output, out_samples,
> > + *                                      input, in_samples);
> > + *     handle_output(output, out_linesize, out_samples);
> > + *     av_freep(&output);
> > + *  }
> > + *  @endcode
> > + *
> > + *  When the conversion is finished and the FIFOs are flushed if required, the
> 
> The "if required" makes the sentence hard to understand here. I'd suggest
> to break it, maybe "When the conversion is finished, the conversion
> context and everything associated with it must be freed with swr_free().
> There will be no memory leak if the data is not completely flushed before
> swr_free()."

changed


> 
> > + *  conversion context and everything associated with it must be freed with
> > + *  swr_free().
> > + */
> >  
> >  #include <inttypes.h>
> 
> unrelated: stdint.h is not enough?

My principle with headers is, add if compile fails, its not failing.
Iam quite sure though we could invest time to make the includes a
tighter subset which then will cause failures and the need to add more
inlcudes earlier on future changes ...
In the end this smells like work to add more work.
Or is it faster even by little if stdint.h is used instead of
inttypes.h ?



> 
> >  #include "libavutil/samplefmt.h"
> > @@ -217,4 +282,8 @@ const char *swresample_configuration(void);
> >   */
> >  const char *swresample_license(void);
> >  
> > +/**
> > + * @}
> > + */
> > +
> >  #endif /* SWRESAMPLE_SWRESAMPLE_H */
> 
> Thanks a lot for the patch, apply anytime you think it's OK.

applied

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121120/8b93dd8d/attachment.asc>


More information about the ffmpeg-devel mailing list