* [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage
@ 2022-06-23 7:24 Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 1/7] avcodec: make AC-3 bit rate table available in a separate header Jan Ekström
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 7:24 UTC (permalink / raw)
To: ffmpeg-devel
The simplified parsing currently in `mov_write_ac3_tag` trusts the content
of the packets a bit too much (the AC-3 parser returns all data fed to it,
including any possible data before the start code), while the existing E-AC-3
logic does proper header validation by utilizing the (E-)AC-3 parser.
Thus, normalize on AC-3 parser usage for both AC-3 and E-AC-3.
Difference to v3:
* Limited bsids higher than 8 (RealAudio AC-3/DolbyNet) from getting muxed,
as it is unclear which bit_rate_code value should be utilized for them.
Jan
Jan Ekström (7):
avcodec: make AC-3 bit rate table available in a separate header
{configure,avformat/movenc}: enable AC-3 parser for movenc
avformat/movenc: enable handle_eac3 to handle AC-3 tracks
avformat/movenc: move eac3_info definition so that it can be used for
AC-3
avformat/movenc: utilize existing AC-3 parsing workflow for AC-3
avformat/movenc: handle OOM situations when parsing AC-3 headers
avformat/movenc: limit ISOBMFF AC-3 mapping to bsids <=8
configure | 2 +-
libavcodec/Makefile | 9 ++-
libavcodec/ac3_bitrate_tab.c | 22 ++++++
libavcodec/ac3_bitrate_tab.h | 33 +++++++++
libavcodec/ac3tab.c | 6 --
libavformat/Makefile | 1 +
libavformat/ac3_bitrate_tab.c | 22 ++++++
libavformat/movenc.c | 129 +++++++++++++++++++---------------
8 files changed, 157 insertions(+), 67 deletions(-)
create mode 100644 libavcodec/ac3_bitrate_tab.c
create mode 100644 libavcodec/ac3_bitrate_tab.h
create mode 100644 libavformat/ac3_bitrate_tab.c
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH v4 1/7] avcodec: make AC-3 bit rate table available in a separate header
2022-06-23 7:24 [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage Jan Ekström
@ 2022-06-23 7:24 ` Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 2/7] {configure, avformat/movenc}: enable AC-3 parser for movenc Jan Ekström
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 7:24 UTC (permalink / raw)
To: ffmpeg-devel
From: Jan Ekström <jan.ekstrom@24i.com>
This makes it possible to include it from libavformat
Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
libavcodec/Makefile | 8 +++++---
libavcodec/ac3_bitrate_tab.c | 22 ++++++++++++++++++++++
libavcodec/ac3_bitrate_tab.h | 33 +++++++++++++++++++++++++++++++++
libavcodec/ac3tab.c | 6 ------
4 files changed, 60 insertions(+), 9 deletions(-)
create mode 100644 libavcodec/ac3_bitrate_tab.c
create mode 100644 libavcodec/ac3_bitrate_tab.h
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 3b8f7b5e01..10697d31f7 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -190,8 +190,9 @@ OBJS-$(CONFIG_AC3_DECODER) += ac3dec_float.o ac3dec_data.o ac3.o \
OBJS-$(CONFIG_AC3_FIXED_DECODER) += ac3dec_fixed.o ac3dec_data.o ac3.o \
kbdwin.o ac3tab.o ac3_channel_layout_tab.o
OBJS-$(CONFIG_AC3_ENCODER) += ac3enc_float.o ac3enc.o ac3tab.o \
- ac3.o kbdwin.o
-OBJS-$(CONFIG_AC3_FIXED_ENCODER) += ac3enc_fixed.o ac3enc.o ac3tab.o ac3.o kbdwin.o
+ ac3.o kbdwin.o ac3_bitrate_tab.o
+OBJS-$(CONFIG_AC3_FIXED_ENCODER) += ac3enc_fixed.o ac3enc.o ac3tab.o \
+ ac3.o kbdwin.o ac3_bitrate_tab.o
OBJS-$(CONFIG_AC3_MF_ENCODER) += mfenc.o mf_utils.o
OBJS-$(CONFIG_ACELP_KELVIN_DECODER) += g729dec.o lsp.o celp_math.o celp_filters.o acelp_filters.o acelp_pitch_delay.o acelp_vectors.o g729postfilter.o
OBJS-$(CONFIG_AGM_DECODER) += agm.o
@@ -1110,7 +1111,8 @@ OBJS-$(CONFIG_LIBZVBI_TELETEXT_DECODER) += libzvbi-teletextdec.o ass.o
OBJS-$(CONFIG_AAC_LATM_PARSER) += latm_parser.o
OBJS-$(CONFIG_AAC_PARSER) += aac_parser.o aac_ac3_parser.o
OBJS-$(CONFIG_AC3_PARSER) += aac_ac3_parser.o ac3tab.o \
- ac3_channel_layout_tab.o
+ ac3_channel_layout_tab.o \
+ ac3_bitrate_tab.o
OBJS-$(CONFIG_ADX_PARSER) += adx_parser.o adx.o
OBJS-$(CONFIG_AMR_PARSER) += amr_parser.o
OBJS-$(CONFIG_AV1_PARSER) += av1_parser.o
diff --git a/libavcodec/ac3_bitrate_tab.c b/libavcodec/ac3_bitrate_tab.c
new file mode 100644
index 0000000000..9ae8794a4c
--- /dev/null
+++ b/libavcodec/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 "ac3_bitrate_tab.h"
diff --git a/libavcodec/ac3_bitrate_tab.h b/libavcodec/ac3_bitrate_tab.h
new file mode 100644
index 0000000000..44fc5f59ec
--- /dev/null
+++ b/libavcodec/ac3_bitrate_tab.h
@@ -0,0 +1,33 @@
+/*
+ * 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
+ */
+
+#ifndef AVCODEC_AC3_BITRATE_TAB_H
+#define AVCODEC_AC3_BITRATE_TAB_H
+
+#include <stdint.h>
+
+/* possible bitrates */
+const uint16_t ff_ac3_bitrate_tab[19] = {
+ 32, 40, 48, 56, 64, 80, 96, 112, 128,
+ 160, 192, 224, 256, 320, 384, 448, 512, 576, 640
+};
+
+#endif
diff --git a/libavcodec/ac3tab.c b/libavcodec/ac3tab.c
index 48c89a8ba0..75865fd7d2 100644
--- a/libavcodec/ac3tab.c
+++ b/libavcodec/ac3tab.c
@@ -94,12 +94,6 @@ const uint8_t ff_ac3_dec_channel_map[8][2][6] = {
/* possible frequencies */
const int ff_ac3_sample_rate_tab[] = { 48000, 44100, 32000, 0 };
-/* possible bitrates */
-const uint16_t ff_ac3_bitrate_tab[19] = {
- 32, 40, 48, 56, 64, 80, 96, 112, 128,
- 160, 192, 224, 256, 320, 384, 448, 512, 576, 640
-};
-
/**
* Table of bin locations for rematrixing bands
* reference: Section 7.5.2 Rematrixing : Frequency Band Definitions
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH v4 2/7] {configure, avformat/movenc}: enable AC-3 parser for movenc
2022-06-23 7:24 [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 1/7] avcodec: make AC-3 bit rate table available in a separate header Jan Ekström
@ 2022-06-23 7:24 ` Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 3/7] avformat/movenc: enable handle_eac3 to handle AC-3 tracks Jan Ekström
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 7:24 UTC (permalink / raw)
To: ffmpeg-devel
From: Jan Ekström <jan.ekstrom@24i.com>
This simplifies the code to no longer have #ifs in a manner which
does not require handling avpriv_ac3_parse_header returning ENOSYS.
As an existing example, the MPEG-TS muxer already requires the AC-3
parser, and in order to fix existing issues with the current AC-3
movenc code, switching to use the AC-3 parser is required, so this
is an enabling change for that.
Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
configure | 2 +-
libavformat/movenc.c | 4 ----
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/configure b/configure
index 3a97610209..5d991ae530 100755
--- a/configure
+++ b/configure
@@ -3441,7 +3441,7 @@ mlp_demuxer_select="mlp_parser"
mmf_muxer_select="riffenc"
mov_demuxer_select="iso_media riffdec"
mov_demuxer_suggest="zlib"
-mov_muxer_select="iso_media riffenc rtpenc_chain vp9_superframe_bsf aac_adtstoasc_bsf"
+mov_muxer_select="iso_media riffenc rtpenc_chain vp9_superframe_bsf aac_adtstoasc_bsf ac3_parser"
mp3_demuxer_select="mpegaudio_parser"
mp3_muxer_select="mpegaudioheader"
mp4_muxer_select="mov_muxer"
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index b7b2f46a17..87cd066c95 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -408,7 +408,6 @@ struct eac3_info {
} substream[1]; /* TODO: support 8 independent substreams */
};
-#if CONFIG_AC3_PARSER
static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
{
AC3HeaderInfo *hdr = NULL;
@@ -549,7 +548,6 @@ end:
return ret;
}
-#endif
static int mov_write_eac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
{
@@ -6046,7 +6044,6 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
}
}
-#if CONFIG_AC3_PARSER
} else if (par->codec_id == AV_CODEC_ID_EAC3) {
size = handle_eac3(mov, pkt, trk);
if (size < 0)
@@ -6054,7 +6051,6 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
else if (!size)
goto end;
avio_write(pb, pkt->data, size);
-#endif
} else if (par->codec_id == AV_CODEC_ID_EIA_608) {
size = 8;
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH v4 3/7] avformat/movenc: enable handle_eac3 to handle AC-3 tracks
2022-06-23 7:24 [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 1/7] avcodec: make AC-3 bit rate table available in a separate header Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 2/7] {configure, avformat/movenc}: enable AC-3 parser for movenc Jan Ekström
@ 2022-06-23 7:24 ` Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 4/7] avformat/movenc: move eac3_info definition so that it can be used for AC-3 Jan Ekström
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 7:24 UTC (permalink / raw)
To: ffmpeg-devel
From: Jan Ekström <jan.ekstrom@24i.com>
Add the AC-3 frame type, as well as early exit from additional packet
parsing in case of AC-3, as only a single packet is required to get
the required information.
Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
libavformat/movenc.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 87cd066c95..c24b11cf5f 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -443,7 +443,8 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
/* this should always be the case, given that our AC-3 parser
* concatenates dependent frames to their independent parent */
- if (hdr->frame_type == EAC3_FRAME_TYPE_INDEPENDENT) {
+ if (hdr->frame_type == EAC3_FRAME_TYPE_INDEPENDENT ||
+ hdr->frame_type == EAC3_FRAME_TYPE_AC3_CONVERT) {
/* substream ids must be incremental */
if (hdr->substreamid > info->num_ind_sub + 1) {
ret = AVERROR(EINVAL);
@@ -475,6 +476,14 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
info->substream[hdr->substreamid].acmod = hdr->channel_mode;
info->substream[hdr->substreamid].lfeon = hdr->lfe_on;
+ if (track->par->codec_id == AV_CODEC_ID_AC3) {
+ // with AC-3 we only require the information of a single packet,
+ // so we can finish as soon as the basic values of the bit stream
+ // have been set to the track's informational structure.
+ info->ec3_done = 1;
+ goto concatenate;
+ }
+
/* Parse dependent substream(s), if any */
if (pkt->size != hdr->frame_size) {
int cumul_size = hdr->frame_size;
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH v4 4/7] avformat/movenc: move eac3_info definition so that it can be used for AC-3
2022-06-23 7:24 [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage Jan Ekström
` (2 preceding siblings ...)
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 3/7] avformat/movenc: enable handle_eac3 to handle AC-3 tracks Jan Ekström
@ 2022-06-23 7:24 ` Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow " Jan Ekström
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 7:24 UTC (permalink / raw)
To: ffmpeg-devel
From: Jan Ekström <jan.ekstrom@24i.com>
Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
libavformat/movenc.c | 64 ++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index c24b11cf5f..103f958d75 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -328,6 +328,38 @@ static int mov_write_amr_tag(AVIOContext *pb, MOVTrack *track)
return 0x11;
}
+struct eac3_info {
+ AVPacket *pkt;
+ uint8_t ec3_done;
+ uint8_t num_blocks;
+
+ /* Layout of the EC3SpecificBox */
+ /* maximum bitrate */
+ uint16_t data_rate;
+ /* number of independent substreams */
+ uint8_t num_ind_sub;
+ struct {
+ /* sample rate code (see ff_ac3_sample_rate_tab) 2 bits */
+ uint8_t fscod;
+ /* bit stream identification 5 bits */
+ uint8_t bsid;
+ /* one bit reserved */
+ /* audio service mixing (not supported yet) 1 bit */
+ /* bit stream mode 3 bits */
+ uint8_t bsmod;
+ /* audio coding mode 3 bits */
+ uint8_t acmod;
+ /* sub woofer on 1 bit */
+ uint8_t lfeon;
+ /* 3 bits reserved */
+ /* number of dependent substreams associated with this substream 4 bits */
+ uint8_t num_dep_sub;
+ /* channel locations of the dependent substream(s), if any, 9 bits */
+ uint16_t chan_loc;
+ /* if there is no dependent substream, then one bit reserved instead */
+ } substream[1]; /* TODO: support 8 independent substreams */
+};
+
static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track)
{
GetBitContext gbc;
@@ -376,38 +408,6 @@ static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *trac
return 11;
}
-struct eac3_info {
- AVPacket *pkt;
- uint8_t ec3_done;
- uint8_t num_blocks;
-
- /* Layout of the EC3SpecificBox */
- /* maximum bitrate */
- uint16_t data_rate;
- /* number of independent substreams */
- uint8_t num_ind_sub;
- struct {
- /* sample rate code (see ff_ac3_sample_rate_tab) 2 bits */
- uint8_t fscod;
- /* bit stream identification 5 bits */
- uint8_t bsid;
- /* one bit reserved */
- /* audio service mixing (not supported yet) 1 bit */
- /* bit stream mode 3 bits */
- uint8_t bsmod;
- /* audio coding mode 3 bits */
- uint8_t acmod;
- /* sub woofer on 1 bit */
- uint8_t lfeon;
- /* 3 bits reserved */
- /* number of dependent substreams associated with this substream 4 bits */
- uint8_t num_dep_sub;
- /* channel locations of the dependent substream(s), if any, 9 bits */
- uint16_t chan_loc;
- /* if there is no dependent substream, then one bit reserved instead */
- } substream[1]; /* TODO: support 8 independent substreams */
-};
-
static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
{
AC3HeaderInfo *hdr = NULL;
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow for AC-3
2022-06-23 7:24 [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage Jan Ekström
` (3 preceding siblings ...)
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 4/7] avformat/movenc: move eac3_info definition so that it can be used for AC-3 Jan Ekström
@ 2022-06-23 7:24 ` Jan Ekström
2022-06-23 8:56 ` Andreas Rheinhardt
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 6/7] avformat/movenc: handle OOM situations when parsing AC-3 headers Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 7/7] avformat/movenc: limit ISOBMFF AC-3 mapping to bsids <=8 Jan Ekström
6 siblings, 1 reply; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 7:24 UTC (permalink / raw)
To: ffmpeg-devel
From: Jan Ekström <jan.ekstrom@24i.com>
Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
libavcodec/Makefile | 1 +
libavformat/Makefile | 1 +
libavformat/ac3_bitrate_tab.c | 22 ++++++++++++++
libavformat/movenc.c | 55 +++++++++++++++++------------------
4 files changed, 51 insertions(+), 28 deletions(-)
create mode 100644 libavformat/ac3_bitrate_tab.c
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 10697d31f7..7e0d278e3d 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1018,6 +1018,7 @@ STLIBOBJS-$(CONFIG_FLV_MUXER) += mpeg4audio_sample_rates.o
STLIBOBJS-$(CONFIG_HLS_DEMUXER) += ac3_channel_layout_tab.o
STLIBOBJS-$(CONFIG_MATROSKA_DEMUXER) += mpeg4audio_sample_rates.o
STLIBOBJS-$(CONFIG_MOV_DEMUXER) += ac3_channel_layout_tab.o
+STLIBOBJS-$(CONFIG_MOV_MUXER) += ac3_bitrate_tab.o
STLIBOBJS-$(CONFIG_MXF_MUXER) += golomb.o
STLIBOBJS-$(CONFIG_MP3_MUXER) += mpegaudiotabs.o
STLIBOBJS-$(CONFIG_NUT_MUXER) += mpegaudiotabs.o
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
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;
+ break;
+ }
+ }
- init_get_bits(&gbc, track->vos_data + 4, (track->vos_len - 4) * 8);
- fscod = get_bits(&gbc, 2);
- frmsizecod = get_bits(&gbc, 6);
- bsid = get_bits(&gbc, 5);
- bsmod = get_bits(&gbc, 3);
- acmod = get_bits(&gbc, 3);
- if (acmod == 2) {
- skip_bits(&gbc, 2); // dsurmod
- } else {
- if ((acmod & 1) && acmod != 1)
- skip_bits(&gbc, 2); // cmixlev
- if (acmod & 4)
- skip_bits(&gbc, 2); // surmixlev
+ if (ac3_bit_rate_code < 0) {
+ av_log(s, AV_LOG_ERROR,
+ "No valid AC3 bit rate code for data rate of %d!\n",
+ info->data_rate);
+ return AVERROR(EINVAL);
}
- lfeon = get_bits1(&gbc);
+
+ avio_wb32(pb, 11);
+ ffio_wfourcc(pb, "dac3");
init_put_bits(&pbc, buf, sizeof(buf));
- put_bits(&pbc, 2, fscod);
- put_bits(&pbc, 5, bsid);
- put_bits(&pbc, 3, bsmod);
- put_bits(&pbc, 3, acmod);
- put_bits(&pbc, 1, lfeon);
- put_bits(&pbc, 5, frmsizecod >> 1); // bit_rate_code
+ put_bits(&pbc, 2, info->substream[0].fscod);
+ put_bits(&pbc, 5, info->substream[0].bsid);
+ put_bits(&pbc, 3, info->substream[0].bsmod);
+ put_bits(&pbc, 3, info->substream[0].acmod);
+ put_bits(&pbc, 1, info->substream[0].lfeon);
+ put_bits(&pbc, 5, ac3_bit_rate_code); // bit_rate_code
put_bits(&pbc, 5, 0); // reserved
flush_put_bits(&pbc);
@@ -5975,8 +5974,7 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
if ((par->codec_id == AV_CODEC_ID_DNXHD ||
par->codec_id == AV_CODEC_ID_H264 ||
par->codec_id == AV_CODEC_ID_HEVC ||
- par->codec_id == AV_CODEC_ID_TRUEHD ||
- par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len &&
+ par->codec_id == AV_CODEC_ID_TRUEHD) && !trk->vos_len &&
!TAG_IS_AVCI(trk->tag)) {
/* copy frame to create needed atoms */
trk->vos_len = size;
@@ -6053,7 +6051,8 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
}
}
- } else if (par->codec_id == AV_CODEC_ID_EAC3) {
+ } else if (par->codec_id == AV_CODEC_ID_EAC3 ||
+ par->codec_id == AV_CODEC_ID_AC3) {
size = handle_eac3(mov, pkt, trk);
if (size < 0)
return size;
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH v4 6/7] avformat/movenc: handle OOM situations when parsing AC-3 headers
2022-06-23 7:24 [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage Jan Ekström
` (4 preceding siblings ...)
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow " Jan Ekström
@ 2022-06-23 7:24 ` Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 7/7] avformat/movenc: limit ISOBMFF AC-3 mapping to bsids <=8 Jan Ekström
6 siblings, 0 replies; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 7:24 UTC (permalink / raw)
To: ffmpeg-devel
From: Jan Ekström <jan.ekstrom@24i.com>
Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
libavformat/movenc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index a071f1cdd5..58ba0bc545 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -420,7 +420,10 @@ static int handle_eac3(MOVMuxContext *mov, AVPacket *pkt, MOVTrack *track)
if (!info->pkt && !(info->pkt = av_packet_alloc()))
return AVERROR(ENOMEM);
- if (avpriv_ac3_parse_header(&hdr, pkt->data, pkt->size) < 0) {
+ if ((ret = avpriv_ac3_parse_header(&hdr, pkt->data, pkt->size) < 0)) {
+ if (ret == AVERROR(ENOMEM))
+ goto end;
+
/* drop the packets until we see a good one */
if (!track->entry) {
av_log(mov->fc, AV_LOG_WARNING, "Dropping invalid packet from start of the stream\n");
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH v4 7/7] avformat/movenc: limit ISOBMFF AC-3 mapping to bsids <=8
2022-06-23 7:24 [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage Jan Ekström
` (5 preceding siblings ...)
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 6/7] avformat/movenc: handle OOM situations when parsing AC-3 headers Jan Ekström
@ 2022-06-23 7:24 ` Jan Ekström
6 siblings, 0 replies; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 7:24 UTC (permalink / raw)
To: ffmpeg-devel
From: Jan Ekström <jan.ekstrom@24i.com>
This leaves out RealAudio DolbyNet, which utilizes bsids 9 and 10,
It is not clear whether the interpreted bit rate value (divided by
2 or 4 depending on the variant), or the original bit rate value
should be utilized to receive the bit_rate_code index.
Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
---
libavformat/movenc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 58ba0bc545..a3e108c5af 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -375,6 +375,14 @@ static int mov_write_ac3_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *trac
return AVERROR(EINVAL);
}
+ if (info->substream[0].bsid > 8) {
+ av_log(s, AV_LOG_ERROR,
+ "RealAudio AC-3/DolbyNet with bsid %d is not defined by the "
+ "ISOBMFF specification in ETSI TS 102 366!\n",
+ info->substream[0].bsid);
+ return AVERROR(EINVAL);
+ }
+
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;
--
2.36.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow for AC-3
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow " Jan Ekström
@ 2022-06-23 8:56 ` Andreas Rheinhardt
2022-06-23 10:11 ` Jan Ekström
0 siblings, 1 reply; 13+ messages in thread
From: Andreas Rheinhardt @ 2022-06-23 8:56 UTC (permalink / raw)
To: ffmpeg-devel
Jan Ekström:
> From: Jan Ekström <jan.ekstrom@24i.com>
>
> Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
> ---
> libavcodec/Makefile | 1 +
> libavformat/Makefile | 1 +
> libavformat/ac3_bitrate_tab.c | 22 ++++++++++++++
> libavformat/movenc.c | 55 +++++++++++++++++------------------
> 4 files changed, 51 insertions(+), 28 deletions(-)
> create mode 100644 libavformat/ac3_bitrate_tab.c
>
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 10697d31f7..7e0d278e3d 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1018,6 +1018,7 @@ STLIBOBJS-$(CONFIG_FLV_MUXER) += mpeg4audio_sample_rates.o
> STLIBOBJS-$(CONFIG_HLS_DEMUXER) += ac3_channel_layout_tab.o
> STLIBOBJS-$(CONFIG_MATROSKA_DEMUXER) += mpeg4audio_sample_rates.o
> STLIBOBJS-$(CONFIG_MOV_DEMUXER) += ac3_channel_layout_tab.o
> +STLIBOBJS-$(CONFIG_MOV_MUXER) += ac3_bitrate_tab.o
> STLIBOBJS-$(CONFIG_MXF_MUXER) += golomb.o
> STLIBOBJS-$(CONFIG_MP3_MUXER) += mpegaudiotabs.o
> STLIBOBJS-$(CONFIG_NUT_MUXER) += mpegaudiotabs.o
> 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
> 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;
> + break;
> + }
> + }
Why don't you just export the bit rate code instead of rederiving it?
This should be easy now that you intend to disallow AC-3 frames with
bsid > 8.
>
> - init_get_bits(&gbc, track->vos_data + 4, (track->vos_len - 4) * 8);
> - fscod = get_bits(&gbc, 2);
> - frmsizecod = get_bits(&gbc, 6);
> - bsid = get_bits(&gbc, 5);
> - bsmod = get_bits(&gbc, 3);
> - acmod = get_bits(&gbc, 3);
> - if (acmod == 2) {
> - skip_bits(&gbc, 2); // dsurmod
> - } else {
> - if ((acmod & 1) && acmod != 1)
> - skip_bits(&gbc, 2); // cmixlev
> - if (acmod & 4)
> - skip_bits(&gbc, 2); // surmixlev
> + if (ac3_bit_rate_code < 0) {
> + av_log(s, AV_LOG_ERROR,
> + "No valid AC3 bit rate code for data rate of %d!\n",
> + info->data_rate);
> + return AVERROR(EINVAL);
> }
> - lfeon = get_bits1(&gbc);
> +
> + avio_wb32(pb, 11);
> + ffio_wfourcc(pb, "dac3");
>
> init_put_bits(&pbc, buf, sizeof(buf));
> - put_bits(&pbc, 2, fscod);
> - put_bits(&pbc, 5, bsid);
> - put_bits(&pbc, 3, bsmod);
> - put_bits(&pbc, 3, acmod);
> - put_bits(&pbc, 1, lfeon);
> - put_bits(&pbc, 5, frmsizecod >> 1); // bit_rate_code
> + put_bits(&pbc, 2, info->substream[0].fscod);
> + put_bits(&pbc, 5, info->substream[0].bsid);
> + put_bits(&pbc, 3, info->substream[0].bsmod);
> + put_bits(&pbc, 3, info->substream[0].acmod);
> + put_bits(&pbc, 1, info->substream[0].lfeon);
> + put_bits(&pbc, 5, ac3_bit_rate_code); // bit_rate_code
> put_bits(&pbc, 5, 0); // reserved
>
> flush_put_bits(&pbc);
> @@ -5975,8 +5974,7 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
> if ((par->codec_id == AV_CODEC_ID_DNXHD ||
> par->codec_id == AV_CODEC_ID_H264 ||
> par->codec_id == AV_CODEC_ID_HEVC ||
> - par->codec_id == AV_CODEC_ID_TRUEHD ||
> - par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len &&
> + par->codec_id == AV_CODEC_ID_TRUEHD) && !trk->vos_len &&
> !TAG_IS_AVCI(trk->tag)) {
> /* copy frame to create needed atoms */
> trk->vos_len = size;
> @@ -6053,7 +6051,8 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
> }
> }
>
> - } else if (par->codec_id == AV_CODEC_ID_EAC3) {
> + } else if (par->codec_id == AV_CODEC_ID_EAC3 ||
> + par->codec_id == AV_CODEC_ID_AC3) {
> size = handle_eac3(mov, pkt, trk);
> if (size < 0)
> return size;
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow for AC-3
2022-06-23 8:56 ` Andreas Rheinhardt
@ 2022-06-23 10:11 ` Jan Ekström
2022-06-27 7:12 ` Jan Ekström
0 siblings, 1 reply; 13+ messages in thread
From: Jan Ekström @ 2022-06-23 10:11 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Thu, Jun 23, 2022 at 11:57 AM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Jan Ekström:
> > From: Jan Ekström <jan.ekstrom@24i.com>
> >
> > Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
> > ---
> > libavcodec/Makefile | 1 +
> > libavformat/Makefile | 1 +
> > libavformat/ac3_bitrate_tab.c | 22 ++++++++++++++
> > libavformat/movenc.c | 55 +++++++++++++++++------------------
> > 4 files changed, 51 insertions(+), 28 deletions(-)
> > create mode 100644 libavformat/ac3_bitrate_tab.c
> >
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index 10697d31f7..7e0d278e3d 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -1018,6 +1018,7 @@ STLIBOBJS-$(CONFIG_FLV_MUXER) += mpeg4audio_sample_rates.o
> > STLIBOBJS-$(CONFIG_HLS_DEMUXER) += ac3_channel_layout_tab.o
> > STLIBOBJS-$(CONFIG_MATROSKA_DEMUXER) += mpeg4audio_sample_rates.o
> > STLIBOBJS-$(CONFIG_MOV_DEMUXER) += ac3_channel_layout_tab.o
> > +STLIBOBJS-$(CONFIG_MOV_MUXER) += ac3_bitrate_tab.o
> > STLIBOBJS-$(CONFIG_MXF_MUXER) += golomb.o
> > STLIBOBJS-$(CONFIG_MP3_MUXER) += mpegaudiotabs.o
> > STLIBOBJS-$(CONFIG_NUT_MUXER) += mpegaudiotabs.o
> > 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
> > 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;
> > + break;
> > + }
> > + }
>
> Why don't you just export the bit rate code instead of rederiving it?
> This should be easy now that you intend to disallow AC-3 frames with
> bsid > 8.
>
Other reasons for why I originally switched to this method are:
1. The first comment I got on IRC from Anton when requesting for
comments on the set was "why don't you make it (the header parsing)
properly public?". My intent is to fix a bug in writing the AC-3 box
with inputs where the read data does not start with an AC-3 start code
(such as MPEG-TS streams that for some reason decide to mux AC-3 in a
manner where PES packets do not start with the start of a packet), not
make such decisions/changes.
2. The value itself is of limited value to other uses of header
parsing. E-AC-3 for example just utilizes the interpreted value.
3. No requirement to think about additional library mismatches since
both libraries will have their own table in these versions. Not like
we support such mismatches, but not coming up with new spots where it
is possible did seem favorable.
In other words, not extending the existing avpriv parsing seemed like
a Good Idea if you attempt to optimize for "less possibilities for
ad-hoc requests", and "possibly safer". My plan was to get initial fix
on this side done and merged, and then move onto improving the parser
framework to flag packets that clearly are not proper in some manner -
as currently it seems like the API user has no way of knowing whether
the buffer they received from a parser is an actual usable/"valid
looking" packet or not, as apparently everything is supposed to be
returned that the API user pushes in, just split into blocks.
Anyways, if you clearly prefer the extension of the avpriv stuff, then
please check if the original v1 plus the bsid > 8 limiting commit from
v4 are acceptable to you. If yes, I can then apply that mix - for me
at this point the main thing is to be able to move from getting this
fix on the muxer side upstreamed, and then to the next thing.
Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow for AC-3
2022-06-23 10:11 ` Jan Ekström
@ 2022-06-27 7:12 ` Jan Ekström
2022-06-29 6:12 ` Jan Ekström
0 siblings, 1 reply; 13+ messages in thread
From: Jan Ekström @ 2022-06-27 7:12 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Thu, Jun 23, 2022 at 1:11 PM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Thu, Jun 23, 2022 at 11:57 AM Andreas Rheinhardt
> <andreas.rheinhardt@outlook.com> wrote:
> >
> > Jan Ekström:
> > > From: Jan Ekström <jan.ekstrom@24i.com>
> > >
> > > Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
> > > ---
> > > libavcodec/Makefile | 1 +
> > > libavformat/Makefile | 1 +
> > > libavformat/ac3_bitrate_tab.c | 22 ++++++++++++++
> > > libavformat/movenc.c | 55 +++++++++++++++++------------------
> > > 4 files changed, 51 insertions(+), 28 deletions(-)
> > > create mode 100644 libavformat/ac3_bitrate_tab.c
> > >
> > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > > index 10697d31f7..7e0d278e3d 100644
> > > --- a/libavcodec/Makefile
> > > +++ b/libavcodec/Makefile
> > > @@ -1018,6 +1018,7 @@ STLIBOBJS-$(CONFIG_FLV_MUXER) += mpeg4audio_sample_rates.o
> > > STLIBOBJS-$(CONFIG_HLS_DEMUXER) += ac3_channel_layout_tab.o
> > > STLIBOBJS-$(CONFIG_MATROSKA_DEMUXER) += mpeg4audio_sample_rates.o
> > > STLIBOBJS-$(CONFIG_MOV_DEMUXER) += ac3_channel_layout_tab.o
> > > +STLIBOBJS-$(CONFIG_MOV_MUXER) += ac3_bitrate_tab.o
> > > STLIBOBJS-$(CONFIG_MXF_MUXER) += golomb.o
> > > STLIBOBJS-$(CONFIG_MP3_MUXER) += mpegaudiotabs.o
> > > STLIBOBJS-$(CONFIG_NUT_MUXER) += mpegaudiotabs.o
> > > 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
> > > 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;
> > > + break;
> > > + }
> > > + }
> >
> > Why don't you just export the bit rate code instead of rederiving it?
> > This should be easy now that you intend to disallow AC-3 frames with
> > bsid > 8.
> >
>
> Other reasons for why I originally switched to this method are:
>
> 1. The first comment I got on IRC from Anton when requesting for
> comments on the set was "why don't you make it (the header parsing)
> properly public?". My intent is to fix a bug in writing the AC-3 box
> with inputs where the read data does not start with an AC-3 start code
> (such as MPEG-TS streams that for some reason decide to mux AC-3 in a
> manner where PES packets do not start with the start of a packet), not
> make such decisions/changes.
> 2. The value itself is of limited value to other uses of header
> parsing. E-AC-3 for example just utilizes the interpreted value.
> 3. No requirement to think about additional library mismatches since
> both libraries will have their own table in these versions. Not like
> we support such mismatches, but not coming up with new spots where it
> is possible did seem favorable.
>
> In other words, not extending the existing avpriv parsing seemed like
> a Good Idea if you attempt to optimize for "less possibilities for
> ad-hoc requests", and "possibly safer". My plan was to get initial fix
> on this side done and merged, and then move onto improving the parser
> framework to flag packets that clearly are not proper in some manner -
> as currently it seems like the API user has no way of knowing whether
> the buffer they received from a parser is an actual usable/"valid
> looking" packet or not, as apparently everything is supposed to be
> returned that the API user pushes in, just split into blocks.
>
> Anyways, if you clearly prefer the extension of the avpriv stuff, then
> please check if the original v1 plus the bsid > 8 limiting commit from
> v4 are acceptable to you. If yes, I can then apply that mix - for me
> at this point the main thing is to be able to move from getting this
> fix on the muxer side upstreamed, and then to the next thing.
To make it more concrete, this is how it would look.
I rebased v1 of this patch set and cherry-picked the bsid limiting
patch on top: https://github.com/jeeb/ffmpeg/commits/movenc_use_ac3_parser_for_ac3_v5
Is this what you would consider acceptable? Of course the avpriv
extension commit would get a minor version bump added to it.
Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow for AC-3
2022-06-27 7:12 ` Jan Ekström
@ 2022-06-29 6:12 ` Jan Ekström
2022-06-29 7:41 ` Andreas Rheinhardt
0 siblings, 1 reply; 13+ messages in thread
From: Jan Ekström @ 2022-06-29 6:12 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Mon, Jun 27, 2022 at 10:12 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Thu, Jun 23, 2022 at 1:11 PM Jan Ekström <jeebjp@gmail.com> wrote:
> >
> > On Thu, Jun 23, 2022 at 11:57 AM Andreas Rheinhardt
> > <andreas.rheinhardt@outlook.com> wrote:
> > >
> > > Jan Ekström:
> > > > From: Jan Ekström <jan.ekstrom@24i.com>
> > > >
> > > > Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
> > > > ---
> > > > libavcodec/Makefile | 1 +
> > > > libavformat/Makefile | 1 +
> > > > libavformat/ac3_bitrate_tab.c | 22 ++++++++++++++
> > > > libavformat/movenc.c | 55 +++++++++++++++++------------------
> > > > 4 files changed, 51 insertions(+), 28 deletions(-)
> > > > create mode 100644 libavformat/ac3_bitrate_tab.c
> > > >
> > > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > > > index 10697d31f7..7e0d278e3d 100644
> > > > --- a/libavcodec/Makefile
> > > > +++ b/libavcodec/Makefile
> > > > @@ -1018,6 +1018,7 @@ STLIBOBJS-$(CONFIG_FLV_MUXER) += mpeg4audio_sample_rates.o
> > > > STLIBOBJS-$(CONFIG_HLS_DEMUXER) += ac3_channel_layout_tab.o
> > > > STLIBOBJS-$(CONFIG_MATROSKA_DEMUXER) += mpeg4audio_sample_rates.o
> > > > STLIBOBJS-$(CONFIG_MOV_DEMUXER) += ac3_channel_layout_tab.o
> > > > +STLIBOBJS-$(CONFIG_MOV_MUXER) += ac3_bitrate_tab.o
> > > > STLIBOBJS-$(CONFIG_MXF_MUXER) += golomb.o
> > > > STLIBOBJS-$(CONFIG_MP3_MUXER) += mpegaudiotabs.o
> > > > STLIBOBJS-$(CONFIG_NUT_MUXER) += mpegaudiotabs.o
> > > > 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
> > > > 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;
> > > > + break;
> > > > + }
> > > > + }
> > >
> > > Why don't you just export the bit rate code instead of rederiving it?
> > > This should be easy now that you intend to disallow AC-3 frames with
> > > bsid > 8.
> > >
> >
> > Other reasons for why I originally switched to this method are:
> >
> > 1. The first comment I got on IRC from Anton when requesting for
> > comments on the set was "why don't you make it (the header parsing)
> > properly public?". My intent is to fix a bug in writing the AC-3 box
> > with inputs where the read data does not start with an AC-3 start code
> > (such as MPEG-TS streams that for some reason decide to mux AC-3 in a
> > manner where PES packets do not start with the start of a packet), not
> > make such decisions/changes.
> > 2. The value itself is of limited value to other uses of header
> > parsing. E-AC-3 for example just utilizes the interpreted value.
> > 3. No requirement to think about additional library mismatches since
> > both libraries will have their own table in these versions. Not like
> > we support such mismatches, but not coming up with new spots where it
> > is possible did seem favorable.
> >
> > In other words, not extending the existing avpriv parsing seemed like
> > a Good Idea if you attempt to optimize for "less possibilities for
> > ad-hoc requests", and "possibly safer". My plan was to get initial fix
> > on this side done and merged, and then move onto improving the parser
> > framework to flag packets that clearly are not proper in some manner -
> > as currently it seems like the API user has no way of knowing whether
> > the buffer they received from a parser is an actual usable/"valid
> > looking" packet or not, as apparently everything is supposed to be
> > returned that the API user pushes in, just split into blocks.
> >
> > Anyways, if you clearly prefer the extension of the avpriv stuff, then
> > please check if the original v1 plus the bsid > 8 limiting commit from
> > v4 are acceptable to you. If yes, I can then apply that mix - for me
> > at this point the main thing is to be able to move from getting this
> > fix on the muxer side upstreamed, and then to the next thing.
>
> To make it more concrete, this is how it would look.
>
> I rebased v1 of this patch set and cherry-picked the bsid limiting
> patch on top: https://github.com/jeeb/ffmpeg/commits/movenc_use_ac3_parser_for_ac3_v5
>
> Is this what you would consider acceptable? Of course the avpriv
> extension commit would get a minor version bump added to it.
>
A friendly ping. I would like to know whether this version (v4) is
good enough, or if I should post the branch I linked as v5 (which goes
back to v1, plus the bsid limitation) to allow this patch set to
proceed.
Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow for AC-3
2022-06-29 6:12 ` Jan Ekström
@ 2022-06-29 7:41 ` Andreas Rheinhardt
0 siblings, 0 replies; 13+ messages in thread
From: Andreas Rheinhardt @ 2022-06-29 7:41 UTC (permalink / raw)
To: ffmpeg-devel
Jan Ekström:
> On Mon, Jun 27, 2022 at 10:12 AM Jan Ekström <jeebjp@gmail.com> wrote:
>>
>> On Thu, Jun 23, 2022 at 1:11 PM Jan Ekström <jeebjp@gmail.com> wrote:
>>>
>>> On Thu, Jun 23, 2022 at 11:57 AM Andreas Rheinhardt
>>> <andreas.rheinhardt@outlook.com> wrote:
>>>>
>>>> Jan Ekström:
>>>>> From: Jan Ekström <jan.ekstrom@24i.com>
>>>>>
>>>>> Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
>>>>> ---
>>>>> libavcodec/Makefile | 1 +
>>>>> libavformat/Makefile | 1 +
>>>>> libavformat/ac3_bitrate_tab.c | 22 ++++++++++++++
>>>>> libavformat/movenc.c | 55 +++++++++++++++++------------------
>>>>> 4 files changed, 51 insertions(+), 28 deletions(-)
>>>>> create mode 100644 libavformat/ac3_bitrate_tab.c
>>>>>
>>>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>>>> index 10697d31f7..7e0d278e3d 100644
>>>>> --- a/libavcodec/Makefile
>>>>> +++ b/libavcodec/Makefile
>>>>> @@ -1018,6 +1018,7 @@ STLIBOBJS-$(CONFIG_FLV_MUXER) += mpeg4audio_sample_rates.o
>>>>> STLIBOBJS-$(CONFIG_HLS_DEMUXER) += ac3_channel_layout_tab.o
>>>>> STLIBOBJS-$(CONFIG_MATROSKA_DEMUXER) += mpeg4audio_sample_rates.o
>>>>> STLIBOBJS-$(CONFIG_MOV_DEMUXER) += ac3_channel_layout_tab.o
>>>>> +STLIBOBJS-$(CONFIG_MOV_MUXER) += ac3_bitrate_tab.o
>>>>> STLIBOBJS-$(CONFIG_MXF_MUXER) += golomb.o
>>>>> STLIBOBJS-$(CONFIG_MP3_MUXER) += mpegaudiotabs.o
>>>>> STLIBOBJS-$(CONFIG_NUT_MUXER) += mpegaudiotabs.o
>>>>> 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
>>>>> 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;
>>>>> + break;
>>>>> + }
>>>>> + }
>>>>
>>>> Why don't you just export the bit rate code instead of rederiving it?
>>>> This should be easy now that you intend to disallow AC-3 frames with
>>>> bsid > 8.
>>>>
>>>
>>> Other reasons for why I originally switched to this method are:
>>>
>>> 1. The first comment I got on IRC from Anton when requesting for
>>> comments on the set was "why don't you make it (the header parsing)
>>> properly public?". My intent is to fix a bug in writing the AC-3 box
>>> with inputs where the read data does not start with an AC-3 start code
>>> (such as MPEG-TS streams that for some reason decide to mux AC-3 in a
>>> manner where PES packets do not start with the start of a packet), not
>>> make such decisions/changes.
>>> 2. The value itself is of limited value to other uses of header
>>> parsing. E-AC-3 for example just utilizes the interpreted value.
>>> 3. No requirement to think about additional library mismatches since
>>> both libraries will have their own table in these versions. Not like
>>> we support such mismatches, but not coming up with new spots where it
>>> is possible did seem favorable.
>>>
>>> In other words, not extending the existing avpriv parsing seemed like
>>> a Good Idea if you attempt to optimize for "less possibilities for
>>> ad-hoc requests", and "possibly safer". My plan was to get initial fix
>>> on this side done and merged, and then move onto improving the parser
>>> framework to flag packets that clearly are not proper in some manner -
>>> as currently it seems like the API user has no way of knowing whether
>>> the buffer they received from a parser is an actual usable/"valid
>>> looking" packet or not, as apparently everything is supposed to be
>>> returned that the API user pushes in, just split into blocks.
>>>
>>> Anyways, if you clearly prefer the extension of the avpriv stuff, then
>>> please check if the original v1 plus the bsid > 8 limiting commit from
>>> v4 are acceptable to you. If yes, I can then apply that mix - for me
>>> at this point the main thing is to be able to move from getting this
>>> fix on the muxer side upstreamed, and then to the next thing.
>>
>> To make it more concrete, this is how it would look.
>>
>> I rebased v1 of this patch set and cherry-picked the bsid limiting
>> patch on top: https://github.com/jeeb/ffmpeg/commits/movenc_use_ac3_parser_for_ac3_v5
>>
>> Is this what you would consider acceptable? Of course the avpriv
>> extension commit would get a minor version bump added to it.
>>
>
> A friendly ping. I would like to know whether this version (v4) is
> good enough, or if I should post the branch I linked as v5 (which goes
> back to v1, plus the bsid limitation) to allow this patch set to
> proceed.
>
I prefer v5, but you obviously prefer v4. But I don't want to force my
preferences upon you, so I won't block you from applying v4.
- 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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-06-29 7:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-23 7:24 [FFmpeg-devel] [PATCH v4 0/7] avformat/movenc: normalize on AC-3 parser usage Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 1/7] avcodec: make AC-3 bit rate table available in a separate header Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 2/7] {configure, avformat/movenc}: enable AC-3 parser for movenc Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 3/7] avformat/movenc: enable handle_eac3 to handle AC-3 tracks Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 4/7] avformat/movenc: move eac3_info definition so that it can be used for AC-3 Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 5/7] avformat/movenc: utilize existing AC-3 parsing workflow " Jan Ekström
2022-06-23 8:56 ` Andreas Rheinhardt
2022-06-23 10:11 ` Jan Ekström
2022-06-27 7:12 ` Jan Ekström
2022-06-29 6:12 ` Jan Ekström
2022-06-29 7:41 ` Andreas Rheinhardt
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 6/7] avformat/movenc: handle OOM situations when parsing AC-3 headers Jan Ekström
2022-06-23 7:24 ` [FFmpeg-devel] [PATCH v4 7/7] avformat/movenc: limit ISOBMFF AC-3 mapping to bsids <=8 Jan Ekström
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git