[FFmpeg-devel] [PATCH] QCELP decoder

[Kenan Gillet]

Hi,
On Nov 25, 2008, at 4:54 PM, Reynaldo H. Verdejo Pinochet wrote:

> Hello
>
> Kenan Gillet wrote:
>>
>> Currently, the only demuxer that handles QCELP is the MOV
>> demuxer.
>> All the samples we have, are in different flavors of MOV files (mov,
>> 3g2, k3g)
>> In all those files, 1 MOV frame corresponds to 1 QCELP frame:
>> there is no bundling.
>
> Kenan, The way I see its simple, use the decoder to decode. To
> parse a frame beyond what the spec mandates, account for transport
> oddities or whatnot, either correctly patch the demuxer or use
> a parser. Its not OK to put such code in the decoder, even if
> it makes everything 'just work', its not the correct solution
> not to mention the need for a parser was long ago understood and
> accounted for.
>
>>>> a parser. But right now, I think the focus is on making the QCELP
>>>> decoder works on the different flavor of MOV files, and it does not
>>>> need any parser.
>
> It does need a parser, thats not under discussion. You're putting
> parsing code in the decoder, that's the issue here.

Again , I don't think that I do. Can you point me where I do in my last
set of patches ?


>
>
>>> Having custom packet parsing code (not talking about codec frame
>>> parsing) in the decoder is not a clean solution, this should be
>>> handled with a parser. Even more if this is just to handle a subset
>>> of files. There is an skeleton parser on the SoC repo. Maybe you
>>> could add to it?
>>
>> Ok, let me rephrase it:
>> Making QCELP decoding available in every possible/existing format
>> is definitely a long term goal
>
> So was making IFQ work or handle ERASURES but we both bitted
> that bullet long ago. I'm just asking you to put the code
> where it belongs, there is a parser in the SoC repo, put
> the parsing code there.

the main difference with the IFQ/erasure is we actually have samples
that needs it...


>
>
>> But my CURRENT focus is to merge the QCELP decoder, so that it
>> can correctly decode all the samples we have and be as specs
>> compliant as possible.
>
> You are going out-spec with the parsing, and please don't get me
> wrong -- I like that, I'm just telling you to put the code where
> it belongs. At least for starters, we both know that parser will
> have to be extended latter.
>
>
>> Furthermore, I am not comfortable to add code that has not been
>> tested on real world samples; it is just my philosophy...
>
> Again --
>
> You are already adding the code. I'm not asking you to add anything
> more, Or to take care about debundling tied-up RTSP transported codec
> frames -- no. Just move YOUR parsing code to the correct place.

I give up. If Michael is OK with the parser, I'll move the code into a  
parser.

[...]

Kenan