[FFmpeg-devel] [RFC] lavc/ffmpeg sample_fmt implementation
Michael Niedermayer
michaelni
Fri Jul 25 23:32:33 CEST 2008
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).
>
> The sample_fmt behavior is modelled on pix_fmt. Codecs will need minor
> modification to support sample_fmt correctly. Namely, decoders need to
> report their output sample format, and encoders validate the input sample
> format (or publish a list of supported formats via the AVCodec struct).
> The PCM codecs are modified in this patch, and the more useful ones have
> been tested.
First id like to say that iam very happy that someone is finally working
on the sample_fmt issue :)
now the review ...
[...]
> @@ -597,6 +602,33 @@
> size_out = size;
> }
>
> + if (ost->audio_reformat) {
> + int ibytes = av_get_bits_per_sample_format(dec->sample_fmt)/8;
> + int obytes = av_get_bits_per_sample_format(enc->sample_fmt)/8;
> + int len = size_out/(dec->channels*ibytes);
> + void *ibuf[6], *obuf[6];
> + int istride[6], ostride[6],n;
> + for(n=0; n<6; n++) { //FIXME: channels>6?
yes 6 is not enough
> + if (n<dec->channels) {
> + istride[n]= ostride[n]= dec->channels;
> + ibuf[n]= buftmp + n*ibytes;
> + obuf[n]= audio_out2 + n*obytes;
> + }else{
> + obuf[n]= NULL;
> + }
also as input & output channel layout is the same, a simple
"1 channel" like convertion should be enough.
> + }
> + if (av_audio_convert(NULL, obuf, ostride, enc->sample_fmt,
> + ibuf, istride, dec->sample_fmt, len)<0) {
this definitly needs a context, otherwise we might end in a situation
similar to the current swscale vs. img convert. That is a future context
based converter would require its context to be created and destroyed
on each such call to emulate the API ...
[...]
> @@ -639,6 +671,7 @@
> case CODEC_ID_PCM_S32BE:
> case CODEC_ID_PCM_U32LE:
> case CODEC_ID_PCM_U32BE:
> + case CODEC_ID_PCM_F32BE:
> size_out = size_out << 1;
> break;
> case CODEC_ID_PCM_S24LE:
this one can be commited anytime
[...]
> @@ -2521,6 +2555,26 @@
> return 0;
> }
>
> +static void list_sample_fmts(void)
> +{
> + int i;
> + char sample_fmt_str[128];
> + for (i=-1; i < SAMPLE_FMT_NB; i++) {
> + avcodec_sample_fmt_string (sample_fmt_str, sizeof(sample_fmt_str), i);
> + fprintf(stdout, "%s\n", sample_fmt_str);
> + }
> +}
> +
> +static void opt_audio_sample_fmt(const char *arg)
> +{
> + if (strcmp(arg, "list"))
> + audio_sample_fmt = avcodec_get_sample_fmt(arg);
> + else {
> + list_sample_fmts();
> + av_exit(0);
> + }
> +}
please add a note that this should be somehow factorized with the pixfmt
case (or if you have an idea on how to cleanly avoid the similar code that
of course is evem better)
[...]
> @@ -3108,6 +3166,17 @@
> }
> audio_enc->thread_count = thread_count;
> audio_enc->channels = audio_channels;
> + audio_enc->sample_fmt = audio_sample_fmt;
> +
> + if(codec && codec->sample_fmts){
> + const enum PixelFormat *p= codec->sample_fmts;
PixelFormat ?
> + for(; *p!=-1; p++){
> + if(*p == audio_enc->sample_fmt)
> + break;
> + }
> + if(*p == -1)
> + audio_enc->sample_fmt = codec->sample_fmts[0];
> + }
> }
> audio_enc->sample_rate = audio_sample_rate;
> audio_enc->time_base= (AVRational){1, audio_sample_rate};
> @@ -3780,6 +3849,7 @@
> { "alang", HAS_ARG | OPT_STRING | OPT_AUDIO, {(void *)&audio_language}, "set the ISO 639 language code (3 letters) of the current audio stream" , "code" },
>
> /* subtitle options */
> + { "sample_fmt", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)opt_audio_sample_fmt}, "set sample format, 'list' as argument shows all the sample formats supported", "format" },
subtitle option? :)
[...]
> @@ -1148,6 +1152,7 @@
> case CODEC_ID_PCM_S32BE:
> case CODEC_ID_PCM_U32LE:
> case CODEC_ID_PCM_U32BE:
> + case CODEC_ID_PCM_F32BE:
> bitrate = enc->sample_rate * enc->channels * 32;
> break;
> case CODEC_ID_PCM_S24LE:
> @@ -1294,6 +1299,7 @@
> case CODEC_ID_PCM_S32LE:
> case CODEC_ID_PCM_U32BE:
> case CODEC_ID_PCM_U32LE:
> + case CODEC_ID_PCM_F32BE:
> return 32;
> default:
> return 0;
these also can be commited ...
[...]
> int av_audio_convert(void *maybe_dspcontext_or_something_av_convert_specific,
> void *out[6], int out_stride[6], enum SampleFormat out_fmt,
> void * in[6], int in_stride[6], enum SampleFormat in_fmt, int len){
> int ch;
> +//FIXME: size calculation will break when SAMPLE_FMT_DBL is added...
double, hmm
i wouldnt waste time with that ...
[...]
> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h (revision 14333)
> +++ libavcodec/avcodec.h (working copy)
> @@ -210,6 +210,7 @@
> CODEC_ID_PCM_ZORK,
> CODEC_ID_PCM_S16LE_PLANAR,
> CODEC_ID_PCM_DVD,
> + CODEC_ID_PCM_F32BE,
>
> /* various ADPCM codecs */
> CODEC_ID_ADPCM_IMA_QT= 0x11000,
> @@ -346,6 +347,7 @@
> SAMPLE_FMT_S24, ///< signed 24 bits
> SAMPLE_FMT_S32, ///< signed 32 bits
> SAMPLE_FMT_FLT, ///< float
> + SAMPLE_FMT_NB ///< Number of sample formats. DO NOT USE if dynamically linking to libavcodec
> };
>
> /* in bytes */
> @@ -2262,6 +2264,7 @@
> */
> const char *long_name;
> const int *supported_samplerates; ///< array of supported audio samplerates, or NULL if unknown, array is terminated by 0
> + const enum SampleFormat *sample_fmts; ///< array of supported sample formats, or NULL if unknown, array is terminated by -1
> } AVCodec;
above looks also ok and can be commited
[...]
> @@ -2491,6 +2522,8 @@
> */
> void avcodec_pix_fmt_string (char *buf, int buf_size, int pix_fmt);
>
> +
> +
> #define FF_ALPHA_TRANSP 0x0001 /* image has some totally transparent pixels */
> #define FF_ALPHA_SEMI_TRANSP 0x0002 /* image has some transparent pixels */
ehh ...
[...]
> ===================================================================
> --- libavformat/au.c (revision 14333)
> +++ libavformat/au.c (working copy)
> @@ -39,6 +39,7 @@
> { CODEC_ID_PCM_MULAW, 1 },
> { CODEC_ID_PCM_S8, 2 },
> { CODEC_ID_PCM_S16BE, 3 },
> + { CODEC_ID_PCM_F32BE, 6 },
> { CODEC_ID_PCM_ALAW, 27 },
> { 0, 0 },
> };
this can also be commited already
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.
-------------- 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/20080725/ed5fbf87/attachment.pgp>
More information about the ffmpeg-devel
mailing list