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

[Michael Niedermayer]

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.

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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- 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/20081117/d77e62b0/attachment.pgp>