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

[zhentan feng]

Hi

On Sat, Mar 27, 2010 at 1:54 AM, Ronald S. Bultje <rsbultje at gmail.com>wrote:

> Hi,
>
> On Thu, Mar 25, 2010 at 11:49 AM, zhentan feng <spyfeng at gmail.com> wrote:
> > here is the new patch according to the reviews.
>
> Couple more small things I noticed while looking over it.
>
> mms_read():
> > +    assert(mms->header_parsed);
> > +
> > +    if(mms->header_parsed) {
>
> Makes no sense, the if() can be removed.
>
> 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?


> 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.
        }


zhentan
-- 
Best wishes~