[FFmpeg-devel] [patch]MMS protocol over TCP

Ronald S. Bultje rsbultje
Sat Mar 27 16:04:17 CET 2010


Hi Zhentan,

On Fri, Mar 26, 2010 at 11:42 PM, zhentan feng <spyfeng at gmail.com> wrote:
> On Sat, Mar 27, 2010 at 1:54 AM, Ronald S. Bultje <rsbultje at gmail.com>wrote:
>> in get_tcp_server_response():
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ?mms->asf_header =
>> av_realloc(mms->asf_header,
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?mms->asf_header_size
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?+ mms->pkt_buf_len);
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ?memcpy(mms->asf_header +
>> mms->asf_header_size,
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? mms->pkt_read_ptr,
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? mms->pkt_buf_len);
>> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ?mms->asf_header_size += mms->pkt_buf_len;
>>
>> Weird indenting...
>
> the code lines are exceed 80 characters, I want split into two more lines.
> which style should I use?

Whichever looks best to you. If this is the best, then it's OK and you
can leave it.

>> in read_mms_packet():
>> > + ? ? ? ?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);
>> > + ? ? ? ?} else if(mms->pkt_buf_len) {
>>
>> After this, you can av_freep() the asf_header already, we no longer need
>> it.
>
> IMHO, we can free the header once it has been parsed.
> ie, we can free the memory after this lines:
> ? case SC_PKT_ASF_HEADER:
> ? ? ? ?if((mms->incoming_flags == 0X08) || (mms->incoming_flags == 0X0C)) {
> ? ? ? ? ? ?ret = asf_header_parser(mms);
> ? ? ? ? ? ?mms->header_parsed = 1;
> ? ? ? ? ? av_freep(mms->asf_header)// add here.
> ? ? ? ?}

That wouldn't work, since we A) parse the header, and then B) serve it
as first data (ASF file header) to the demuxer. Only after that can we
free it. If you implement what you suggest, the demuxer would probably
not work anymore, or does it?

Ronald



More information about the ffmpeg-devel mailing list