Re: [libav-devel] [PATCH 06/11] rtpdec_h264: Clean up comments

2012-05-04 Thread Martin Storsjö

On Fri, 4 May 2012, Alex Converse wrote:


On Fri, May 4, 2012 at 3:06 PM, Martin Storsjö  wrote:

Convert C++ comments into C comments, clean up the comment
content (remove trailing periods).


Whats wrong with c99 comments?


Hmm, I thought some of our coding style guidelines preferred C style 
comments, but after looking at other common code, there's quite a bit of 
C99/C++ style comments around too, so I guess I can keep it in that style 
here, too.



---
 libavformat/rtpdec_h264.c |   87 -
 1 file changed, 47 insertions(+), 40 deletions(-)

diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
index 505bc81..a567d3d 100644
--- a/libavformat/rtpdec_h264.c
+++ b/libavformat/rtpdec_h264.c
@@ -20,7 +20,7 @@
 */

 /**
-* @file
+ * @file
 * @brief H.264 / RTP Code (RFC3984)
 * @author Ryan Martell 
 *
@@ -29,7 +29,8 @@
 * This currently supports packetization mode:
 * Single Nal Unit Mode (0), or
 * Non-Interleaved Mode (1).  It currently does not support
- * Interleaved Mode (2). (This requires implementing STAP-B, MTAP16, MTAP24, 
FU-B packet types)
+ * Interleaved Mode (2). (This requires implementing STAP-B, MTAP16, MTAP24,
+ *                        FU-B packet types)
 */

 #include "libavutil/base64.h"
@@ -46,7 +47,7 @@
 #include "rtpdec_formats.h"

 struct PayloadContext {
-    //sdp setup parameters
+    /* sdp setup parameters */
    uint8_t profile_idc;
    uint8_t profile_iop;
    uint8_t level_idc;
@@ -68,10 +69,11 @@ static int sdp_parse_fmtp_config_h264(AVStream * stream,
        av_log(codec, AV_LOG_DEBUG, "RTP Packetization Mode: %d\n", 
atoi(value));
        h264_data->packetization_mode = atoi(value);
        /*
-           Packetization Mode:
-           0 or not present: Single NAL mode (Only nals from 1-23 are allowed)
-           1: Non-interleaved Mode: 1-23, 24 (STAP-A), 28 (FU-A) are allowed.
-           2: Interleaved Mode: 25 (STAP-B), 26 (MTAP16), 27 (MTAP24), 28 
(FU-A), and 29 (FU-B) are allowed.
+         * Packetization Mode:
+         * 0 or not present: Single NAL mode (Only nals from 1-23 are allowed)
+         * 1: Non-interleaved Mode: 1-23, 24 (STAP-A), 28 (FU-A) are allowed.
+         * 2: Interleaved Mode: 25 (STAP-B), 26 (MTAP16), 27 (MTAP24), 28 
(FU-A),
+         *                      and 29 (FU-B) are allowed.
         */


This isn't doxy, why the extra *s?


I prefer it this way, and I think this is the common style.

// Martin___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 06/11] rtpdec_h264: Clean up comments

2012-05-04 Thread Alex Converse
On Fri, May 4, 2012 at 3:06 PM, Martin Storsjö  wrote:
> Convert C++ comments into C comments, clean up the comment
> content (remove trailing periods).

Whats wrong with c99 comments?

> ---
>  libavformat/rtpdec_h264.c |   87 
> -
>  1 file changed, 47 insertions(+), 40 deletions(-)
>
> diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
> index 505bc81..a567d3d 100644
> --- a/libavformat/rtpdec_h264.c
> +++ b/libavformat/rtpdec_h264.c
> @@ -20,7 +20,7 @@
>  */
>
>  /**
> -* @file
> + * @file
>  * @brief H.264 / RTP Code (RFC3984)
>  * @author Ryan Martell 
>  *
> @@ -29,7 +29,8 @@
>  * This currently supports packetization mode:
>  * Single Nal Unit Mode (0), or
>  * Non-Interleaved Mode (1).  It currently does not support
> - * Interleaved Mode (2). (This requires implementing STAP-B, MTAP16, MTAP24, 
> FU-B packet types)
> + * Interleaved Mode (2). (This requires implementing STAP-B, MTAP16, MTAP24,
> + *                        FU-B packet types)
>  */
>
>  #include "libavutil/base64.h"
> @@ -46,7 +47,7 @@
>  #include "rtpdec_formats.h"
>
>  struct PayloadContext {
> -    //sdp setup parameters
> +    /* sdp setup parameters */
>     uint8_t profile_idc;
>     uint8_t profile_iop;
>     uint8_t level_idc;
> @@ -68,10 +69,11 @@ static int sdp_parse_fmtp_config_h264(AVStream * stream,
>         av_log(codec, AV_LOG_DEBUG, "RTP Packetization Mode: %d\n", 
> atoi(value));
>         h264_data->packetization_mode = atoi(value);
>         /*
> -           Packetization Mode:
> -           0 or not present: Single NAL mode (Only nals from 1-23 are 
> allowed)
> -           1: Non-interleaved Mode: 1-23, 24 (STAP-A), 28 (FU-A) are allowed.
> -           2: Interleaved Mode: 25 (STAP-B), 26 (MTAP16), 27 (MTAP24), 28 
> (FU-A), and 29 (FU-B) are allowed.
> +         * Packetization Mode:
> +         * 0 or not present: Single NAL mode (Only nals from 1-23 are 
> allowed)
> +         * 1: Non-interleaved Mode: 1-23, 24 (STAP-A), 28 (FU-A) are allowed.
> +         * 2: Interleaved Mode: 25 (STAP-B), 26 (MTAP16), 27 (MTAP24), 28 
> (FU-A),
> +         *                      and 29 (FU-B) are allowed.
>          */

This isn't doxy, why the extra *s?

>         if (h264_data->packetization_mode > 1)
>             av_log(codec, AV_LOG_ERROR,
> @@ -79,7 +81,7 @@ static int sdp_parse_fmtp_config_h264(AVStream * stream,
>     } else if (!strcmp(attr, "profile-level-id")) {
>         if (strlen(value) == 6) {
>             char buffer[3];
> -            // 6 characters=3 bytes, in hex.
> +            /* 6 characters=3 bytes, in hex. */
>             uint8_t profile_idc;
>             uint8_t profile_iop;
>             uint8_t level_idc;
> @@ -149,7 +151,7 @@ static int sdp_parse_fmtp_config_h264(AVStream * stream,
>     return 0;
>  }
>
> -// return 0 on packet, no more left, 1 on packet, 1 on partial packet...
> +/* return 0 on packet, no more left, 1 on packet, 1 on partial packet */
>  static int h264_handle_packet(AVFormatContext *ctx,
>                               PayloadContext *data,
>                               AVStream *st,
> @@ -173,10 +175,12 @@ static int h264_handle_packet(AVFormatContext *ctx,
>     assert(data);
>     assert(buf);
>
> +    /* Simplify the case (these are all the nal types used internally by
> +     * the h264 codec). */
>     if (type >= 1 && type <= 23)
> -        type = 1;              // simplify the case. (these are all the nal 
> types used internally by the h264 codec)
> +        type = 1;
>     switch (type) {
> -    case 0:                    // undefined, but pass them through
> +    case 0:                    /* undefined, but pass them through */
>     case 1:
>         av_new_packet(pkt, len+sizeof(start_sequence));
>         memcpy(pkt->data, start_sequence, sizeof(start_sequence));
> @@ -186,11 +190,11 @@ static int h264_handle_packet(AVFormatContext *ctx,
>  #endif
>         break;
>
> -    case 24:                   // STAP-A (one packet, multiple nals)
> -        // consume the STAP-A NAL
> +    case 24:                   /* STAP-A (one packet, multiple nals) */
> +        /* consume the STAP-A NAL */
>         buf++;
>         len--;
> -        // first we are going to figure out the total size
> +        /* first we are going to figure out the total size */
>         {
>             int pass= 0;
>             int total_length= 0;
> @@ -203,16 +207,16 @@ static int h264_handle_packet(AVFormatContext *ctx,
>                 while (src_len > 2) {
>                     uint16_t nal_size = AV_RB16(src);
>
> -                    // consume the length of the aggregate...
> +                    /* consume the length of the aggregate */
>                     src += 2;
>                     src_len -= 2;
>
>                     if (nal_size <= src_len) {
>                         if(pass==0) {
> -                            // counting...
> +                            /* counting */
>