[FFmpeg-cvslog] r14692 - in trunk: libavcodec/pcm.c tests/regression.sh

Michael Niedermayer michaelni
Sat Aug 30 14:39:18 CEST 2008


On Sat, Aug 30, 2008 at 11:10:47AM +1000, Peter Ross wrote:
> On Sat, Aug 30, 2008 at 02:24:43AM +0200, Michael Niedermayer wrote:
> > On Fri, Aug 29, 2008 at 04:59:16PM -0700, Baptiste Coudurier wrote:
> > > Daniel Serpell wrote:
> > > > Hi!
> > > > 
> > > > On Wed, Aug 13, 2008 at 3:43 AM,  <pross at xvid.org> wrote:
> > > >> On Tue, Aug 12, 2008 at 08:45:23PM -0400, Daniel Serpell wrote:
> > > >>> On Tue, Aug 12, 2008 at 8:40 PM, Daniel Serpell
> > > >>> <daniel.serpell at gmail.com> wrote:
> > > >>>> Hi!
> > > >>>>
> > > >>>> On Mon, Aug 11, 2008 at 5:52 AM, pross <subversion at mplayerhq.hu> wrote:
> > > >>>>> Author: pross
> > > >>>>> Date: Mon Aug 11 11:52:17 2008
> > > >>>>> New Revision: 14692
> > > >>>>>
> > > >>>>> Log:
> > > >>>>> Apply PCM ENCODE/DECODE() macros to the S/U,8/24/32,LE/BE PCM codecs.
> > > >>>>>
> > > >>>>>
> > > >>>>> Modified:
> > > >>>>>   trunk/libavcodec/pcm.c
> > > >>>>>   trunk/tests/regression.sh
> > > >>>>>
> > > >>>> This commit also broke transcoding from PCM from 8 bit to 16 bit, I
> > > >>>> uploaded a sample to
> > > >>>>   ftp://upload.mplayerhq.hu:/MPlayer/incoming/pcm-audio-11024
> > > >>>>
> > > >>>> The bug can be heard in output.avi from the command line:
> > > >>> Sorry, the correct command line is:
> > > >>>
> > > >>> ffmpeg -y -i pcm-audio-bug-r14692.avi -acodec pcm_s16le -ar 48000
> > > >>> -vcodec copy output.avi
> > > >>>
> > > >>> The bug is not present with only 8-16 bit conversion, you need to resample audio
> > > >>> also.
> > > >> The resampler only supports SAMPLE_FMT_S16, and is performed by conversion
> > > >> to the target format. Hence why transcoding U8->S16 fails.
> > > >>
> > > >> I guess the next step is to make resample.c handle different foramts.
> > > >>
> > > > 
> > > > I think is better to resample *after* conversion to S16.
> > > > 
> > > > This set of patches fixes my issue, first one exits ffmpeg if the resample is
> > > > called on any sample format different of S16.
> > > > 
> > > > The second patch resamples after sample format conversion, allowing to resample
> > > > from U8 to S16.
> > > > 
> > > > Please, consider applying.
> > > > 
> > > >    Daniel.
> > > > 
> > > > 
> > > > ------------------------------------------------------------------------
> > > > 
> > > > Index: ffmpeg.c
> > > > ===================================================================
> > > > --- ffmpeg.c	(revision 14745)
> > > > +++ ffmpeg.c	(working copy)
> > > > @@ -534,6 +534,11 @@
> > > >          ost->audio_resample = 1;
> > > >  
> > > >      if (ost->audio_resample && !ost->resample) {
> > > > +        if (dec->sample_fmt != SAMPLE_FMT_S16)
> > > > +        {
> > > > +            fprintf(stderr, "Resampler only works with 16 bits per sample\n");
> > > > +            av_exit(1);
> > > > +        }
> > > >          ost->resample = audio_resample_init(enc->channels,    dec->channels,
> > > >                                              enc->sample_rate, dec->sample_rate);
> > > >          if (!ost->resample) {
> > > > 
> > > > 
> > > > ------------------------------------------------------------------------
> > > > 
> > > > --- ffmpeg.ab.c	2008-08-13 21:36:23.000000000 -0400
> > > > +++ ffmpeg.c	2008-08-13 21:36:47.000000000 -0400
> > > > @@ -534,7 +534,7 @@
> > > >          ost->audio_resample = 1;
> > > >  
> > > >      if (ost->audio_resample && !ost->resample) {
> > > > -        if (dec->sample_fmt != SAMPLE_FMT_S16)
> > > > +        if (enc->sample_fmt != SAMPLE_FMT_S16)
> > > >          {
> > > >              fprintf(stderr, "Resampler only works with 16 bits per sample\n");
> > > >              av_exit(1);
> > > > @@ -616,23 +616,12 @@
> > > >          ost->sync_opts= lrintf(get_sync_ipts(ost) * enc->sample_rate)
> > > >                          - av_fifo_size(&ost->fifo)/(ost->st->codec->channels * 2); //FIXME wrong
> > > >  
> > > > -    if (ost->audio_resample) {
> > > > -        buftmp = audio_buf;
> > > > -        size_out = audio_resample(ost->resample,
> > > > -                                  (short *)buftmp, (short *)buf,
> > > > -                                  size / (ist->st->codec->channels * 2));
> > > > -        size_out = size_out * enc->channels * 2;
> > > > -    } else {
> > > > -        buftmp = buf;
> > > > -        size_out = size;
> > > > -    }
> > > > -
> > > >      if (dec->sample_fmt!=enc->sample_fmt) {
> > > > -        const void *ibuf[6]= {buftmp};
> > > > +        const void *ibuf[6]= {buf};
> > > >          void *obuf[6]= {audio_out2};
> > > >          int istride[6]= {av_get_bits_per_sample_format(dec->sample_fmt)/8};
> > > >          int ostride[6]= {av_get_bits_per_sample_format(enc->sample_fmt)/8};
> > > > -        int len= size_out/istride[0];
> > > > +        int len= size/istride[0];
> > > >          if (av_audio_convert(ost->reformat_ctx, obuf, ostride, ibuf, istride, len)<0) {
> > > >              printf("av_audio_convert() failed\n");
> > > >              return;
> > > > @@ -641,6 +630,17 @@
> > > >          /* FIXME: existing code assume that size_out equals framesize*channels*2
> > > >                    remove this legacy cruft */
> > > >          size_out = len*2;
> > > > +    } else {
> > > > +        buftmp = buf;
> > > > +        size_out = size;
> > > > +    }
> > > > +
> > > > +    if (ost->audio_resample) {
> > > > +        size_out = audio_resample(ost->resample,
> > > > +                                  (short *)audio_buf, (short *)buftmp,
> > > > +                                  size / (ist->st->codec->channels * 2));
> > > > +        size_out = size_out * enc->channels * 2;
> > > > +        buftmp = audio_buf;
> > > >      }
> > > >  
> > > >      /* now encode as many frames as possible */
> > > > 
> > > 
> > > Ping ? This is related to roundup issue #582.
> > 
> > Isnt this just moving the problem around?
> > I mean non 16bit decoder output vs. non 16bit encoder input being a
> > problem
> > I really think the resampler should be fixed to support all the sample
> > formats similar to swscale that also can scale from anything to anything.
> 
> Agree. For the interim, can the warning msg be commited?

ok


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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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-cvslog/attachments/20080830/6f82b880/attachment.pgp>



More information about the ffmpeg-cvslog mailing list