[FFmpeg-devel] [patch] 6 channelrawaudioinputresultsininvalid PCM packet error

[Baptiste Coudurier]

On Thu, Apr 02, 2009 at 02:32:27PM +0200, Michael Niedermayer wrote:
> On Wed, Apr 01, 2009 at 10:05:04PM -0700, Baptiste Coudurier wrote:
> > Michael Niedermayer wrote:
> > > On Sun, Nov 16, 2008 at 01:48:21PM -0800, Baptiste Coudurier wrote:
> > >> Hi,
> > >>
> > >> Michael Niedermayer wrote:
> > >>> On Fri, Nov 14, 2008 at 02:28:50PM -0800, Baptiste Coudurier wrote:
> > >>>> Michael Niedermayer wrote:
> > >>>>> On Fri, Nov 14, 2008 at 10:52:37AM -0800, Baptiste Coudurier wrote:
> > >>>>>> Michael Niedermayer wrote:
> > >>>>>>> On Wed, Nov 12, 2008 at 04:30:12PM -0800, Phil Rutschman wrote:
> > >>>>>>>>> your original logic generates packets that are not a multiple of 512
> > >>>>>>>> or
> > >>>>>>>>> 1024 thus not a multiple of disk sectors, this seems not ideal
> > >>>>>>>> It generates these packets only in cases which currently fail entirely.
> > >>>>>>>> While this may not be ideal (at least, if there is neither OS nor C
> > >>>>>>>> library caching going on), it has the virtue of solving the problem at
> > >>>>>>>> hand without changing current behavior.
> > >>>>>>>>
> > >>>>>>>>> in principle updating the regression test checksums would be fine, but
> > >>>>>>>>> why does g726 change? this looks like there is something wrong in g726
> > >>>>>>>>> which has to be found and fixed ...
> > >>>>>>>> It is worth observing that the size of the mov and mkv files change, not
> > >>>>>>>> just their checksum. When I extract the raw audio data from
> > >>>>>>>> tests/data/a-pcm_s16be.mov and strip the header, the file matches the
> > >>>>>>>> original asynth1.sw file except that it is not quite as long. Reducing
> > >>>>>>>> RAW_PACKET_SIZE increases the size of the resulting mov file, and
> > >>>>>>>> conversely increasing it results in a smaller mov.
> > >>>>>>>>
> > >>>>>>>> Something odd overall is clearly happening related to RAW_PACKET_SIZE,
> > >>>>>>>> which may or may not be related to the g726 issue, but I don't currently
> > >>>>>>>> have the time to investigate the specifics of either.
> > >>>>>>> so the summary is we need some volunteer to fix
> > >>>>>>> 1. g726
> > >>>>>>> 2. mov/mkv loosing some samples depending on RAW_PACKET_SIZE
> > >>>>>>>
> > >>>>>> Huh ? If there is a problem with mov I'll fix it right now.
> > >>>>> I dont know if there is, i just added mov because phil said:
> > >>>>> "It is worth observing that the size of the mov and mkv files change, not
> > >>>>>  just their checksum. When I extract the raw audio data from
> > >>>>>  tests/data/a-pcm_s16be.mov and strip the header, the file matches the
> > >>>>>  original asynth1.sw file except that it is not quite as long."
> > >>>>>                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >>>>>
> > >>>>> ive not confirmed this or investigated at all ...
> > >>>>>
> > >>>> What about this patch ? Regression tests pass.
> > >>> [...]
> > > [...]
> > >>>> +            st->codec->block_align = st->codec->bits_per_coded_sample*st->codec->channels/8;
> > >>>>              av_set_pts_info(st, 64, 1, st->codec->sample_rate);
> > >>>>              break;
> > >>>>          case CODEC_TYPE_VIDEO:
> > >>>> @@ -139,9 +146,9 @@
> > >>>>  static int raw_read_packet(AVFormatContext *s, AVPacket *pkt)
> > >>>>  {
> > >>>>      int ret, size, bps;
> > >>>> -    //    AVStream *st = s->streams[0];
> > >>>> +    AVStream *st = s->streams[0];
> > >>>>  
> > >>>> -    size= RAW_PACKET_SIZE;
> > >>>> +    size= (RAW_PACKET_SIZE/st->codec->block_align)*st->codec->block_align;
> > >>> i think i prefer if the packet size contains a constant number of samples
> > >>> across channels.
> > >> I don't see what you mean here.
> > > 
> > > i mean that i would prefer if packets with 6 channels would be 3 times as large
> > > as 2 channel packets. That way the duration (ignoring samplerate) would be
> > > constant. Also if 1 channel 1 byte packets are a multiple of 512 bytes then
> > > all others also would have "nice" sizes.
> > > 
> > > Besides i feel that picking one choice instead of the other because the other
> > > triggers a bunch of unrelated bugs is not ideal. And this really is the
> > > main reason for my preferance. On its own i am fine with your patch but i
> > > feel that we are making this decission in front of the background that the
> > > other cant be taken without some bugfixing, thus our decission might be
> > > sub-optimal.
> > > IMHO the bugs uncovered here should be investigated and fixed then we can
> > > look again which of the 2 variants is better.
> > > 
> > 
> > What about this ?
> 
> probably ok
> 

Applied and updated regression refs.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA    
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org