[FFmpeg-cvslog] r19234 - trunk/libavcodec/wma.c

[Vitor Sessak]

Sascha Sommer wrote:
> Hi,
> 
> On Samstag, 20. Juni 2009, Ivan Kalvachev wrote:
>> On 6/20/09, Sascha Sommer <saschasommer at freenet.de> wrote:
>>> Hi,
>>>
>>> On Samstag, 20. Juni 2009, Ivan Kalvachev wrote:
>>>> On 6/20/09, faust3 <subversion at mplayerhq.hu> wrote:
>>>>> Author: faust3
>>>>> Date: Sat Jun 20 13:06:48 2009
>>>>> New Revision: 19234
>>>>>
>>>>> Log:
>>>>> Simplify run level decoding:
>>>>> - remove unneeded vlc code < 0 check
>>>> Hum, would you explain this a little bit more.
>>>> Afaik -1 is returned when sequence that doesn't belong
>>>> to any known vlc is met.
>>>> This usually happens on broken streams.
>>>>
>>>> Can you prove that wma vlc don't have invalid sequences?
>>> In libavcodec/bitstream.c build_table() the table for get_vlc2 is built
>>> with unused codes set to -1
>>>
>>>     for(i=0;i<table_size;i++) {
>>>         table[i][1] = 0; //bits
>>>         table[i][0] = -1; //codes
>>>     }
>>>
>>> After the tables are filled none of the table entries are -1 anymore.
>> OK, if you have checked all tables.
>> I still would like at least a comment in the code explaining this.
>>
> 
> I'm not the maintainer of wma.c but I wouldn't mind if you add one.
> 
>> BTW, why all code comments are in doxygen style? /** */?
> 
> I think comments in new patches have to be doxygen compatible.

I think that comments in new patches that can be doxyfied should be 
doxygen compatible, but not all of them (our development policy is 
ambiguous about that). For example, for the following code:

> /** some function that does something */
> static void func()
> {
>     /** I don't know why this constant should be 27 */
>     int i = 27;
> 
>     /** neither why this one should be 33 */
>     some_function(33);
> 
>     {some random thing}
> }

Doxygen will generate the attached documentation. Note that the all the 
doxy comments about the constants inside the function makes the 
generated html completely obfuscated. IMHO, this function should be 
written as

> /** some function that does something */
> static void func()
> {
>     /* I don't know why this constant should be 27 */
>     int i = 27;
> 
>     /* neither why this one should be 33 */
>     some_function(33);
> 
>     {some random thing}
> }

which will not clutter the doxy documentation which comments that do not 
make sense without reading the code.

-Vitor