Jan Ekström:
> On Mon, Jun 20, 2022 at 12:10 PM Andreas Rheinhardt
> <andreas.rheinha...@outlook.com> wrote:
>>
>> Jan Ekström:
>>> From: Jan Ekström <jan.ekst...@24i.com>
>>>
>>> Signed-off-by: Jan Ekström <jan.ekst...@24i.com>
>>> ---
>>> libavformat/Makefile | 1 +
>>> libavformat/ac3_bitrate_tab.c | 22 ++++++++++++++
>>> libavformat/movenc.c | 55 +++++++++++++++++------------------
>>> 3 files changed, 50 insertions(+), 28 deletions(-)
>>> create mode 100644 libavformat/ac3_bitrate_tab.c
>>>
>>> diff --git a/libavformat/Makefile b/libavformat/Makefile
>>> index 1416bf31bd..c71de95b2f 100644
>>> --- a/libavformat/Makefile
>>> +++ b/libavformat/Makefile
>>> @@ -699,6 +699,7 @@ SHLIBOBJS-$(CONFIG_FLV_MUXER) +=
>>> mpeg4audio_sample_rates.o
>>> SHLIBOBJS-$(CONFIG_HLS_DEMUXER) += ac3_channel_layout_tab.o
>>> SHLIBOBJS-$(CONFIG_MATROSKA_DEMUXER) += mpeg4audio_sample_rates.o
>>> SHLIBOBJS-$(CONFIG_MOV_DEMUXER) += ac3_channel_layout_tab.o
>>> +SHLIBOBJS-$(CONFIG_MOV_MUXER) += ac3_bitrate_tab.o
>>> SHLIBOBJS-$(CONFIG_MP3_MUXER) += mpegaudiotabs.o
>>> SHLIBOBJS-$(CONFIG_MXF_MUXER) += golomb_tab.o
>>> SHLIBOBJS-$(CONFIG_NUT_MUXER) += mpegaudiotabs.o
>>
>> The above line only takes care of duplicating ac3_bitrate_tab.o into
>> lavf for shared builds; yet there needs to be a corresponding STLIBOBJS
>> entry in lavc/Makefile so that lavc/ac3_bitrate_tab.o is compiled in
>> case the mov muxer is enabled in a static build.
>> It would work either way in this case because #2 made the mov muxer
>> depend upon the ac3 parser which makes lavc/ac3_bitrate_tab.o be included.
>>
>
> OK, then I clearly missed something since I thought I was following
> how you had done things earlier for the other table. Will look into
> this tomorrow morning.
>
>>> diff --git a/libavformat/ac3_bitrate_tab.c b/libavformat/ac3_bitrate_tab.c
>>> new file mode 100644
>>> index 0000000000..57b6181511
>>> --- /dev/null
>>> +++ b/libavformat/ac3_bitrate_tab.c
>>> @@ -0,0 +1,22 @@
>>> +/*
>>> + * AC-3 bit rate table
>>> + * copyright (c) 2001 Fabrice Bellard
>>> + *
>>> + * This file is part of FFmpeg.
>>> + *
>>> + * FFmpeg is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * FFmpeg is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with FFmpeg; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> 02110-1301 USA
>>> + */
>>> +
>>> +#include "libavcodec/ac3_bitrate_tab.h"
>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>> index 103f958d75..a071f1cdd5 100644
>>> --- a/libavformat/movenc.c
>>> +++ b/libavformat/movenc.c
>>> @@ -36,6 +36,7 @@
>>> #include "av1.h"
>>> #include "avc.h"
>>> #include "libavcodec/ac3_parser_internal.h"
>>> +#include "libavcodec/ac3tab.h"
>>> #include "libavcodec/dnxhddata.h"
>>> #include "libavcodec/flac.h"
>>> #include "libavcodec/get_bits.h"
>>> @@ -362,44 +363,42 @@ struct eac3_info {
>>>
>>> static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack
>>> *track)
>>> {
>>> - GetBitContext gbc;
>>> + struct eac3_info *info = track->eac3_priv;
>>> + int8_t ac3_bit_rate_code = -1;
>>> PutBitContext pbc;
>>> uint8_t buf[3];
>>> - int fscod, bsid, bsmod, acmod, lfeon, frmsizecod;
>>>
>>> - if (track->vos_len < 7) {
>>> + if (!info || !info->ec3_done) {
>>> av_log(s, AV_LOG_ERROR,
>>> "Cannot write moov atom before AC3 packets."
>>> " Set the delay_moov flag to fix this.\n");
>>> return AVERROR(EINVAL);
>>> }
>>>
>>> - avio_wb32(pb, 11);
>>> - ffio_wfourcc(pb, "dac3");
>>> + for (unsigned int i = 0; i < FF_ARRAY_ELEMS(ff_ac3_bitrate_tab); i++) {
>>> + if (info->data_rate == ff_ac3_bitrate_tab[i]) {
>>> + ac3_bit_rate_code = i;
>>
>> Anyway, I see that you use ff_ac3_bitrate_tab in lavf instead of adding
>> the frame_size_code to AC3HeaderInfo. Did it turn out to be easier this
>> way or what is the reason for this?
>
> I did partially go through the reasoning in the cover letter, but
> here's a short rewording:
>
> 1. This way doesn't extend the semi-public avpriv.
> 2. While reviewing I noticed that bsid 9, 10 streams (apparently some
> RealAudio fork of AC-3) actually have the sample/bit rate values bit
> shifted (divided by either 2 or 4). The table in the ETSI
> specification is based on the effective bit rate. While it is highly
> unlikely that someone would put that type of AC-3 into MP4, In that
> case I'd then have to re-index the value based on the effective bit
> rate instead of just taking the value from the bit stream in the
> parser. So as the effective bit rate is already available through the
> header parser, it just seems simpler to have the re-indexing happen on
> the using side in this case.
>
I have to admit not to get this point at all. Let's work through an
example: You have bsid 10 and you use ff_ac3_bitrate_tab[17] (i.e. 576).
AC3HeaderInfo.bit_rate will then contain 144000 (i.e. the value is
already divided by four). Your data_rate will be set to 144 based upon
this (if there is no higher datarate already encountered). But there is
no entry for 144 in ff_ac3_bitrate_tab, so that one will enter the "No
valid AC3 bit rate code for data rate" branch. Is this intended?
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
