[FFmpeg-devel] [PATCH] reformat rtsp.c
Benoit Fouet
benoit.fouet
Tue Jan 5 09:28:20 CET 2010
Hi,
On Mon, 4 Jan 2010 19:12:17 -0500 Ronald S. Bultje wrote:
> Hi,
>
> $subj, requested by Diego.
>
> How do I format this? The following looks logical, but is ugly:
>
> switch (x) {
> case x: {
> int bla;
> }
> }
>
> Ronald
>
> Index: libavformat/rtsp.c
> ===================================================================
> --- libavformat/rtsp.c (revision 21020)
> +++ libavformat/rtsp.c (working copy)
> @@ -101,23 +103,24 @@
>
> [...]
>
> } else {
> - /* We are in a standard case ( from http://www.iana.org/assignments/rtp-parameters) */
> + /* We are in a standard case
> + * ( from http://www.iana.org/assignments/rtp-parameters). */
I'd remove the space between the parenthesis and from
> @@ -160,18 +167,18 @@
> return -1;
> }
>
> -/* return the length and optionnaly the data */
> +/** return the length and optionally the data */
> static int hex_to_data(uint8_t *data, const char *p)
> {
> int c, len, v;
>
> len = 0;
> v = 1;
> - for(;;) {
> + for (;;) {
> skip_spaces(&p);
> if (*p == '\0')
> break;
> - c = toupper((unsigned char)*p++);
> + c = toupper((unsigned char) *p++);
I'd leave the space out
> @@ -193,33 +200,33 @@
>
> [...]
>
> + case CODEC_ID_MPEG4:
> + case CODEC_ID_AAC:
> + if (!strcmp(attr, "config")) {
> + /* decode the hexa encoded parameter */
> + int len = hex_to_data(NULL, value);
> + if (codec->extradata)
> + av_free(codec->extradata);
not related to the prettyfication, but the if is useless
> @@ -227,22 +234,29 @@
> #define ATTR_NAME_TYPE_STR 1
> static const AttrNameMap attr_names[]=
> {
> - {"SizeLength", ATTR_NAME_TYPE_INT, offsetof(RTPPayloadData, sizelength)},
> - {"IndexLength", ATTR_NAME_TYPE_INT, offsetof(RTPPayloadData, indexlength)},
> - {"IndexDeltaLength", ATTR_NAME_TYPE_INT, offsetof(RTPPayloadData, indexdeltalength)},
> - {"profile-level-id", ATTR_NAME_TYPE_INT, offsetof(RTPPayloadData, profile_level_id)},
> - {"StreamType", ATTR_NAME_TYPE_INT, offsetof(RTPPayloadData, streamtype)},
> - {"mode", ATTR_NAME_TYPE_STR, offsetof(RTPPayloadData, mode)},
> - {NULL, -1, -1},
> + { "SizeLength", ATTR_NAME_TYPE_INT,
> + offsetof(RTPPayloadData, sizelength) },
> + { "IndexLength", ATTR_NAME_TYPE_INT,
> + offsetof(RTPPayloadData, indexlength) },
> + { "IndexDeltaLength", ATTR_NAME_TYPE_INT,
> + offsetof(RTPPayloadData, indexdeltalength) },
> + { "profile-level-id", ATTR_NAME_TYPE_INT,
> + offsetof(RTPPayloadData, profile_level_id) },
> + { "StreamType", ATTR_NAME_TYPE_INT,
> + offsetof(RTPPayloadData, streamtype) },
> + { "mode", ATTR_NAME_TYPE_STR,
> + offsetof(RTPPayloadData, mode) },
> + { NULL, -1, -1 },
I find the former version much more readable than the latter
> @@ -262,24 +276,26 @@
> * encoded, giving a 12KB * (4/3) = 16KB FMTP line. */
> char value[16384];
> int i;
> -
> RTSPStream *rtsp_st = st->priv_data;
> AVCodecContext *codec = st->codec;
> RTPPayloadData *rtp_payload_data = &rtsp_st->rtp_payload_data;
>
> /* loop on each attribute */
> - while(rtsp_next_attr_and_value(&p, attr, sizeof(attr), value, sizeof(value)))
> - {
> - /* grab the codec extra_data from the config parameter of the fmtp line */
> + while (rtsp_next_attr_and_value(&p, attr, sizeof(attr),
> + value, sizeof(value))) {
align with the arguments ?
> @@ -1160,7 +1182,7 @@
> return 0;
>
> fail:
> - for (i=0; i<rt->nb_rtsp_streams; i++) {
> + for (i = 0; i<rt->nb_rtsp_streams; i++) {
I don't understand why the '=' and not the '<'
> @@ -1298,9 +1321,8 @@
> continue;
> } else if (!strncasecmp(reply->server, "WMServer/", 9)) {
> rt->server_type = RTSP_SERVER_WMS;
> - } else if (rt->server_type == RTSP_SERVER_REAL) {
> + } else if (rt->server_type == RTSP_SERVER_REAL)
> strcpy(real_challenge, reply->real_challenge);
> - }
why ?
> @@ -1536,22 +1557,20 @@
> return len;
> if (len == 0)
> return AVERROR_EOF;
> - if (rt->transport == RTSP_TRANSPORT_RDT)
> + if (rt->transport == RTSP_TRANSPORT_RDT) {
> ret = ff_rdt_parse_packet(rtsp_st->transport_priv, pkt, buf, len);
> - else
> + } else
> ret = rtp_parse_packet(rtsp_st->transport_priv, pkt, buf, len);
this syntax looks weird to me (but IIRC it was already discussed)
Ben
More information about the ffmpeg-devel
mailing list