[FFmpeg-devel] [RFC] lavc/ffmpeg sample_fmt implementation
Michael Niedermayer
michaelni
Wed Jul 30 22:09:02 CEST 2008
On Mon, Jul 28, 2008 at 10:23:29PM +1000, pross at xvid.org wrote:
> On Sun, Jul 27, 2008 at 11:00:05PM +0200, Michael Niedermayer wrote:
> > On Sun, Jul 27, 2008 at 06:17:48PM +1000, pross at xvid.org wrote:
> > > On Sat, Jul 26, 2008 at 01:08:09AM +1000, pross at xvid.org wrote:
> > > > Hi.
> > > >
> > > > This patch adds sample_fmt conversion support to lavc (and ffmpeg).
> > >
> > > Round two patches enclosed.
> > >
>
> Round three.
>
> > > I noted that svq1dec is the only video *decoder* that populates
> > > pix_fmts. This seems unnecesary. Anyone have a view on this?
> >
> > hmmm, it could be used to avoid a close() function. The generic code could
> > that way initialize pix/sample_fmt.
>
> open() right?
yes
>
> -- Peter
> (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
> Index: ffmpeg.c
> ===================================================================
> --- ffmpeg.c (revision 14454)
> +++ ffmpeg.c (working copy)
[...]
> @@ -103,6 +104,7 @@
> static int frame_height = 0;
> static float frame_aspect_ratio = 0;
> static enum PixelFormat frame_pix_fmt = PIX_FMT_NONE;
> +static enum SampleFormat audio_sample_fmt = SAMPLE_FMT_NONE;
> static int frame_padtop = 0;
> static int frame_padbottom = 0;
> static int frame_padleft = 0;
[...]
> @@ -2453,13 +2489,13 @@
> }
> }
>
> -static void list_pix_fmts(void)
> +static void list_fmts(void (*get_fmt_string)(char *buf, int buf_size, int fmt), int nb_fmts)
> {
> int i;
> - char pix_fmt_str[128];
> - for (i=-1; i < PIX_FMT_NB; i++) {
> - avcodec_pix_fmt_string (pix_fmt_str, sizeof(pix_fmt_str), i);
> - fprintf(stdout, "%s\n", pix_fmt_str);
> + char fmt_str[128];
> + for (i=-1; i < nb_fmts; i++) {
> + get_fmt_string (fmt_str, sizeof(fmt_str), i);
> + fprintf(stdout, "%s\n", fmt_str);
> }
> }
>
> @@ -2468,7 +2504,7 @@
> if (strcmp(arg, "list"))
> frame_pix_fmt = avcodec_get_pix_fmt(arg);
> else {
> - list_pix_fmts();
> + list_fmts(avcodec_pix_fmt_string, PIX_FMT_NB);
> av_exit(0);
> }
> }
> @@ -2522,6 +2558,16 @@
> return 0;
> }
>
> +static void opt_audio_sample_fmt(const char *arg)
> +{
> + if (strcmp(arg, "list"))
> + audio_sample_fmt = avcodec_get_sample_fmt(arg);
> + else {
> + list_fmts(avcodec_sample_fmt_string, SAMPLE_FMT_NB);
> + av_exit(0);
> + }
> +}
> +
> static int opt_audio_rate(const char *opt, const char *arg)
> {
> audio_sample_rate = parse_number_or_die(opt, arg, OPT_INT64, 0, INT_MAX);
above can already be commited
[...]
> Index: libavcodec/utils.c
> ===================================================================
> --- libavcodec/utils.c (revision 14454)
> +++ libavcodec/utils.c (working copy)
[...]
> @@ -1148,6 +1149,10 @@
> enc->sample_rate,
> channels_str);
> }
> + if (enc->sample_fmt != SAMPLE_FMT_NONE) {
> + snprintf(buf + strlen(buf), buf_size - strlen(buf),
> + ", %s", avcodec_get_sample_fmt_name(enc->sample_fmt));
> + }
>
> /* for PCM codecs, compute bitrate directly */
> switch(enc->codec_id) {
so can this
> Index: libavcodec/Makefile
> ===================================================================
> --- libavcodec/Makefile (revision 14454)
> +++ libavcodec/Makefile (working copy)
> @@ -21,7 +21,7 @@
> utils.o \
>
>
> -HEADERS = avcodec.h opt.h
> +HEADERS = avcodec.h opt.h audioconvert.h
this is not ok, yet
Id like to wait with making the API public as much as possible to avoid
ending up with API breakage issues if we decide that we have to change
something. [that is it should all be working and have been tested before]
[...]
> Index: libavcodec/audioconvert.c
> ===================================================================
> --- libavcodec/audioconvert.c (revision 14454)
> +++ libavcodec/audioconvert.c (working copy)
> @@ -25,29 +25,98 @@
> * @author Michael Niedermayer <michaelni at gmx.at>
> */
>
> -#include "avcodec.h"
> +#include "audioconvert.h"
[...]
> +typedef struct SampleFmtInfo {
> + const char *name;
> + int bits;
> +} SampleFmtInfo;
> +
> +/** this table gives more information about formats */
> +static const SampleFmtInfo sample_fmt_info[SAMPLE_FMT_NB] = {
> + [SAMPLE_FMT_U8] = { .name = "u8", .bits = 8 },
> + [SAMPLE_FMT_S16] = { .name = "s16", .bits = 16 },
> + [SAMPLE_FMT_S24] = { .name = "s24", .bits = 24 },
> + [SAMPLE_FMT_S32] = { .name = "s32", .bits = 32 },
> + [SAMPLE_FMT_FLT] = { .name = "flt", .bits = 32 }
> +};
> +
> +const char *avcodec_get_sample_fmt_name(int sample_fmt)
> +{
> + if (sample_fmt < 0 || sample_fmt >= SAMPLE_FMT_NB)
> + return NULL;
> + return sample_fmt_info[sample_fmt].name;
> +}
> +
> +enum SampleFormat avcodec_get_sample_fmt(const char* name)
> +{
> + int i;
> +
> + for (i=0; i < SAMPLE_FMT_NB; i++)
> + if (!strcmp(sample_fmt_info[i].name, name))
> + return i;
> + return SAMPLE_FMT_NONE;
> +}
> +
> +void avcodec_sample_fmt_string (char *buf, int buf_size, int sample_fmt)
> +{
> + /* print header */
> + if (sample_fmt < 0)
> + snprintf (buf, buf_size, "name " " depth");
> + else if (sample_fmt < SAMPLE_FMT_NB) {
> + SampleFmtInfo info= sample_fmt_info[sample_fmt];
> + snprintf (buf, buf_size, "%-6s" " %2d ", info.name, info.bits);
> + }
> +}
that is also ok
> +
> +struct AVAudioConvert {
> + int in_channels, out_channels;
> + int isize,osize;
> + int fmt_pair;
> +};
> +
> +AVAudioConvert *av_audio_convert_alloc(enum SampleFormat in_fmt, int in_channels,
> + enum SampleFormat out_fmt, int out_channels,
> + const const float *matrix, int flags)
> +{
> + AVAudioConvert *ctx;
> + if (in_channels!=out_channels)
> + return NULL; /* FIXME: not supported */
> + ctx = av_malloc(sizeof(AVAudioConvert));
> + if (!ctx)
> + return NULL;
> + ctx->in_channels = in_channels;
> + ctx->out_channels = out_channels;
> + ctx->isize= FFMIN( in_fmt+1, 4);
> + ctx->osize= FFMIN(out_fmt+1, 4);
> + ctx->fmt_pair = out_fmt + SAMPLE_FMT_NB*in_fmt;
> + return ctx;
> +}
> +
> +void av_audio_convert_free(AVAudioConvert *ctx)
> +{
> + av_free(ctx);
> +}
> +
> +int av_audio_convert(AVAudioConvert *ctx,
> + void * const out[6], const int out_stride[6],
> + const void * const in[6], const int in_stride[6], int len)
> +{
> int ch;
> - const int isize= FFMIN( in_fmt+1, 4);
> - const int osize= FFMIN(out_fmt+1, 4);
> - const int fmt_pair= out_fmt + 5*in_fmt;
>
> //FIXME optimize common cases
>
> - for(ch=0; ch<6; ch++){
> - const int is= in_stride[ch] * isize;
> - const int os= out_stride[ch] * osize;
> + for(ch=0; ch<ctx->out_channels; ch++){
> + const int is= in_stride[ch] * ctx->isize;
> + const int os= out_stride[ch] * ctx->osize;
> uint8_t *pi= in[ch];
> uint8_t *po= out[ch];
> - uint8_t *end= po + os;
> + uint8_t *end= po + os*len;
> if(!out[ch])
> continue;
>
> #define CONV(ofmt, otype, ifmt, expr)\
> -if(fmt_pair == ofmt + 5*ifmt){\
> +if(ctx->fmt_pair == ofmt + SAMPLE_FMT_NB*ifmt){\
> do{\
> *(otype*)po = expr; pi += is; po += os;\
> }while(po < end);\
that is ok too but it should ideally be in a seperate commit
> Index: libavcodec/audioconvert.h
> ===================================================================
> --- libavcodec/audioconvert.h (revision 0)
> +++ libavcodec/audioconvert.h (revision 0)
> @@ -0,0 +1,91 @@
> +/*
> + * audio conversion
> + * Copyright (c) 2006 Michael Niedermayer <michaelni at gmx.at>
> + * Copyright (c) 2008 Peter Ross
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef FFMPEG_AUDIOCONVERT_H
> +#define FFMPEG_AUDIOCONVERT_H
> +
> +/**
> + * @file audioconvert.h
> + * Audio format conversion routines
> + */
> +
> +
> +#include "avcodec.h"
ok
> +
> +
> +/**
> + * Print in buf the string corresponding to the sample format with
print?
store, put, place, write, ... sounds better to me
> + * number sample_fmt, or an header if sample_fmt is negative.
> + *
> + * @param[in] buf the buffer where to write the string
> + * @param[in] buf_size the size of buf
> + * @param[in] sample_fmt the number of the sample format to print the corresponding info string, or
> + * a negative value to print the corresponding header.
> + * Meaningful values for obtaining a sample format info vary from 0 to SAMPLE_FMT_NB -1.
> + */
> +void avcodec_sample_fmt_string(char *buf, int buf_size, int sample_fmt);
> +
> +/**
> + * @return NULL on error
> + */
> +const char *avcodec_get_sample_fmt_name(int sample_fmt);
> +
> +/**
> + * @return SAMPLE_FMT_NONE on error
> + */
> +enum SampleFormat avcodec_get_sample_fmt(const char* name);
> +
> +struct AVAudioConvert;
> +typedef struct AVAudioConvert AVAudioConvert;
ok
> +
> +/**
> + * Create audio sample format converter context
Create AN audio ... .
> + * @param in_fmt Input sample format
> + * @param in_channels Number of input channels
> + * @param out_fmt Output sample format
> + * @param out_channels Number of output channels
> + * @param matrix Channel mixing matrix (of dimension in_channel*out_channels). Set to NULL to ignore.
> + * @param flags See FF_MM_xx
> + * @return NULL on error
> + */
> +AVAudioConvert *av_audio_convert_alloc(enum SampleFormat in_fmt, int in_channels,
> + enum SampleFormat out_fmt, int out_channels,
> + const float *matrix, int flags);
Please place out_fmt first so it matches av_audio_convert()
> +
> +/**
> + * Free audio sample format converter context
> + */
> +void av_audio_convert_free(AVAudioConvert *ctx);
> +
> +/**
> + * Convert between audio sample formats
> + * @param[in] out array of output buffers for each channel. set to NULL to ignore processing of the given channel.
> + * @param[in] out_stride distance between consecutive input samples (measured in samples)
Iam not sure if bytes would not be better ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080730/5a12b067/attachment.pgp>
More information about the ffmpeg-devel
mailing list