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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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/20090402/20d391e5/attachment.pgp>