[FFmpeg-devel] [PATCH] change default sample_fmt to SAMPLE_FMT_NONE

[Michael Niedermayer]

On Sat, Nov 07, 2009 at 10:37:56PM -0500, Justin Ruggles wrote:
> Michael Niedermayer wrote:
> 
> > On Sat, Nov 07, 2009 at 08:23:22PM -0500, Justin Ruggles wrote:
> >> Patch attached to change the default sample_fmt from SAMPLE_FMT_S16 to
> >> SAMPLE_FMT_NONE.
> >>
> >> With the patch, av_find_stream_info() will work how it is supposed to
> >> with regard to sample_fmt.  has_codec_parameters() requires sample_fmt
> >> to be set.  Currently the requirement is pointless because it is already
> >> set by default, and that default value may or may not be correct.
> >>
> >> example console output from ffmpeg with 24-bit FLAC file:
> >>
> >> current SVN: Stream #0.0: Audio: flac, 192000 Hz, 2 channels, s16
> >> with patch:  Stream #0.0: Audio: flac, 192000 Hz, 2 channels, s32
> >>
> >> One side effect of this patch is that av_find_stream_info() will fail
> >> when the probe size is larger than the file size and the decoder for the
> >> stream type is disabled or non-existant.
> >>
> >> example when FLAC decoder is disabled:
> >>
> >> file size=2901817, -probesize 2000000:
> >> current SVN: Stream #0.0: Audio: 0x0000, 192000 Hz, 2 channels, s16
> >> with patch:  Stream #0.0: Audio: 0x0000, 192000 Hz, 2 channels
> >>
> >> file size=2901817, -probesize 3000000:
> >> current SVN: Stream #0.0: Audio: 0x0000, 192000 Hz, 2 channels, s16
> >> with patch:  Could not find codec parameters (Audio: 0x0000, 192000 Hz,
> >> 2 channels)
> >>
> >>
> >> If it is truly desired behavior for ffmpeg to fail when it cannot find
> >> all codec parameters, one of those being sample_fmt, then this is
> >> correct behavior.  Otherwise, we need to reevaluate whether we want
> >> sample_fmt to be one of those required parameters.
> > 
> > only require sample_fmt when a decoder is available
> 
> good idea.  new patch attached.
> 
> -Justin

>  libavcodec/options.c |    2 +-
>  libavformat/utils.c  |    4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> eee81d99b65d18735836e539caa049438b9f969b  default_sample_fmt_2.patch
> Index: libavcodec/options.c
> ===================================================================
> --- libavcodec/options.c	(revision 20474)
> +++ libavcodec/options.c	(working copy)
> @@ -436,7 +436,7 @@
>      s->execute2= avcodec_default_execute2;
>      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;
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c	(revision 20474)
> +++ libavformat/utils.c	(working copy)
> @@ -1882,7 +1882,9 @@
>      int val;
>      switch(enc->codec_type) {
>      case CODEC_TYPE_AUDIO:
> -        val = enc->sample_rate && enc->channels && enc->sample_fmt != SAMPLE_FMT_NONE;
> +        val = enc->sample_rate && enc->channels &&
> +              (!avcodec_find_decoder(enc->codec_id) ||
> +              enc->sample_fmt != SAMPLE_FMT_NONE);

this is not ideal because avcodec_find_decoder isnt fast

maybe try_decode_frame() could set something when its avcodec_find_decoder()
failed and that could then prevent further try_decode_frame() tries as well
as the sample_fmt requirement


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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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/20091109/8877b3db/attachment.pgp>