[FFmpeg-devel] [Patch] MMS protocol bug fixed
zhentan feng
spyfeng
Sat Jul 17 05:43:16 CEST 2010
Hi
The problem is focused on #3 patch.
let me explain it.
On Sat, Jul 17, 2010 at 12:42 AM, Ronald S. Bultje <rsbultje at gmail.com>wrote:
> Hi,
>
> On Fri, Jul 16, 2010 at 11:41 AM, zhentan feng <spyfeng at gmail.com> wrote:
> > 0: add a guid for asf header parser.
> [..]
> > +const ff_asf_guid ff_asf_stream_bitrate_properties = {
> > + 0xce, 0x75, 0xf8, 0x7b, 0x8d, 0x46, 0xd1, 0x11, 0x8d, 0x82,
> 0x00, 0x60, 0x97, 0xc9, 0xa2, 0xb2
> > +};
>
> Is this still used? (See below.)
>
>
I think it's necessary.
some asf header only has ff_asf_stream_bitrate_properties without stream
properties.
if don't parse it, we cann't get the stream_num.
for example the link:
mmst://61.139.126.90/mtv
> > 1: add time test data message for making it strictly with spec.
>
> OK.
>
> > 2: check the returned error code from the server side.
> [..]
> > + hr = AV_RL32(mms->in_buffer + 40);
> > + if (hr)
> > + {
>
> Same line please ("if (..) {").
>
> > 3: recv asf header sent by multiple packet
> [..]
> > @@ -335,6 +333,8 @@
> > mms->remaining_in_len);
> > mms->asf_header_size +=
> mms->remaining_in_len;
> > }
> > + if (mms->incoming_flags == 0x04)
> > + continue;
> > } else if(packet_id_type == mms->packet_id) {
> > packet_type = SC_PKT_ASF_MEDIA;
> > } else {
>
> Add a comment please what that means ("header split over multiple
> packets").
>
>
In MMS protocol specification section 2.2.2 (see the link)
http://msdn.microsoft.com/en-us/library/cc234781(v=PROT.10).aspx
"When the Payload field contains an ASF file header, the AFFlags
field MUST be set to 0x04 if the ASF file header is split into
multiple Data packets. AFFlags
MUST be set to 0x0C if this is the last Data packet in the
sequence. "
> @@ -472,6 +472,22 @@
> > dprintf(NULL, "Too many streams.\n");
> > return -1;
> > }
> > + } else if (!memcmp(p, ff_asf_head1_guid, sizeof(ff_asf_guid))) {
> > + chunksize = 46;
>
> I don't quite see the point. Why do you force the chunksize for head1
> GUIDs? Does that fix a bug? If so, which?
>
>
In asf specification section 3.4
Object Size
Specifies the size, in bytes, of the *Header Extension Object*. The value of
this field shall be set to 46 bytes.
the chunksize read from the top is not 46.
this is an unusual situation, should be handled particularly.
> > + } else if (!memcmp(p, ff_asf_stream_bitrate_properties,
> sizeof(ff_asf_guid))) {
> > + int record_cnt = AV_RL16(p + sizeof(ff_asf_guid) + 8);
> > + if (record_cnt*6 + 16 + 8 + 2 > chunksize) {
> > + dprintf(NULL, "Too many stream record count.\n");
> > + return -1;
> > + }
> > + if (mms->stream_num < MAX_STREAMS &&
> > + 46 + mms->stream_num * 6 < sizeof(mms->out_buffer)) {
> > + mms->stream_num = record_cnt;
> > + is_stream_num_known = 1;
> > + } else {
> > + dprintf(NULL, "Too many streams(bitrate properties)\n");
> > + return -1;
> > + }
> > }
> > p += chunksize;
> > }
>
> This doesn't so anything other than setting mms->stream_num, which
> only complicates the code (so this commit introduces a bug which you
> fix in a next patch). What is the goal of this code? If only to set
> stream_num, can't we just skip this, thus simplifying the next
> patches?
>
> This also doesn't compile because is_stream_num_known is undeclared.
>
> my fault, I modified it in #6.
> For both chunks, how is this part of the "receive ASF header sent in
> multiple packets" patch?
>
> > 4: make the message 8 bytes aligned.
>
> OK.
>
> > 5: fix a possible bug when cann't read the asf header one time.
>
> Should be in patch #3, most likely. OK otherwise.
>
> no. it's a separated patch.
this is for read_mms_packet().
in original code, read_header_size is a local variable.
if read_header_size< mms->asf_header_size at first read,
then call read_mms_packet() again, we can not satisfy the condition:
if (mms->asf_header_size == read_header_size) {
av_freep(&mms->asf_header);
mms->is_playing = 1;
}
> > 6: fix a bug for send stream message select.
>
> I'm hoping we don't need this if we don't apply the relevant chunk in #3...
>
>
the attachment are updated patches with order same as before.
thanks
zhentan
--
Best wishes~
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0_add_guid.patch
Type: application/octet-stream
Size: 1069 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100717/1baac0ce/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1_time_test.patch
Type: application/octet-stream
Size: 827 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100717/1baac0ce/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2_error_code.patch
Type: application/octet-stream
Size: 1053 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100717/1baac0ce/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3_multi_asf_header.patch
Type: application/octet-stream
Size: 3968 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100717/1baac0ce/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 4_8bytes_msg_aligned.patch
Type: application/octet-stream
Size: 1084 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100717/1baac0ce/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 5_asf_header_read_size.patch
Type: application/octet-stream
Size: 1865 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100717/1baac0ce/attachment-0005.obj>
More information about the ffmpeg-devel
mailing list