[FFmpeg-devel] [PATCH] AMR-WB Decoder

[Diego Biurrun]

On Mon, Sep 20, 2010 at 11:21:53AM +0200, Vitor Sessak wrote:
> On 09/20/2010 09:09 AM, Diego Biurrun wrote:
>> On Sun, Sep 19, 2010 at 11:25:17PM +0200, Vitor Sessak wrote:
>>> On 09/19/2010 10:54 PM, Diego Biurrun wrote:
>>>> On Fri, Sep 17, 2010 at 05:15:26PM -0300, Marcelo Galv?o P?voa wrote:
>>>>>
>>>>> The code has already been reviewed mostly by Vitor Sessak, discussed
>>>>> and tested at the ffmpeg-soc list.
>>>>
>>>> Then why does this patch not even follow everything listed in the
>>>> New codecs or formats checklist:
>>>>
>>>> http://www.ffmpeg.org/developer.html#SEC7
>>>>
>>>> Vitor?
>>>
>>> I saw a couple of things that didn't followed the guidelines, but I
>>> focused on other more important things (code duplication, mostly) and
>>> forgot to point out the nits later.
>>
>> BTW, sorry if I sound like I'm picking on you in particular, this has
>> been an issue with our SoC processes basically since day one.  Anyway..
>>
>> IMNSHO patches should be cooked into shape on ffmpeg-soc before they
>> are proposed here.
>
> I think this patch is in a very good shape (much better than the average  
> patch posted to -devel), and that's the reason I *explicitly* asked  
> Marcelo to post it here. I don't think a couple of nits proves me wrong.

I'm not talking about the nits.  Here is the list:

   1.  Did you use av_cold for codec initialization and close functions?
   2. Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or AVInputFormat/AVOutputFormat struct?
   3. Did you bump the minor version number in `avcodec.h' or `avformat.h'?
   4. Did you register it in `allcodecs.c' or `allformats.c'?
   5. Did you add the CodecID to `avcodec.h'?
   6. If it has a fourcc, did you add it to `libavformat/riff.c', even if it is only a decoder?
   7. Did you add a rule to compile the appropriate files in the Makefile? Remember to do this even if you're just adding a format to a file that is already being compiled by some other rule, like a raw demuxer.
   8. Did you add an entry to the table of supported formats or codecs in `doc/general.texi'?
   9. Did you add an entry in the Changelog?
  10. If it depends on a parser or a library, did you add that dependency in configure?
  11. Did you "svn add" the appropriate files before commiting?

The patch fails 8. and 9. and I suspect 10. as well.

Diego