[Ffmpeg-devel] [PATCH] MS-GSM support: draft for review

[Michel Bardiaux]

Michael Niedermayer wrote:
> Hi
> 
> On Tue, Nov 07, 2006 at 11:33:53AM +0100, Michel Bardiaux wrote:
> [...]
>>>>> ) are packed together into one
>>>>> 65 byte packet instead of each padded to a multiple of 8 (=33 byte)
>>>>> this is nasty, MS programmers should be shot for this
>>>> The alternative, 4 bits of padding per frame, isnt clean either.
>>> every codec pads to bytes with very few exceptions, 
>> Which exceptions?
> 
> i thought h.261 allowed it but iam not sure
> 
> 
>>> also there are rules
>>> on what a AVPacket is, you cant just put 2 packets in it, thats violating
>>> the API, an AVPacket by definition is 1 packet
>>>
>>>
>>>>> the solution is IMHO a AVParser which splits these packet pairs before 
>>>>> the
>>>>> gsm decoder
>>>> This is not the way I see collaboration to an open-source project. When 
>>>> someone has something that works (in a domain where things didnt work or 
>>>> were not implemented before), you may reject code that is *grossly* 
>>>> wrong or bloated or slow; not because you find the style not one-liney 
>>>> enough, or complicated enough, or obfuscated enough.
>>> a patch which violates several rules of our policy is rejected no matter
>>> what great things it implements
>>>
>>>
>>>> Then *you* are free to submit something better.
>>>>
>>>> With 30 years of experience under my belt, I would not tolerate that 
>>>> attitude from my head of department, and I dont think I have to accept 
>>>> it from you.
>>> you dont have to, thats the beauty of free software, just fork
>> That's not realistic and you know it.
> 
> why not?

Because I have neither the time nor the manpower even to maintain a set 
of unofficial patches over successive versions.

> 
> 
> [...]
>>> second 
>>>  it breaks the rule of 1 packet per AVPacket, all codecs be it mpeg1
>>>  mpeg2, mpeg4, mp3, h264, h263, ac3, ... get split to packets msgsm if it
>>>  is supported will be too (and yes there is some old code which doesnt
>>>  split things into packets but that is wrong and should be fixed)
>>> third
>>>  your implementation does not allow -acodec copy between msgsm<->gsm
>> That's circular reasoning: if the codecs are deemed different enough 
>> then its normal that copy does not work.
> 
> copy should always work if possible (its lossless vs lossy issue)
> 
> 
>> Now, I've completed my dev on basis of my initial patch, including the 
>> changes to the WAV header, producing a WAV file that works in WinAmp. 
>> Now that I have that as reference implementation, I'll have a go at the 
>> parser-based approach, but I'll need some help. ITIYM things like pnm_parse?
> 
> yes, though, pnm_parse() is a little more complex then what should be needed
> i mean pnm_parse() parses some header to determine frame size, while here
> the the size is constant

It looked like the simplest parser I could find. Or did I miss a better 
example?

> pnm_parse() and many other parsers due to the need of header parsing also
> need to deal with overread (the problem that they need a few bytes of the
> start of one frames before they know that it is the start of one) that
> is also unneeded for gsm due to the constant size
> 

Is there somewhere an explanation of the parser API?


-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/