[FFmpeg-devel] [patch]MMS protocol over TCP
Michael Niedermayer
michaelni
Tue Apr 20 15:09:37 CEST 2010
On Tue, Apr 20, 2010 at 12:46:17AM +0800, zhentan feng wrote:
> Hi
>
> On Mon, Apr 19, 2010 at 6:39 AM, Michael Niedermayer <michaelni at gmx.at>wrote:
[...]
> >
> > [...]
> > > +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?
here:
> + int padding_size = mms->asf_packet_len - mms->pkt_buf_len;
> + memset(mms->incoming_buffer + mms->pkt_buf_len, 0, padding_size);
the memset maybe can write over the end of the array
[...]
> here is the new version patch attached below.
> please review.
> zhentan
> --
> Best wishes~
> Makefile | 1
> allformats.c | 1
> mmst.c | 679 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 681 insertions(+)
> ca0c1492faa9d4343af8882ec02392ccfe4c7851 mmst_10.patch
> Index: libavformat/mmst.c
> ===================================================================
> --- libavformat/mmst.c (revision 0)
> +++ libavformat/mmst.c (revision 0)
> @@ -0,0 +1,679 @@
> +/*
> + * MMS protocol over TCP
> + * Copyright (c) 2006,2007 Ryan Martell
> + * Copyright (c) 2007 Bj?rn Axelsson
> + * Copyright (c) 2010 Zhentan Feng <spyfeng at gmail dot com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include "avformat.h"
> +#include "internal.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavcodec/bytestream.h"
> +#include "network.h"
> +#include "asf.h"
> +
> +#define LOCAL_ADDRESS 0xc0a80081 // FIXME get and use correct local ip address.
> +#define LOCAL_PORT 1037 // as above.
> +/** Client to server packet types. */
> +typedef enum {
> + CS_PKT_INITIAL = 0x01,
> + CS_PKT_PROTOCOL_SELECT = 0x02,
> + CS_PKT_MEDIA_FILE_REQUEST = 0x05,
> + CS_PKT_START_FROM_PKT_ID = 0x07,
> + CS_PKT_STREAM_PAUSE = 0x09,
> + CS_PKT_STREAM_CLOSE = 0x0d,
> + CS_PKT_MEDIA_HEADER_REQUEST = 0x15,
> + CS_PKT_TIMING_DATA_REQUEST = 0x18,
> + CS_PKT_USER_PASSWORD = 0x1a,
> + CS_PKT_KEEPALIVE = 0x1b,
> + CS_PKT_STREAM_ID_REQUEST = 0x33,
> +} MMSCSPacketType;
> +
> +/** Server to client packet types. */
> +typedef enum {
> + /** Control packets. */
> + /*@{*/
> + SC_PKT_CLIENT_ACCEPTED = 0x01,
> + SC_PKT_PROTOCOL_ACCEPTED = 0x02,
> + SC_PKT_PROTOCOL_FAILED = 0x03,
> + SC_PKT_MEDIA_PKT_FOLLOWS = 0x05,
> + SC_PKT_MEDIA_FILE_DETAILS = 0x06,
> + SC_PKT_HEADER_REQUEST_ACCEPTED = 0x11,
> + SC_PKT_TIMING_TEST_REPLY = 0x15,
> + SC_PKT_PASSWORD_REQUIRED = 0x1a,
> + SC_PKT_KEEPALIVE = 0x1b,
> + SC_PKT_STREAM_STOPPED = 0x1e,
> + SC_PKT_STREAM_CHANGING = 0x20,
> + SC_PKT_STREAM_ID_ACCEPTED = 0x21,
> + /*@}*/
> +
> + /** Pseudo packets. */
> + /*@{*/
> + SC_PKT_CANCEL = -1,
> + SC_PKT_NO_DATA = -2,
> + SC_PKT_HTTP_CONTROL_ACKNOWLEDGE = -3,
> + /*@}*/
> +
> + /** Data packets. */
> + /*@{*/
> + SC_PKT_ASF_HEADER = 0x81,
> + SC_PKT_ASF_MEDIA = 0x82,
> + /*@}*/
> +} MMSSCPacketType;
> +
> +typedef struct {
> + int id;
> +}MMSStream;
> +
> +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; ///< Pointer for writting the buffer.
> + uint8_t outgoing_packet_buffer[512]; ///< Buffer for outgoing packet.
> + /*@}*/
> +
> + /** Buffer for incoming packets. */
> + /*@{*/
> + uint8_t incoming_buffer[8192]; ///< Buffer for incoming packets.
> + uint8_t *pkt_read_ptr; ///< Pointer for reading from incoming buffer.
> + int pkt_buf_len; ///< Reading length from incoming buffer.
> + /*@}*/
i would suggest more consistentnames like for example:
uint8_t out_buffer[512];
uint8_t *write_out_ptr;
uint8_t in_buffer[8192]
uint8_t *read_in_ptr;
int remaining_in_length;
(of course you can use different names if you prefer)
[...]
> +/** Read incoming MMST media, header or command packet. */
> +static MMSSCPacketType get_tcp_server_response(MMSContext *mms)
> +{
> + int read_result;
> + MMSSCPacketType packet_type= -1;
> +
> + for(;;) {
> + 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) {
> + packet_type= AV_RL16(mms->incoming_buffer+36);
is any value valid here?
i think this is missing some checks
> +
> + } 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
this comment does not seem correct anymore
[...]
> +/** Send MMST stream selection command based on the AVStream->discard values. */
> +static int send_stream_selection_request(MMSContext *mms)
> +{
> + int i;
> +
> + // 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
> + if (mms->write_ptr - mms->outgoing_packet_buffer >
> + sizeof(mms->outgoing_packet_buffer) - 6 * mms->stream_num) {
> + dprintf("buffer will overflow for too many streams: %d.\n", mms->stream_num);
> + return AVERROR_IO;
> + }
this should probably be checked before adding the streams
[...]
> +/** Read at most one media packet (or a whole header). */
> +static int read_mms_packet(MMSContext *mms, uint8_t *buf, int buf_size)
> +{
> + int result = 0;
> + MMSSCPacketType packet_type;
> + int size_to_copy;
> +
> + do {
> + if(mms->asf_header_read_pos < mms->asf_header_size) {
> + /* Read from ASF header buffer */
> + size_to_copy= FFMIN(buf_size,
> + mms->asf_header_size - mms->asf_header_read_pos);
> + memcpy(buf, mms->asf_header + mms->asf_header_read_pos, size_to_copy);
> + mms->asf_header_read_pos += size_to_copy;
> + result += size_to_copy;
> + dprintf(NULL, "Copied %d bytes from stored header. left: %d\n",
> + size_to_copy, mms->asf_header_size - mms->asf_header_read_pos);
> + av_freep(&mms->asf_header);
> + } else if(mms->pkt_buf_len) {
> + /* Read from media packet buffer */
> + result = read_data(mms, buf, buf_size);
> + } else {
> + /* Read from network */
> + packet_type= get_tcp_server_response(mms);
> + if (packet_type == SC_PKT_ASF_MEDIA) {
> + if(mms->pkt_buf_len>mms->asf_packet_len) {
> + dprintf(NULL, "Incoming packet"
a non null context would be better
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100420/653846b1/attachment.pgp>
More information about the ffmpeg-devel
mailing list