[FFmpeg-devel] [FFmpeg-cvslog] asfdec: skip the stream bitrate list

[Anton Khirnov]

On Sat, Feb 12, 2011 at 11:42:17AM +0100, Reimar D?ffinger wrote:
> On Fri, Feb 11, 2011 at 03:52:13AM +0100, Anton Khirnov wrote:
> > ffmpeg | branch: master | Anton Khirnov <anton at khirnov.net> | Wed Feb  9 21:55:52 2011 +0100| [d9286510110638bcec3ef8ea56975e0154dea609] | committer: Michael Niedermayer
> > 
> > asfdec: skip the stream bitrate list
> > 
> > Its contents aren't used for anything.
> > 
> > Signed-off-by: Ronald S. Bultje <rsbultje at gmail.com>
> > (cherry picked from commit d7a5106eb2dad33765b0e5f11fd8b1a87e5a9b4b)
> > 
> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=d9286510110638bcec3ef8ea56975e0154dea609
> > ---
> > 
> >  libavformat/asfdec.c |   15 ---------------
> >  1 files changed, 0 insertions(+), 15 deletions(-)
> > 
> > diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
> > index 8d79983..41f849e 100644
> > --- a/libavformat/asfdec.c
> > +++ b/libavformat/asfdec.c
> > @@ -425,21 +425,6 @@ static int asf_read_header(AVFormatContext *s, AVFormatParameters *ap)
> >              get_tag(s, "copyright", 0, len3);
> >              get_tag(s, "comment"  , 0, len4);
> >              url_fskip(pb, len5);
> > -        } else if (!ff_guidcmp(&g, &stream_bitrate_guid)) {
> > -            int stream_count = get_le16(pb);
> > -            int j;
> > -
> > -//            av_log(s, AV_LOG_ERROR, "stream bitrate properties\n");
> > -//            av_log(s, AV_LOG_ERROR, "streams %d\n", streams);
> > -            for(j = 0; j < stream_count; j++) {
> > -                int flags, bitrate, stream_id;
> > -
> > -                flags= get_le16(pb);
> > -                bitrate= get_le32(pb);
> > -                stream_id= (flags & 0x7f);
> > -//                av_log(s, AV_LOG_ERROR, "flags: 0x%x stream id %d, bitrate %d\n", flags, stream_id, bitrate);
> > -                asf->stream_bitrates[stream_id]= bitrate;
> > -            }
> 
> Are you really sure about that?
> IIRC these values are only overriden by an "ext" header, which I'd expect to be
> optional, also mms:// etc. usually sends a (partial?) header to allow stream
> selection and I don't know if it might be possible that only this one exists
> in some cases.
> It might be worth to at least keep a comment about the format of this.

What I'm sure about is that this code doesn't do anything (and never did
-- 0e3b6a6f55eaa060166ab4f02f4d22cfc75d8d27) while pretending that it
does, which is confusing and evil. And since nobody changed this in all that
time, I think removing it is the right thing to do.

If/when somebody finds a use for those values, then PATCHWELCOME.
As for leaving a comment -- the specs are publicly available now. I
don't think duplicating even the parts we don't use here is a good idea.

-- 
Anton Khirnov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110212/ff0319c0/attachment.pgp>