[FFmpeg-devel] [PATCH] reformat rtsp.c

[Ronald S. Bultje]

Hi,

On Tue, Jan 5, 2010 at 3:28 AM, Benoit Fouet <benoit.fouet at free.fr> wrote:
> On Mon, 4 Jan 2010 19:12:17 -0500 Ronald S. Bultje wrote:
>> ? ? ?} 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

Done.

>> ? ? ? ? ?skip_spaces(&p);
>> ? ? ? ? ?if (*p == '\0')
>> ? ? ? ? ? ? ?break;
>> - ? ? ? ?c = toupper((unsigned char)*p++);
>> + ? ? ? ?c = toupper((unsigned char) *p++);
>
> I'd leave the space out

Casts always have spaces.

>> + ? ?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

I remove all non-prettification changes, so I'll do this but in a
separate commit.

>> - ? ?{"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

I broke line-length, and thus became unreadable in a 80-char terminal.

>> - ? ?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 ?

Done.

>> ?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 '<'

Fixed.

>> ? ? ? ? ?} 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 ?
[..]
>> ? ? ? ? ?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)

This was already discussed, if the } adds a new line it should be
removed, else it should be added. I tried to be consistent.

Patch will follow in reply to Diego.

Ronald