On date Monday 2009-11-30 23:21:46 +0200, Sergiy encoded:

2009/11/30 Diego Biurrun <diego at biurrun.de>:

...

Please respect the surrounding (K&R) style and be consistent, you mix styles even in the code you add: Place spaces after if/for/while/switch.

...

.. and space before {

Added. I also fix code for sending channel_id >= 64 in ff_rtmp_packet_write().

diff --git a/libavformat/rtmppkt.c b/libavformat/rtmppkt.c index e1f0647..a07e996 100644 --- a/libavformat/rtmppkt.c +++ b/libavformat/rtmppkt.c @@ -111,6 +111,13 @@ int ff_rtmp_packet_read(URLContext *h, RTMPPacket *p, } } } + //extract ts extension + if (timestamp == 0xffffff) { + if (url_read_complete(h, buf, 4) != 4) + return AVERROR(EIO); + timestamp = AV_RB32(buf); + } +

This belongs to a separate patch.

if (ff_rtmp_packet_create(p, channel_id, type, timestamp, data_size)) return -1; p->extra = extra; @@ -144,17 +151,47 @@ int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt, int mode = RTMP_PS_TWELVEBYTES; int off = 0;

- //TODO: header compression

Don't remove the todo if you didn't fixed it yet.

- bytestream_put_byte(&p, pkt->channel_id | (mode << 6)); + //if channel_id = 0, this is first presentation of prev_pkt, send full hdr. + if (prev_pkt[pkt->channel_id].channel_id && + pkt->extra == prev_pkt[pkt->channel_id].extra) { + if (pkt->type == prev_pkt[pkt->channel_id].type && + pkt->data_size == prev_pkt[pkt->channel_id].data_size) { + mode = RTMP_PS_FOURBYTES; + if (pkt->timestamp == prev_pkt[pkt->channel_id].timestamp) + mode = RTMP_PS_ONEBYTE; + } else + mode = RTMP_PS_EIGHTBYTES; + } + + if (pkt->channel_id < 64) + bytestream_put_byte(&p, pkt->channel_id | (mode << 6)); + else if (pkt->channel_id - 64 < 256) { + bytestream_put_byte(&p, 0 | (mode << 6)); + bytestream_put_byte(&p, pkt->channel_id-64); + } else { + bytestream_put_byte(&p, 1 | (mode << 6)); + bytestream_put_le16(&p, pkt->channel_id-64); + }

This belongs to a separate patch.

if (mode != RTMP_PS_ONEBYTE) { - bytestream_put_be24(&p, pkt->timestamp); + bytestream_put_be24(&p, pkt->timestamp < 0xffffff ? pkt->timestamp : 0xffffff); if (mode != RTMP_PS_FOURBYTES) { bytestream_put_be24(&p, pkt->data_size); bytestream_put_byte(&p, pkt->type); if (mode == RTMP_PS_TWELVEBYTES) bytestream_put_le32(&p, pkt->extra); } + //send ts extension + if (pkt->timestamp >= 0xffffff) + bytestream_put_be32(&p, pkt->timestamp); } + // save history + prev_pkt[pkt->channel_id].channel_id = pkt->channel_id; + prev_pkt[pkt->channel_id].type = pkt->type; + prev_pkt[pkt->channel_id].data_size = pkt->data_size; + prev_pkt[pkt->channel_id].timestamp = pkt->timestamp; + prev_pkt[pkt->channel_id].extra = pkt->extra; + url_write(h, pkt_hdr, p-pkt_hdr); while (off < pkt->data_size) { int towrite = FFMIN(chunk_size, pkt->data_size - off); diff --git a/libavformat/rtmppkt.h b/libavformat/rtmppkt.h index f50996f..2c4f92b 100644 --- a/libavformat/rtmppkt.h +++ b/libavformat/rtmppkt.h @@ -34,6 +34,7 @@ enum RTMPChannel { RTMP_NETWORK_CHANNEL = 2, ///< channel for network-related messages (bandwidth report, ping, etc) RTMP_SYSTEM_CHANNEL, ///< channel for sending server control messages + RTMP_SOURCE_CHANNEL, ///< channel for sending a/v to server RTMP_VIDEO_CHANNEL = 8, ///< channel for video data RTMP_AUDIO_CHANNEL, ///< channel for audio data }; diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c index 6d52214..4640da3 100644 --- a/libavformat/rtmpproto.c +++ b/libavformat/rtmpproto.c @@ -57,12 +57,16 @@ typedef struct RTMPContext { URLContext* stream; ///< TCP stream used in interactions with RTMP server RTMPPacket prev_pkt[2][RTMP_CHANNELS]; ///< packet history used when reading and sending packets int chunk_size; ///< size of the chunks RTMP packets are divided into + double pkt_id; ///< each packet sends number, that increased by 1 + int is_input; ///< input/output flag char playpath[256]; ///< path to filename to play (with possible "mp4:" prefix)

+ char app[128]; ///< application

This belongs to a separate patch.

ClientState state; ///< current state int main_channel_id; ///< an additional channel ID which is used for some invocations uint8_t* flv_data; ///< buffer with data for demuxer int flv_size; ///< current buffer size int flv_off; ///< number of bytes read from current buffer + RTMPPacket out_pkt; ///< type of flv packet (a/v or metadata) uint32_t video_ts; ///< current video timestamp in milliseconds uint32_t audio_ts; ///< current audio timestamp in milliseconds } RTMPContext; @@ -94,42 +98,53 @@ static const uint8_t rtmp_server_key[] = { * Generates 'connect' call and sends it to the server. */ static void gen_connect(URLContext *s, RTMPContext *rt, const char *proto, - const char *host, int port, const char *app) + const char *host, int port) { RTMPPacket pkt; - uint8_t ver[32], *p; + uint8_t ver[64], *p; char tcurl[512];

- ff_rtmp_packet_create(&pkt, RTMP_VIDEO_CHANNEL, RTMP_PT_INVOKE, 0, 4096); + ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0, 4096); p = pkt.data;

- snprintf(tcurl, sizeof(tcurl), "%s://%s:%d/%s", proto, host, port, app); + snprintf(tcurl, sizeof(tcurl), "%s://%s:%d/%s", proto, host, port, rt->app); ff_amf_write_string(&p, "connect"); - ff_amf_write_number(&p, 1.0); + ff_amf_write_number(&p, rt->pkt_id); + rt->pkt_id++; ff_amf_write_object_start(&p); ff_amf_write_field_name(&p, "app"); - ff_amf_write_string(&p, app); + ff_amf_write_string(&p, rt->app); + + if (rt->is_input) { + snprintf(ver, sizeof(ver), "%s %d,%d,%d,%d", RTMP_CLIENT_PLATFORM, RTMP_CLIENT_VER1, + RTMP_CLIENT_VER2, RTMP_CLIENT_VER3, RTMP_CLIENT_VER4); + } else { + snprintf(ver, sizeof(ver), "FMLE/3.0 (compatible; %s)", LIBAVFORMAT_IDENT); + ff_amf_write_field_name(&p, "type"); + ff_amf_write_string(&p, "nonprivate"); + }

- snprintf(ver, sizeof(ver), "%s %d,%d,%d,%d", RTMP_CLIENT_PLATFORM, RTMP_CLIENT_VER1, - RTMP_CLIENT_VER2, RTMP_CLIENT_VER3, RTMP_CLIENT_VER4); ff_amf_write_field_name(&p, "flashVer"); ff_amf_write_string(&p, ver); ff_amf_write_field_name(&p, "tcUrl"); ff_amf_write_string(&p, tcurl); - ff_amf_write_field_name(&p, "fpad"); - ff_amf_write_bool(&p, 0); - ff_amf_write_field_name(&p, "capabilities"); - ff_amf_write_number(&p, 15.0); - ff_amf_write_field_name(&p, "audioCodecs"); - ff_amf_write_number(&p, 1639.0); - ff_amf_write_field_name(&p, "videoCodecs"); - ff_amf_write_number(&p, 252.0); - ff_amf_write_field_name(&p, "videoFunction"); - ff_amf_write_number(&p, 1.0); + ff_amf_write_field_name(&p, "swfUrl"); //justin.tv want this + ff_amf_write_string(&p, tcurl); + if (rt->is_input) { + ff_amf_write_field_name(&p, "fpad"); + ff_amf_write_bool(&p, 0); + ff_amf_write_field_name(&p, "capabilities"); + ff_amf_write_number(&p, 15.0); + ff_amf_write_field_name(&p, "audioCodecs"); + ff_amf_write_number(&p, 1639.0); + ff_amf_write_field_name(&p, "videoCodecs"); + ff_amf_write_number(&p, 252.0); + ff_amf_write_field_name(&p, "videoFunction"); + ff_amf_write_number(&p, 1.0); + }

Leave the reindent for a separate cosmetic patch, this way the patch is easier to read.

ff_amf_write_object_end(&p);

pkt.data_size = p - pkt.data; - ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]);

Cosmetics.

}

@@ -142,13 +157,37 @@ static void gen_create_stream(URLContext *s, RTMPContext *rt) RTMPPacket pkt; uint8_t *p;

- av_log(LOG_CONTEXT, AV_LOG_DEBUG, "Creating stream...\n"); - ff_rtmp_packet_create(&pkt, RTMP_VIDEO_CHANNEL, RTMP_PT_INVOKE, 0, 25); + ff_rtmp_packet_create(&pkt, RTMP_SYSTEM_CHANNEL, RTMP_PT_INVOKE, 0, 4096); + + if (!rt->is_input) { + av_log(LOG_CONTEXT, AV_LOG_DEBUG, "Releasing stream...\n"); + p = pkt.data; + ff_amf_write_string(&p, "releaseStream"); + ff_amf_write_number(&p, rt->pkt_id); + rt->pkt_id++; + ff_amf_write_null(&p); + ff_amf_write_string(&p, rt->playpath); + pkt.data_size = p - pkt.data; + ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]); + + av_log(LOG_CONTEXT, AV_LOG_DEBUG, "Publishing stream...\n");

Add some trace for debugging is fine but again it belongs to a separate patch.

+ p = pkt.data; + ff_amf_write_string(&p, "FCPublish"); + ff_amf_write_number(&p, rt->pkt_id); + rt->pkt_id++; + ff_amf_write_null(&p); + ff_amf_write_string(&p, rt->playpath); + pkt.data_size = p - pkt.data; + ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]); + }

+ av_log(LOG_CONTEXT, AV_LOG_DEBUG, "Creating stream...\n"); p = pkt.data; ff_amf_write_string(&p, "createStream"); - ff_amf_write_number(&p, 3.0); + ff_amf_write_number(&p, rt->pkt_id); + rt->pkt_id++; ff_amf_write_null(&p); + pkt.data_size = p - pkt.data;

ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]); ff_rtmp_packet_destroy(&pkt); @@ -163,30 +202,40 @@ static void gen_play(URLContext *s, RTMPContext *rt) RTMPPacket pkt; uint8_t *p;

- av_log(LOG_CONTEXT, AV_LOG_DEBUG, "Sending play command for '%s'\n", rt->playpath); - ff_rtmp_packet_create(&pkt, RTMP_VIDEO_CHANNEL, RTMP_PT_INVOKE, 0, - 20 + strlen(rt->playpath)); + av_log(LOG_CONTEXT, AV_LOG_DEBUG, "Sending %s command for '%s'\n", rt->is_input ? "play" : "publish", + rt->playpath); + ff_rtmp_packet_create(&pkt, rt->is_input ? RTMP_VIDEO_CHANNEL : RTMP_SOURCE_CHANNEL, RTMP_PT_INVOKE, 0, + 4096); pkt.extra = rt->main_channel_id;

p = pkt.data; - ff_amf_write_string(&p, "play"); + if (rt->is_input) + ff_amf_write_string(&p, "play");

Again cosmetics.

+ else + ff_amf_write_string(&p, "publish"); + ff_amf_write_number(&p, 0.0); ff_amf_write_null(&p); ff_amf_write_string(&p, rt->playpath); + if (!rt->is_input) + ff_amf_write_string(&p, "live"); + pkt.data_size = p - pkt.data;

ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]); ff_rtmp_packet_destroy(&pkt);

- // set client buffer time disguised in ping packet - ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_PING, 1, 10); + if (rt->is_input) { + // set client buffer time disguised in ping packet + ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, RTMP_PT_PING, 1, 10);

- p = pkt.data; - bytestream_put_be16(&p, 3); - bytestream_put_be32(&p, 1); - bytestream_put_be32(&p, 256); //TODO: what is a good value here? + p = pkt.data; + bytestream_put_be16(&p, 3); + bytestream_put_be32(&p, 1); + bytestream_put_be32(&p, 256); //TODO: what is a good value here?

- ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]); - ff_rtmp_packet_destroy(&pkt); + ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]); + ff_rtmp_packet_destroy(&pkt); + } }

/** @@ -205,6 +254,25 @@ static void gen_pong(URLContext *s, RTMPContext *rt, RTMPPacket *ppkt) ff_rtmp_packet_destroy(&pkt); }

+/** + * Generates client/server bandwidth reply and sends it to the server. + */ +static void gen_bw(URLContext *s, RTMPContext *rt, RTMPPacket *ppkt) +{ + RTMPPacket pkt; + uint8_t *p; + + if ((rt->is_input && ppkt->type == RTMP_PT_CLIENT_BW) || + (!rt->is_input && ppkt->type == RTMP_PT_SERVER_BW)) + return; + ff_rtmp_packet_create(&pkt, RTMP_NETWORK_CHANNEL, rt->is_input ? RTMP_PT_CLIENT_BW : RTMP_PT_SERVER_BW, + 0, 4); + p = pkt.data; + bytestream_put_be32(&p, AV_RB32(ppkt->data)); + ff_rtmp_packet_write(rt->stream, &pkt, rt->chunk_size, rt->prev_pkt[1]); + ff_rtmp_packet_destroy(&pkt); +}

Also this seems to belong to a separate patch. [...] This feature is definitively welcome, but please try to split the patch in separate minimal changes. Regards. -- FFmpeg = Fiendish and Foolish Multimedia Peaceless Eccentric Gadget