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

[Peter Ross]

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?

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- 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/c2f9012f/attachment.pgp>