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

[Baptiste Coudurier]

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 ?

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: raw_block_align2.patch
Type: text/x-diff
Size: 1082 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090401/6e3a7134/attachment.patch>