[FFmpeg-cvslog] r19234 - trunk/libavcodec/wma.c
Vitor Sessak
vitor1001
Mon Jun 22 20:19:52 CEST 2009
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
More information about the ffmpeg-cvslog
mailing list