[FFmpeg-devel] [PATCH] alsdec: channel sorting

Thilo Borgmann thilo.borgmann at googlemail.com
Fri Dec 21 21:27:22 CET 2012


Am 21.12.12 19:16, schrieb Paul B Mahol:
> On 12/21/12, Thilo Borgmann <thilo.borgmann at googlemail.com> wrote:
>> Am 21.12.12 17:52, schrieb Paul B Mahol:
>>> On 12/21/12, Paul B Mahol <onemda at gmail.com> wrote:
>>>> On 12/21/12, Thilo Borgmann <thilo.borgmann at googlemail.com> wrote:
>>>>> Am 21.12.12 15:05, schrieb Paul B Mahol:
>>>>>> On 12/21/12, Thilo Borgmann <thilo.borgmann at googlemail.com> wrote:
>>>>>>> Am 21.12.12 14:21, schrieb Paul B Mahol:
>>>>>>>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>>>>>>>> ---
>>>>>>>>  libavcodec/alsdec.c | 13 ++++++++++---
>>>>>>>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> Probably ok. Have you tested and veryfied using the reference
>>>>>>> encoder/decoder?
>>>>>>>
>>>>>>
>>>>>> Yes, without this patch ffmpeg reports CRC error.
>>>>>>
>>>>>
>>>>>> Though instead of aborting i will just ignore channel sorting if
>>>>>> invalid
>>>>>> values
>>>>>> are encountered.
>>>>>
>>
>>>>> This sounds like a "will do in another patch" ? Currently your patch
>>>>> returns
>>>>> INVALIDDATA...?
>>
>> see below...
>>
>>>>>> [...]
>>>>>> also reference decoder gives broken output for some 6ch files i found
>>>>>> on
>>>>>> their
>>>>>> ftp server.
>>>>>
>>>>> Please provide me a link to these files segfaulting the reference
>>>>> decoder
>>>>> (which
>>>>> version do you use btw?)
>>>>
>>>> It is not really file which cause segv.
>>>> How to segfault, latest 23 release:
>>>> /tmp/mp4alsRM23/bin/{arch}/mp4alsRM23 -m2,1,3,4,7,6 /tmp/out.wav
>>>> /tmp/out.mp4
>>>
>>> The point is that input wav have 6 channels.
>>
>> Ok it segfaults for wrong channel numbers then like you already told.
>>
>> I think ignoring reordering in this case is reasonable but your patch does
>> not
>> do that, does it?
> 
> I already comitted one that ignore it.

"Probably ok." combined with more comments does hardly mean to commit this patch
in my eyes. However I ignored that. That you commit something different does
make it worse - let's just say I'm not amused.

I'm still "probably ok" with the patch but I think it should be done
differently. Please see Log for what I think should be done differently...

-Thilo




More information about the ffmpeg-devel mailing list