[FFmpeg-devel] [RFC] lavc/ffmpeg sample_fmt implementation

Michael Niedermayer michaelni
Fri Aug 1 21:31:20 CEST 2008


On Sat, Aug 02, 2008 at 12:21:04AM +1000, pross at xvid.org wrote:
> On Thu, Jul 31, 2008 at 06:39:12PM +0200, Michael Niedermayer wrote:
> > On Fri, Aug 01, 2008 at 12:02:12AM +1000, pross at xvid.org wrote:
> > > On Wed, Jul 30, 2008 at 10:09:02PM +0200, Michael Niedermayer wrote:
> > > > 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.
> > > > > 
> 
> Round four. FFplay patch included as well.
> 
> > > Related matter: When codecs output/input sample formats other than
> > > SAMPLE_FMT_S16, existing apps that expect the avcodec_decode_audio2() and
> > > avcodec_encode_audio() functions to handle shorts ints will break.
> > 
> > Yes, but they will need to be fixed eventually anyway and sample_fmt is not
> > a new field, it was there since a long time. IMHO applicaions that ignore its
> > value are already broken.
> > 
> > I really do not want to spend too much time thinking about what happens with
> > a few applications with some codecs and latest lavc for the period of a week.
> > Any maintained application will be fixed within that time, users dont need
> > to use latest lavc with applications ignoreing sample_fmt for that week.
> > 
> > Also its no API or ABI breakage, its rather applications ignoring it because
> > it happened to be always NONE/S16
> > 
> > [...]
> 
> Sounds ok. Pun intended.
> 
> -- Peter
> (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

> Index: utils.c
> ===================================================================
> --- libavcodec/utils.c	(revision 14495)
> +++ libavcodec/utils.c	(working copy)
> @@ -766,7 +766,7 @@
>      s->execute= avcodec_default_execute;
>      s->sample_aspect_ratio= (AVRational){0,1};
>      s->pix_fmt= PIX_FMT_NONE;
> -    s->sample_fmt= SAMPLE_FMT_S16; // FIXME: set to NONE
> +    s->sample_fmt= SAMPLE_FMT_NONE;
>  
>      s->palctrl = NULL;
>      s->reget_buffer= avcodec_default_reget_buffer;

ok


> Index: ffmpeg.c
> ===================================================================
> --- ffmpeg.c	(revision 14495)
> +++ ffmpeg.c	(working copy)
> @@ -257,6 +257,8 @@
>      /* audio only */
>      int audio_resample;
>      ReSampleContext *resample; /* for audio resampling */

> +    int audio_reformat;
> +    AVAudioConvert *reformat_ctx;

couldnt reformat_ctx!= NULL be used instead of audio_reformat ?


[...]

> @@ -2162,6 +2195,8 @@
>                      sws_freeContext(ost->img_resample_ctx);
>                  if (ost->resample)
>                      audio_resample_close(ost->resample);
> +                if (ost->reformat_ctx)
> +                    av_audio_convert_free(ost->reformat_ctx);
>                  av_free(ost);
>              }
>          }
> @@ -2763,6 +2798,7 @@
>      ap->width = frame_width + frame_padleft + frame_padright;
>      ap->height = frame_height + frame_padtop + frame_padbottom;
>      ap->pix_fmt = frame_pix_fmt;
> +   // ap->sample_fmt = audio_sample_fmt; //FIXME:not implemented in libavformat
>      ap->channel = video_channel;
>      ap->standard = video_standard;
>      ap->video_codec_id = find_codec_or_die(video_codec_name, CODEC_TYPE_VIDEO, 0);
> @@ -2827,6 +2863,7 @@
>              //fprintf(stderr, "\nInput Audio channels: %d", enc->channels);
>              audio_channels = enc->channels;
>              audio_sample_rate = enc->sample_rate;
> +            audio_sample_fmt = enc->sample_fmt;
>              if(audio_disable)
>                  ic->streams[i]->discard= AVDISCARD_ALL;
>              break;
> @@ -3109,6 +3146,7 @@
>          st->stream_copy = 1;
>          audio_enc->channels = audio_channels;
>      } else {
> +        AVCodec *codec;
>          codec_id = av_guess_codec(oc->oformat, NULL, oc->filename, NULL, CODEC_TYPE_AUDIO);
>  
>          set_context_opts(audio_enc, avctx_opts[CODEC_TYPE_AUDIO], AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM);
> @@ -3116,6 +3154,7 @@
>          if (audio_codec_name)
>              codec_id = find_codec_or_die(audio_codec_name, CODEC_TYPE_AUDIO, 1);
>          audio_enc->codec_id = codec_id;
> +        codec = avcodec_find_encoder(codec_id);
>  
>          if (audio_qscale > QSCALE_NONE) {
>              audio_enc->flags |= CODEC_FLAG_QSCALE;
> @@ -3123,6 +3162,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 SampleFormat *p= codec->sample_fmts;
> +            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};
> @@ -3793,6 +3843,7 @@
>      { "vol", OPT_INT | HAS_ARG | OPT_AUDIO, {(void*)&audio_volume}, "change audio volume (256=normal)" , "volume" }, //
>      { "newaudio", OPT_AUDIO, {(void*)opt_new_audio_stream}, "add a new audio stream to the current output stream" },
>      { "alang", HAS_ARG | OPT_STRING | OPT_AUDIO, {(void *)&audio_language}, "set the ISO 639 language code (3 letters) of the current audio stream" , "code" },
> +    { "sample_fmt", HAS_ARG | OPT_EXPERT | OPT_AUDIO, {(void*)opt_audio_sample_fmt}, "set sample format, 'list' as argument shows all the sample formats supported", "format" },
>  
>      /* subtitle options */
>      { "sn", OPT_BOOL | OPT_SUBTITLE, {(void*)&subtitle_disable}, "disable subtitle" },

ok


> Index: ffplay.c
> ===================================================================
> --- ffplay.c	(revision 14495)
> +++ ffplay.c	(working copy)
> @@ -26,6 +26,7 @@
>  #include "libavformat/rtsp.h"
>  #include "libavdevice/avdevice.h"
>  #include "libswscale/swscale.h"
> +#include "libavcodec/audioconvert.h"
>  
>  #include "cmdutils.h"
>  

ok


[...]

>  {
>      AVPacket *pkt = &is->audio_pkt;
> +    AVCodecContext *dec= is->audio_st->codec;
>      int n, len1, data_size;
>      double pts;

this and the related changes are ok but they are a seperate simplification and
should be commited seperately from the audioconvert stuff.


[...]
> @@ -1730,6 +1770,10 @@
>              return -1;
>          }
>          is->audio_hw_buf_size = spec.size;
> +
> +         /* destination audio format; must correspond to wanted_spec format */
> +        is->audio_dst_fmt= SAMPLE_FMT_S16;

audio_dst_fmt seems to never be set to a different value thus it isnt needed


> +        is->audio_src_fmt= is->audio_dst_fmt;
>      }
>  
>      if(thread_count>1)

> @@ -1796,6 +1840,8 @@
>          SDL_CloseAudio();
>  
>          packet_queue_end(&is->audioq);
> +        if (is->reformat_ctx)
> +            av_free(is->reformat_ctx);

av_audio_convert_free

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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20080801/10a677d1/attachment.pgp>



More information about the ffmpeg-devel mailing list