[Ffmpeg-devel] Third Try: Set bit_rate for asf format

[Michael Niedermayer]

Hi

On Mon, Apr 16, 2007 at 09:27:11AM +0800, Zuxy Meng wrote:
> Hi,
> 
> 2007/4/16, Michael Niedermayer <michaelni at gmx.at>:
> >Hi
> >
> >On Mon, Apr 02, 2007 at 01:26:40PM +0800, Zuxy Meng wrote:
> >> Hi,
> >>
> >> 2007/3/31, Zuxy Meng <zuxy.meng at gmail.com>:
> >> >Hi,
> >> >
> >> >2007/3/30, Zuxy Meng <zuxy.meng at gmail.com>:
> >> >> Hi,
> >> >>
> >> >> 2007/3/30, Michael Niedermayer <michaelni at gmx.at>:
> >> >> > Hi
> >> >> >
> >> >> > still i have a bad feeling with setting it just for video knowing it
> >> >> > breakes audio if it would be set there too ...
> >> >>
> >> >> Me too. I'll dig into wma.c this weekend.
> >> >>
> >> >> > somehow this isnt a clean solution at all
> >> >
> >> >The reason is that for audio streams, there will be a wave header
> >> >defined at the asf container level, and get_wav_header will set
> >> >bit_rate as 8*nAvgBytesPerSec. So unlike video streams, we don't need
> >> >to look for the optional extended stream property object for bitrate.
> >> >
> >> >And something more about the fulltest: asf files used/generated in
> >> >fulltest don't contain extended stream property objects so bitrate[i]
> >> >== 0, and the subsequent assignment will effectively overwrite what
> >> >has been found in get_wav_header. This is absolutely my fault; I
> >> >forgot that such objects are optional, not mandatory.
> >>
> >> Please take a look at this patch; hopefully this is cleaner. And it
> >> passes fulltest, too.
> >[...]
> >> @@ -500,11 +505,14 @@
> >>
> >>      for(i=0; i<128; i++){
> >>          int stream_num= asf->asfid2avid[i];
> >> -        if(stream_num>=0 && dar[i].num>0 && dar[i].den>0){
> >> +        if(stream_num>=0){
> >>              AVCodecContext *codec= s->streams[stream_num]->codec;
> >> -            av_reduce(&codec->sample_aspect_ratio.num,
> >> -                    &codec->sample_aspect_ratio.den,
> >> -                    dar[i].num, dar[i].den, INT_MAX);
> >> +            if (bitrate[i])
> >> +                codec->bit_rate = bitrate[i];
> >
> >this still overrides the bitrate and IMHO will break WMA like the previous
> >patches, its just that bitrate[i] is 0 during the regression tests but this
> >is not guranteed to be true for files in the wild
> 
> bitrate[i] is 0 because asf files used in the regression tests don't
> contain a bitrate object. IMHO the encoder should guanrantee that the
> bitrate in the wave header and the one in the object (if present) be
> the same.
> 
> Is codec->bit_rate always initialized to zero? If so we can use

i think so


> if (codec->bit_rate)
> so bitrate in the bitrate object won't overwrite the wave header info.

this looks better

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070416/2b719806/attachment.pgp>