[FFmpeg-devel] [patch]MMS protocol over TCP
zhentan feng
spyfeng
Mon Apr 19 18:46:17 CEST 2010
Hi
On Mon, Apr 19, 2010 at 6:39 AM, Michael Niedermayer <michaelni at gmx.at>wrote:
>
> [...]
> > +typedef struct {
> > + int outgoing_packet_seq; ///< Outgoing packet sequence
> number.
> > + char path[256]; ///< Path of the resource being
> asked for.
> > + char host[128]; ///< Host of the resources.
> > +
> > + URLContext *mms_hd; ///< TCP connection handle
> > + MMSStream streams[MAX_STREAMS];
> > +
> > + /** Buffer for outgoing packets. */
> > + /*@{*/
> > + uint8_t *write_ptr;
> > + uint8_t outgoing_packet_buffer[512]; ///< Outgoing packet data
> > + /*@}*/
> > +
> > + /** Buffer for incoming control packets. */
> > + /*@{*/
> > + uint8_t incoming_buffer[8192]; ///< Incoming buffer location.
> > + int incoming_buffer_length; ///< Incoming buffer length.
> > + /*@}*/
> > +
> > + /** Buffer for incoming media/header packets. */
>
> > + /*@{*/
> > + uint8_t *pkt_read_ptr; ///< Pointer for partial reads.
> > + int pkt_buf_len; ///< Buffer length.
> > + int pkt_offset; ///< offset in packet.
> > + /*@}*/
>
> the doxy is quite terse. Which makes it hard to understand what these
> fields represent exactly. One would have to look at how they are use to
> understand what they are
>
> also nothing ever sets pkt_offset to a non zero value thus the variable
> is uneeded
>
>
fixed.
> [...]
>
> > +}
> > +
> > +/** Read incoming MMST media, header or command packet. */
> > +static MMSSCPacketType get_tcp_server_response(MMSContext *mms)
> > +{
> > + int read_result;
> > + MMSSCPacketType packet_type= -1;
> > + int done;
> > +
> > + do {
> > + done= 1;
> > + if((read_result= url_read_complete(mms->mms_hd,
> mms->incoming_buffer, 8))==8) {
> > + // handle command packet.
> > + if(AV_RL32(mms->incoming_buffer + 4)==0xb00bface) {
> > + mms->incoming_flags= mms->incoming_buffer[3];
> > + read_result= url_read_complete(mms->mms_hd,
> mms->incoming_buffer+8, 4);
> > + if(read_result == 4) {
> > + int length_remaining=
> AV_RL32(mms->incoming_buffer+8) + 4;
> > +
> > + dprintf(NULL, "Length remaining is %d\n",
> length_remaining);
> > + // read the rest of the packet.
> > + if (length_remaining < 0
> > + || length_remaining >
> sizeof(mms->incoming_buffer) - 12) {
> > + dprintf("Incoming message len %d exceeds buffer
> len %d\n",
> > + length_remaining,
> sizeof(mms->incoming_buffer) - 12);
> > + break;
> > + }
> > + read_result = url_read_complete(mms->mms_hd,
> mms->incoming_buffer + 12,
> > + length_remaining) ;
> > + if (read_result == length_remaining) {
> > + mms->incoming_buffer_length=
> length_remaining+12;
> > + packet_type= AV_RL16(mms->incoming_buffer+36);
> > +
> > + } else {
> > + dprintf(NULL, "3 read returned %d!\n",
> read_result);
> > + }
> > + } else {
> > + dprintf(NULL, "2 read returned %d!\n", read_result);
> > + }
> > + } else {
> > + int length_remaining;
> > + int packet_id_type;
> > + int tmp;
> > +
> > + assert(mms->pkt_buf_len==0);
> > +
> > + //** VERIFY LENGTH REMAINING HAS SPACE
> > + // note we cache the first 8 bytes,
> > + // then fill up the buffer with the others
> > + tmp = AV_RL16(mms->incoming_buffer
> + 6);
> > + length_remaining = (tmp - 8) & 0xffff;
> > + mms->incoming_packet_seq =
> AV_RL32(mms->incoming_buffer);
> > + packet_id_type = mms->incoming_buffer[4];
> > + mms->incoming_flags = mms->incoming_buffer[5];
> > +
> > + if (length_remaining < 0
> > + || length_remaining >
> sizeof(mms->incoming_buffer)) {
> > + dprintf("Incoming data len %d exceeds buffer len
> %d\n",
> > + length_remaining,
> sizeof(mms->incoming_buffer));
> > + break;
> > + }
> > + mms->pkt_buf_len = length_remaining;
> > + mms->pkt_read_ptr = mms->incoming_buffer;
> > + read_result= url_read_complete(mms->mms_hd,
> mms->incoming_buffer, length_remaining);
> > + if(read_result != length_remaining) {
> > + dprintf(NULL, "read_bytes result: %d asking for
> %d\n",
> > + read_result, length_remaining);
> > + break;
> > + } else {
> > + // if we successfully read everything.
> > + if(packet_id_type == mms->header_packet_id) {
> > + packet_type = SC_PKT_ASF_HEADER;
> > + // Store the asf header
> > + if(!mms->header_parsed) {
> > + void *p = av_realloc(mms->asf_header,
> > + mms->asf_header_size
> > + + mms->pkt_buf_len);
> > + if (!p) {
> > + av_freep(&mms->asf_header);
> > + return AVERROR(ENOMEM);
> > + }
> > + mms->asf_header = p;
> > + memcpy(mms->asf_header +
> mms->asf_header_size,
> > + mms->pkt_read_ptr,
> > + mms->pkt_buf_len);
> > + mms->asf_header_size += mms->pkt_buf_len;
> > + }
> > + } else if(packet_id_type == mms->packet_id) {
> > + packet_type = SC_PKT_ASF_MEDIA;
> > + } else {
> > + dprintf(NULL, "packet id type %d is old.",
> packet_id_type);
> > + done= 0;
> > + }
> > + }
> > + }
> > + } else {
> > + if(read_result<0) {
> > + dprintf(NULL, "Read error (or cancelled) returned
> %d!\n", read_result);
> > + packet_type = SC_PKT_CANCEL;
> > + } else {
> > + dprintf(NULL, "Read result of zero?!\n");
> > + packet_type = SC_PKT_NO_DATA;
> > + }
> > + done = 1;
> > + }
> > + } while(!done);
>
> done can be replaced by break; and continue; which are clearer to
> understand
>
>
fixed.
>
> [...]
> > +static int asf_header_parser(MMSContext *mms)
> > +{
> > + uint8_t *p = mms->asf_header, *end = mms->asf_header +
> mms->asf_header_size;
> > + int flags, stream_id;
> > + mms->stream_num = 0;
> > +
> > + if (mms->asf_header_size < sizeof(ff_asf_guid) * 2 + 22 ||
> > + memcmp(p, ff_asf_header, sizeof(ff_asf_guid)))
> > + return -1;
> > +
> > + p += sizeof(ff_asf_guid) + 14;
> > + do {
>
> > + uint64_t chunksize = AV_RL64(p + sizeof(ff_asf_guid));
> > + if (!memcmp(p, ff_asf_file_header, sizeof(ff_asf_guid))) {
> > + /* read packet size */
> > + if (end - p > sizeof(ff_asf_guid) * 2 + 68) {
>
> > + mms->asf_packet_len = AV_RL32(p + sizeof(ff_asf_guid) *
> 2 + 64);
>
> this looks unsafe, there is code that writes based on asf_packet_len
how the code make it unsafe? could you please explain it?
> > +
> > + // send the streams we want back...
> > + start_command_packet(mms, CS_PKT_STREAM_ID_REQUEST);
> > + bytestream_put_le32(&mms->write_ptr, mms->stream_num); //
> stream nums
> > + for(ii= 0; ii<mms->stream_num; ii++) {
> > + bytestream_put_le16(&mms->write_ptr, 0xffff); //
> flags
> > + bytestream_put_le16(&mms->write_ptr, mms->streams[ii].id); //
> stream id
> > + bytestream_put_le16(&mms->write_ptr, 0); //
> selection
> > + }
>
> buffer overflow
>
>
added the code for checking the buffer before writting.
>
> > +
> > + bytestream_put_le16(&mms->write_ptr, 0);
> > +
> > + return send_command_packet(mms);
> > +}
> > +
> > +static void read_data(MMSContext *mms, uint8_t *buf, const int buf_size,
> int* result)
> > +{
> > + int read_size;
> > + read_size = FFMIN(buf_size, mms->pkt_buf_len);
> > + memcpy(buf, mms->pkt_read_ptr, read_size);
> > + mms->pkt_buf_len -= read_size;
> > + mms->pkt_read_ptr+= read_size;
> > + *result += read_size;
> > +}
>
> It would be cleaner to return "result" normally by return
>
fixed.
here is the new version patch attached below.
please review.
zhentan
--
Best wishes~
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mmst_10.patch
Type: application/octet-stream
Size: 26527 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100420/8859b54d/attachment.obj>
More information about the ffmpeg-devel
mailing list