On Wed, Jan 06, 2021 at 06:14:07PM +0100, Marton Balint wrote: > > > On Wed, 6 Jan 2021, Andreas Rheinhardt wrote: > > > lance.lmw...@gmail.com: > > > From: Limin Wang <lance.lmw...@gmail.com> > > > > > > Signed-off-by: Limin Wang <lance.lmw...@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@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > 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". -- Thanks, Limin Wang _______________________________________________ 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".