[FFmpeg-devel] [PATCH 2/5] avformat/mxfenc: simplify the code and use mode for OP1a/D10/OPAtom muxer
lance.lmwang at gmail.com
lance.lmwang at gmail.com
Thu Jan 7 03:27:12 EET 2021
On Wed, Jan 06, 2021 at 06:14:07PM +0100, Marton Balint wrote:
>
>
> On Wed, 6 Jan 2021, Andreas Rheinhardt wrote:
>
> > lance.lmwang at gmail.com:
> > > From: Limin Wang <lance.lmwang at gmail.com>
> > >
> > > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > > ---
> > > libavformat/mxfenc.c | 73 ++++++++++++++++++++++++++++++----------------------
> > > 1 file changed, 42 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > > index d8678c9..7fce7b9 100644
> > > --- a/libavformat/mxfenc.c
> > > +++ b/libavformat/mxfenc.c
> > > @@ -60,12 +60,13 @@
> > > #include "mxf.h"
> > > #include "config.h"
> > >
> > > -extern AVOutputFormat ff_mxf_d10_muxer;
> > > -extern AVOutputFormat ff_mxf_opatom_muxer;
> >
> > Comparing the pointers is actually a better (cheaper) way than comparing
> > the name for determining the mode.
>
> Yes, this patch does not look simpler in any way to me, I'd rather just keep
> things as is.
OK, please ignore the patch.
>
> Thanks,
> Marton
>
> >
> > > -
> > > #define EDIT_UNITS_PER_BODY 250
> > > #define KAG_SIZE 512
> > >
> > > +#define MODE_OP1a 0x01
> > > +#define MODE_D10 0x02
> > > +#define MODE_OPA 0x04
> >
> > You are treating this as if it were a bitfield when it is not. It is
> > just an enumeration and should be an enum.
> >
> > > +
> > > typedef struct MXFIndexEntry {
> > > uint64_t offset;
> > > unsigned slice_offset; ///< offset of audio slice
> > > @@ -258,6 +259,7 @@ typedef struct MXFContext {
> > > int store_user_comments;
> > > int track_instance_count; // used to generate MXFTrack uuids
> > > int cbr_index; ///< use a constant bitrate index
> > > + int mode;
> >
> > You do not need to add it at the end, there are no ABI compatibility
> > concerns here.
> >
> > > } MXFContext;
> > >
> > > static const uint8_t uuid_base[] = { 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff,0x29,0xbd };
> > > @@ -630,7 +632,7 @@ static void mxf_write_preface(AVFormatContext *s)
> > >
> > > // operational pattern
> > > mxf_write_local_tag(pb, 16, 0x3B09);
> > > - if (s->oformat == &ff_mxf_opatom_muxer)
> > > + if (mxf->mode == MODE_OPA)
> > > avio_write(pb, opatom_ul, 16);
> > > else
> > > avio_write(pb, op1a_ul, 16);
> > > @@ -723,7 +725,7 @@ static void mxf_write_identification(AVFormatContext *s)
> > > MXFContext *mxf = s->priv_data;
> > > AVIOContext *pb = s->pb;
> > > const char *company = "FFmpeg";
> > > - const char *product = s->oformat != &ff_mxf_opatom_muxer ? "OP1a Muxer" : "OPAtom Muxer";
> > > + const char *product = mxf->mode != MODE_OPA ? "OP1a Muxer" : "OPAtom Muxer";
> > > const char *version;
> > > int length;
> > >
> > > @@ -821,7 +823,7 @@ static void mxf_write_track(AVFormatContext *s, AVStream *st, MXFPackage *packag
> > > // write edit rate
> > > mxf_write_local_tag(pb, 8, 0x4B01);
> > >
> > > - if (st == mxf->timecode_track && s->oformat == &ff_mxf_opatom_muxer) {
> > > + if (st == mxf->timecode_track && mxf->mode == MODE_OPA) {
> > > avio_wb32(pb, mxf->tc.rate.num);
> > > avio_wb32(pb, mxf->tc.rate.den);
> > > } else {
> > > @@ -857,7 +859,7 @@ static void mxf_write_common_fields(AVFormatContext *s, AVStream *st)
> > > // write duration
> > > mxf_write_local_tag(pb, 8, 0x0202);
> > >
> > > - if (st != mxf->timecode_track && s->oformat == &ff_mxf_opatom_muxer && st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> > > + if (st != mxf->timecode_track && mxf->mode == MODE_OPA && st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> > > avio_wb64(pb, mxf->body_offset / mxf->edit_unit_byte_count);
> > > } else {
> > > avio_wb64(pb, mxf->duration);
> > > @@ -1023,7 +1025,7 @@ static int64_t mxf_write_generic_desc(AVFormatContext *s, AVStream *st, const UI
> > > avio_wb32(pb, st->index+2);
> > >
> > > mxf_write_local_tag(pb, 8, 0x3001);
> > > - if (s->oformat == &ff_mxf_d10_muxer) {
> > > + if (mxf->mode == MODE_D10) {
> > > avio_wb32(pb, mxf->time_base.den);
> > > avio_wb32(pb, mxf->time_base.num);
> > > } else {
> > > @@ -1064,6 +1066,7 @@ static inline uint32_t rescale_mastering_luma(AVRational q)
> > >
> > > static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID key)
> > > {
> > > + MXFContext *mxf = s->priv_data;
> > > MXFStreamContext *sc = st->priv_data;
> > > AVIOContext *pb = s->pb;
> > > int stored_width = 0;
> > > @@ -1095,7 +1098,7 @@ static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
> > > mxf_write_local_tag(pb, 4, 0x3202);
> > > avio_wb32(pb, stored_height>>sc->interlaced);
> > >
> > > - if (s->oformat == &ff_mxf_d10_muxer) {
> > > + if (mxf->mode == MODE_D10) {
> > > //Stored F2 Offset
> > > mxf_write_local_tag(pb, 4, 0x3216);
> > > avio_wb32(pb, 0);
> > > @@ -1392,7 +1395,7 @@ static int64_t mxf_write_generic_sound_common(AVFormatContext *s, AVStream *st,
> > > int show_warnings = !mxf->footer_partition_offset;
> > > int64_t pos = mxf_write_generic_desc(s, st, key);
> > >
> > > - if (s->oformat == &ff_mxf_opatom_muxer) {
> > > + if (mxf->mode == MODE_OPA) {
> > > mxf_write_local_tag(pb, 8, 0x3002);
> > > avio_wb64(pb, mxf->body_offset / mxf->edit_unit_byte_count);
> > > }
> > > @@ -1406,17 +1409,17 @@ static int64_t mxf_write_generic_sound_common(AVFormatContext *s, AVStream *st,
> > > avio_wb32(pb, st->codecpar->sample_rate);
> > > avio_wb32(pb, 1);
> > >
> > > - if (s->oformat == &ff_mxf_d10_muxer) {
> > > + if (mxf->mode == MODE_D10) {
> > > mxf_write_local_tag(pb, 1, 0x3D04);
> > > avio_w8(pb, 0);
> > > }
> > >
> > > mxf_write_local_tag(pb, 4, 0x3D07);
> > > if (mxf->channel_count == -1) {
> > > - if (show_warnings && (s->oformat == &ff_mxf_d10_muxer) && (st->codecpar->channels != 4) && (st->codecpar->channels != 8))
> > > + if (show_warnings && (mxf->mode == MODE_D10) && (st->codecpar->channels != 4) && (st->codecpar->channels != 8))
> > > av_log(s, AV_LOG_WARNING, "the number of audio channels shall be 4 or 8 : the output will not comply to MXF D-10 specs, use -d10_channelcount to fix this\n");
> > > avio_wb32(pb, st->codecpar->channels);
> > > - } else if (s->oformat == &ff_mxf_d10_muxer) {
> > > + } else if (mxf->mode == MODE_D10) {
> > > if (show_warnings && (mxf->channel_count < st->codecpar->channels))
> > > av_log(s, AV_LOG_WARNING, "d10_channelcount < actual number of audio channels : some channels will be discarded\n");
> > > if (show_warnings && (mxf->channel_count != 4) && (mxf->channel_count != 8))
> > > @@ -1916,7 +1919,7 @@ static int mxf_write_partition(AVFormatContext *s, int bodysid,
> > > avio_wb32(pb, index_byte_count ? indexsid : 0); // indexSID
> > >
> > > // BodyOffset
> > > - if (bodysid && mxf->edit_units_count && mxf->body_partitions_count && s->oformat != &ff_mxf_opatom_muxer)
> > > + if (bodysid && mxf->edit_units_count && mxf->body_partitions_count && mxf->mode != MODE_OPA)
> > > avio_wb64(pb, mxf->body_offset);
> > > else
> > > avio_wb64(pb, 0);
> > > @@ -1924,7 +1927,7 @@ static int mxf_write_partition(AVFormatContext *s, int bodysid,
> > > avio_wb32(pb, bodysid); // bodySID
> > >
> > > // operational pattern
> > > - if (s->oformat == &ff_mxf_opatom_muxer)
> > > + if (mxf->mode == MODE_OPA)
> > > avio_write(pb, opatom_ul, 16);
> > > else
> > > avio_write(pb, op1a_ul, 16);
> > > @@ -2343,6 +2346,7 @@ static const UID *mxf_get_mpeg2_codec_ul(AVCodecParameters *par)
> > > static int mxf_parse_mpeg2_frame(AVFormatContext *s, AVStream *st,
> > > AVPacket *pkt, MXFIndexEntry *e)
> > > {
> > > + MXFContext *mxf = s->priv_data;
> > > MXFStreamContext *sc = st->priv_data;
> > > uint32_t c = -1;
> > > int i;
> > > @@ -2399,7 +2403,7 @@ static int mxf_parse_mpeg2_frame(AVFormatContext *s, AVStream *st,
> > > }
> > > }
> > > }
> > > - if (s->oformat != &ff_mxf_d10_muxer)
> > > + if (mxf->mode != MODE_D10)
> > > sc->codec_ul = mxf_get_mpeg2_codec_ul(st->codecpar);
> > > return !!sc->codec_ul;
> > > }
> > > @@ -2466,7 +2470,7 @@ static int mxf_write_header(AVFormatContext *s)
> > > if (!s->nb_streams)
> > > return -1;
> > >
> > > - if (s->oformat == &ff_mxf_opatom_muxer && s->nb_streams !=1) {
> > > + if (mxf->mode == MODE_OPA && s->nb_streams !=1) {
> > > av_log(s, AV_LOG_ERROR, "there must be exactly one stream for mxf opatom\n");
> > > return -1;
> > > }
> > > @@ -2474,6 +2478,13 @@ static int mxf_write_header(AVFormatContext *s)
> > > if (!av_dict_get(s->metadata, "comment_", NULL, AV_DICT_IGNORE_SUFFIX))
> > > mxf->store_user_comments = 0;
> > >
> > > + mxf->mode = MODE_OP1a;
> > > + if (s->oformat) {
> > > + if (!strcmp("mxf", s->oformat->name)) mxf->mode = MODE_OP1a;
> > > + else if (!strcmp("mxf_d10",s->oformat->name)) mxf->mode = MODE_D10;
> > > + else if (!strcmp("mxf_opatom", s->oformat->name)) mxf->mode = MODE_OPA;
> > > + }
> > > +
> > > for (i = 0; i < s->nb_streams; i++) {
> > > AVStream *st = s->streams[i];
> > > MXFStreamContext *sc = av_mallocz(sizeof(*sc));
> > > @@ -2482,7 +2493,7 @@ static int mxf_write_header(AVFormatContext *s)
> > > st->priv_data = sc;
> > > sc->index = -1;
> > >
> > > - if (((i == 0) ^ (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO)) && s->oformat != &ff_mxf_opatom_muxer) {
> > > + if (((i == 0) ^ (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO)) && mxf->mode != MODE_OPA) {
> > > av_log(s, AV_LOG_ERROR, "there must be exactly one video stream and it must be the first one\n");
> > > return -1;
> > > }
> > > @@ -2526,12 +2537,12 @@ static int mxf_write_header(AVFormatContext *s)
> > >
> > > sc->video_bit_rate = st->codecpar->bit_rate;
> > >
> > > - if (s->oformat == &ff_mxf_d10_muxer ||
> > > + if (mxf->mode == MODE_D10 ||
> > > st->codecpar->codec_id == AV_CODEC_ID_DNXHD ||
> > > st->codecpar->codec_id == AV_CODEC_ID_DVVIDEO)
> > > mxf->cbr_index = 1;
> > >
> > > - if (s->oformat == &ff_mxf_d10_muxer) {
> > > + if (mxf->mode == MODE_D10) {
> > > int ntsc = mxf->time_base.den != 25;
> > > int ul_index;
> > >
> > > @@ -2569,7 +2580,7 @@ static int mxf_write_header(AVFormatContext *s)
> > > return -1;
> > > }
> > > avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
> > > - if (s->oformat == &ff_mxf_d10_muxer) {
> > > + if (mxf->mode == MODE_D10) {
> > > if (st->index != 1) {
> > > av_log(s, AV_LOG_ERROR, "MXF D-10 only support one audio track\n");
> > > return -1;
> > > @@ -2581,7 +2592,7 @@ static int mxf_write_header(AVFormatContext *s)
> > > sc->index = INDEX_D10_AUDIO;
> > > sc->container_ul = ((MXFStreamContext*)s->streams[0]->priv_data)->container_ul;
> > > sc->frame_size = 4 + 8 * av_rescale_rnd(st->codecpar->sample_rate, mxf->time_base.num, mxf->time_base.den, AV_ROUND_UP) * 4;
> > > - } else if (s->oformat == &ff_mxf_opatom_muxer) {
> > > + } else if (mxf->mode == MODE_OPA) {
> > > AVRational tbc = av_inv_q(mxf->audio_edit_rate);
> > >
> > > if (st->codecpar->codec_id != AV_CODEC_ID_PCM_S16LE &&
> > > @@ -2647,7 +2658,7 @@ static int mxf_write_header(AVFormatContext *s)
> > > present[sc->index]++;
> > > }
> > >
> > > - if (s->oformat == &ff_mxf_d10_muxer || s->oformat == &ff_mxf_opatom_muxer) {
> > > + if (mxf->mode == MODE_D10 || mxf->mode == MODE_OPA) {
> > > mxf->essence_container_count = 1;
> > > }
> > >
> > > @@ -2818,7 +2829,7 @@ static void mxf_compute_edit_unit_byte_count(AVFormatContext *s)
> > > MXFContext *mxf = s->priv_data;
> > > int i;
> > >
> > > - if (s->oformat == &ff_mxf_opatom_muxer) {
> > > + if (mxf->mode == MODE_OPA) {
> > > MXFStreamContext *sc = s->streams[0]->priv_data;
> > > mxf->edit_unit_byte_count = sc->frame_size;
> > > return;
> > > @@ -2889,7 +2900,7 @@ static int mxf_write_packet(AVFormatContext *s, AVPacket *pkt)
> > > mxf_compute_edit_unit_byte_count(s);
> > > }
> > >
> > > - if (s->oformat == &ff_mxf_opatom_muxer)
> > > + if (mxf->mode == MODE_OPA)
> > > return mxf_write_opatom_packet(s, pkt, &ie);
> > >
> > > if (!mxf->header_written) {
> > > @@ -2937,7 +2948,7 @@ static int mxf_write_packet(AVFormatContext *s, AVPacket *pkt)
> > >
> > > mxf_write_klv_fill(s);
> > > avio_write(pb, sc->track_essence_element_key, 16); // write key
> > > - if (s->oformat == &ff_mxf_d10_muxer &&
> > > + if (mxf->mode == MODE_D10 &&
> > > st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> > > mxf_write_d10_audio_packet(s, st, pkt);
> > > } else {
> > > @@ -2959,7 +2970,7 @@ static void mxf_write_random_index_pack(AVFormatContext *s)
> > > avio_write(pb, random_index_pack_key, 16);
> > > klv_encode_ber_length(pb, 28 + 12LL*mxf->body_partitions_count);
> > >
> > > - if (mxf->edit_unit_byte_count && s->oformat != &ff_mxf_opatom_muxer)
> > > + if (mxf->edit_unit_byte_count && mxf->mode != MODE_OPA)
> > > avio_wb32(pb, 1); // BodySID of header partition
> > > else
> > > avio_wb32(pb, 0);
> > > @@ -2983,7 +2994,7 @@ static int mxf_write_footer(AVFormatContext *s)
> > > int i, err;
> > >
> > > if (!mxf->header_written ||
> > > - (s->oformat == &ff_mxf_opatom_muxer && !mxf->body_partition_offset)) {
> > > + (mxf->mode == MODE_OPA && !mxf->body_partition_offset)) {
> > > /* reason could be invalid options/not supported codec/out of memory */
> > > return AVERROR_UNKNOWN;
> > > }
> > > @@ -2992,7 +3003,7 @@ static int mxf_write_footer(AVFormatContext *s)
> > >
> > > mxf_write_klv_fill(s);
> > > mxf->footer_partition_offset = avio_tell(pb);
> > > - if (mxf->edit_unit_byte_count && s->oformat != &ff_mxf_opatom_muxer) { // no need to repeat index
> > > + if (mxf->edit_unit_byte_count && mxf->mode != MODE_OPA) { // no need to repeat index
> > > if ((err = mxf_write_partition(s, 0, 0, footer_partition_key, 0)) < 0)
> > > return err;
> > > } else {
> > > @@ -3006,7 +3017,7 @@ static int mxf_write_footer(AVFormatContext *s)
> > > mxf_write_random_index_pack(s);
> > >
> > > if (s->pb->seekable & AVIO_SEEKABLE_NORMAL) {
> > > - if (s->oformat == &ff_mxf_opatom_muxer) {
> > > + if (mxf->mode == MODE_OPA) {
> > > /* rewrite body partition to update lengths */
> > > avio_seek(pb, mxf->body_partition_offset[0], SEEK_SET);
> > > if ((err = mxf_write_opatom_body_partition(s)) < 0)
> > > @@ -3014,7 +3025,7 @@ static int mxf_write_footer(AVFormatContext *s)
> > > }
> > >
> > > avio_seek(pb, 0, SEEK_SET);
> > > - if (mxf->edit_unit_byte_count && s->oformat != &ff_mxf_opatom_muxer) {
> > > + if (mxf->edit_unit_byte_count && mxf->mode != MODE_OPA) {
> > > if ((err = mxf_write_partition(s, 1, 2, header_closed_partition_key, 1)) < 0)
> > > return err;
> > > mxf_write_klv_fill(s);
> > >
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
--
Thanks,
Limin Wang
More information about the ffmpeg-devel
mailing list