----- Original Message ----- > From: Diego Biurrun <[email protected]> > To: libav development <[email protected]> > Cc: > Sent: Friday, March 2, 2012 6:03 AM > Subject: Re: [libav-devel] [PATCH] Windows Media Audio Lossless decoder [...] >> +static int decode_tilehdr(WmallDecodeCtx *s) >> +{ >> + /* Should never consume more than 3073 bits (256 iterations for the >> + * while loop when the minimum amount of 128 samples is subtracted >> + * from missing samples in the 8 channel case). >> + * 1 + BLOCK_MAX_SIZE * MAX_CHANNELS / BLOCK_MIN_SIZE * (MAX_CHANNELS > + 4) >> + */ > > Is the mystery of this comment resolved now? I have already told you - ask Andreas. Since he didn't reply, I assume he doesn't read libav-devel ML. I don't care enough about that comment to send him a private mail.
[...] >> +/** >> + * @brief Decode one WMA frame. >> + * @param s codec context >> + * @return 0 if the trailer bit indicates that this is the last frame, >> + * 1 if there are additional frames >> + */ >> +static int decode_frame(WmallDecodeCtx *s) > > I remember Justin making me remove such Doxygen comments for > "standard" > functions in the review for the escape130 decoder. And indeed, it > should not be: This doxy has made it through 3 rounds of reviewing and you want to tell me that you noticed it just now? I don't see how decode_frame is a "standard" function. For WMA Lossless this function might as well not implement any "standard" interface. It is used only internally. For this particular decoder, that standard interface is implemented by decode_packet, not decode_frame. > > 00:45 <@DonDiego> ruggles: i remember you considering doxygen comments for > "standard" > functions in decoders unnecessary, like decode_frame() etc > 00:45 <@DonDiego> so which ones should be documented, which should not be? > 00:52 <@peloverde> anything that implements a standard interface should > not be documented > 00:52 <@peloverde> the interface itself should be documented > 00:53 <@DonDiego> what is "standard interface" in this case? the > functions that all > decoders share, i.e. avcodec callbacks? > 00:54 <@peloverde> If it is being called from a function pointer where > there are other > functions that are assigned to the pointer > 00:55 <@peloverde> basically ask yourself is this a vtable? I don't see any comment from Justin in there. All in all, none of these are big issue. I'd wait to see if anyone comes up with any issue that involves actual code. Otherwise, consider this to be my final submission. > > Diego > _______________________________________________ > libav-devel mailing list > [email protected] > https://lists.libav.org/mailman/listinfo/libav-devel > _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
