On 08/04/14 17:16, Alessandro Ghedini wrote:
> From: Michael Niedermayer <[email protected]>
>
> Instead of using a fixed bitrate_idx, try to calculate a matching bitrate for
> the XING header.
>
> Using a fixed bitrate_idx causes tools such as file(1) and mediainfo(1) to
> report wrong bitrate and bitrate mode when using CBR.
>
> Bug: https://bugs.debian.org/736088
> ---
> This was adapted from ffmpeg's commit 40176fc3 and pre-existing code.
>
> libavformat/mp3enc.c | 45 +++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
> index 46889fc..4d9ba2b 100644
> --- a/libavformat/mp3enc.c
> +++ b/libavformat/mp3enc.c
> @@ -119,6 +119,8 @@ static void mp3_write_xing(AVFormatContext *s)
> MPADecodeHeader mpah;
> int srate_idx, i, channels;
> int bitrate_idx;
> + int best_bitrate_idx;
> + int best_bitrate_error = INT_MAX;
> int xing_offset;
> int ver = 0;
>
> @@ -150,14 +152,49 @@ static void mp3_write_xing(AVFormatContext *s)
> return;
> }
>
> - /* 64 kbps frame, should be large enough */
> - bitrate_idx = (ver == 3) ? 5 : 8;
> -
> /* dummy MPEG audio header */
> header = 0xff << 24; // sync
> header |= (0x7 << 5 | ver << 3 | 0x1 << 1 | 0x1) << 16; //
> sync/audio-version/layer 3/no crc*/
> - header |= (bitrate_idx << 4 | srate_idx << 2) << 8;
> + header |= (srate_idx << 2) << 8;
> header |= channels << 6;
> +
> + for (bitrate_idx = 1; bitrate_idx < 15; bitrate_idx++) {
> + int error;
> +
So you build the header,
> + avpriv_mpegaudio_decode_header(&mpah, header | (bitrate_idx <<
> (4+8)));
And then parse it again instead of
getting
layer = 4 - ((header >> 17) & 3);
if (header & (1<<20) && header & (1<<19))
lsf = 0;
else
lsf = 1;
once
and then compute bitrate as
bit_rate = 1000 * avpriv_mpa_bitrate_tab[lsf][layer - 1][bitrate_index];
and be done with that.
nutenc mp3 header elision code would share a similar function (it needs
the frame size) and I have an excuse to remove an exported table for the
joy of the windows users.
> + error = FFABS(mpah.bit_rate - codec->bit_rate);
> +
> + if (error < best_bitrate_error){
> + best_bitrate_error = error;
> + best_bitrate_idx = bitrate_idx;
> + }
> + }
> + av_assert0(best_bitrate_idx >= 0);
Please do not bring asserts when porting patches.
> + for (bitrate_idx = best_bitrate_idx; bitrate_idx < 15; bitrate_idx++) {
> + int needed;
> + int32_t mask;
> +
> + mask = bitrate_idx << (4 + 8);
> + header |= mask;
> + avpriv_mpegaudio_decode_header(&mpah, header);
I think the number of channels is already available above w/out having
to parse again many times the header.
> + xing_offset = xing_offtbl[mpah.lsf == 1][mpah.nb_channels == 1];
> + needed = 4 // header
> + + xing_offset
> + + 4 // xing tag
> + + 4 // frames/size/toc flags
> + + 4 // frames
> + + 4 // size
> + + XING_TOC_SIZE // toc
> + + 24
> + ;
> +
> + if (needed <= mpah.frame_size)
> + break;
> +
> + header &= ~mask;
> + }
> avpriv_mpegaudio_decode_header(&mpah, header);
Maybe I can get you a function returning bitrate and framesize and make
this code a bit more simple.
lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
