On 06/15/2017 11:52 AM, John Stebbins wrote: > This simplifies the code and adds the capability to support alternative > tags for the same codec_id > --- > libavformat/movenc.c | 137 > ++++++++++++++++++++++++++------------------------- > 1 file changed, 70 insertions(+), 67 deletions(-) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index 6c3d36c..eb12431 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -32,6 +32,7 @@ > #include "isom.h" > #include "avc.h" > > +#include "libavcodec/internal.h" > #include "libavcodec/bitstream.h" > #include "libavcodec/put_bits.h" > #include "libavcodec/vc1_common.h" > @@ -778,50 +779,39 @@ static int mov_write_avid_tag(AVIOContext *pb, MOVTrack > *track) > return 0; > } > > -static int mp4_get_codec_tag(AVFormatContext *s, MOVTrack *track) > +static int validate_codec_tag(const AVCodecTag * const tags[], unsigned int > tag, > + enum AVCodecID id) > { > - int tag = track->par->codec_tag; > - > - if (!ff_codec_get_tag(ff_mp4_obj_type, track->par->codec_id)) > - return 0; > + const AVCodecTag *avctag; > + int n; > > - if (track->par->codec_id == AV_CODEC_ID_H264) tag = > MKTAG('a','v','c','1'); > - else if (track->par->codec_id == AV_CODEC_ID_HEVC) tag = > MKTAG('h','e','v','1'); > - else if (track->par->codec_id == AV_CODEC_ID_AC3) tag = > MKTAG('a','c','-','3'); > - else if (track->par->codec_id == AV_CODEC_ID_DIRAC) tag = > MKTAG('d','r','a','c'); > - else if (track->par->codec_id == AV_CODEC_ID_MOV_TEXT) tag = > MKTAG('t','x','3','g'); > - else if (track->par->codec_id == AV_CODEC_ID_VC1) tag = > MKTAG('v','c','-','1'); > - else if (track->par->codec_type == AVMEDIA_TYPE_VIDEO) tag = > MKTAG('m','p','4','v'); > - else if (track->par->codec_type == AVMEDIA_TYPE_AUDIO) tag = > MKTAG('m','p','4','a'); > - else if (track->par->codec_id == AV_CODEC_ID_DVD_SUBTITLE) tag = > MKTAG('m','p','4','s'); > - > - return tag; > + for (n = 0; tags[n]; n++) { > + avctag = tags[n]; > + while (avctag->id != AV_CODEC_ID_NONE) { > + if (avpriv_toupper4(avctag->tag) == avpriv_toupper4(tag) && > + avctag->id == id) > + return 1; > + avctag++; > + } > + } > + return 0; > } > > -static const AVCodecTag codec_ipod_tags[] = { > - { AV_CODEC_ID_H264, MKTAG('a','v','c','1') }, > - { AV_CODEC_ID_MPEG4, MKTAG('m','p','4','v') }, > - { AV_CODEC_ID_AAC, MKTAG('m','p','4','a') }, > - { AV_CODEC_ID_ALAC, MKTAG('a','l','a','c') }, > - { AV_CODEC_ID_AC3, MKTAG('a','c','-','3') }, > - { AV_CODEC_ID_MOV_TEXT, MKTAG('t','x','3','g') }, > - { AV_CODEC_ID_MOV_TEXT, MKTAG('t','e','x','t') }, > - { AV_CODEC_ID_NONE, 0 }, > -}; > - > -static int ipod_get_codec_tag(AVFormatContext *s, MOVTrack *track) > +static int get_codec_tag(AVFormatContext *s, MOVTrack *track) > { > int tag = track->par->codec_tag; > > - // keep original tag for subs, ipod supports both formats > - if (!(track->par->codec_type == AVMEDIA_TYPE_SUBTITLE && > - (tag == MKTAG('t', 'x', '3', 'g') || > - tag == MKTAG('t', 'e', 'x', 't')))) > - tag = ff_codec_get_tag(codec_ipod_tags, track->par->codec_id); > - > - if (!av_match_ext(s->filename, "m4a") && !av_match_ext(s->filename, > "m4v")) > - av_log(s, AV_LOG_WARNING, "Warning, extension is not .m4a nor .m4v " > - "Quicktime/Ipod might not play the file\n"); > + if (!tag || (s->strict_std_compliance >= FF_COMPLIANCE_NORMAL && > + s->oformat->codec_tag && > + !validate_codec_tag(s->oformat->codec_tag, tag, > + track->par->codec_id))) { > + int n = 0; > + > + tag = 0; > + while (!tag && s->oformat->codec_tag[n]) > + tag = ff_codec_get_tag(s->oformat->codec_tag[n++], > + track->par->codec_id); > + } > > return tag; > } > @@ -926,42 +916,24 @@ static int mov_get_codec_tag(AVFormatContext *s, > MOVTrack *track) > return tag; > } > > -static const AVCodecTag codec_3gp_tags[] = { > - { AV_CODEC_ID_H263, MKTAG('s','2','6','3') }, > - { AV_CODEC_ID_H264, MKTAG('a','v','c','1') }, > - { AV_CODEC_ID_MPEG4, MKTAG('m','p','4','v') }, > - { AV_CODEC_ID_AAC, MKTAG('m','p','4','a') }, > - { AV_CODEC_ID_AMR_NB, MKTAG('s','a','m','r') }, > - { AV_CODEC_ID_AMR_WB, MKTAG('s','a','w','b') }, > - { AV_CODEC_ID_MOV_TEXT, MKTAG('t','x','3','g') }, > - { AV_CODEC_ID_NONE, 0 }, > -}; > - > -static const AVCodecTag codec_f4v_tags[] = { > - { AV_CODEC_ID_MP3, MKTAG('.','m','p','3') }, > - { AV_CODEC_ID_AAC, MKTAG('m','p','4','a') }, > - { AV_CODEC_ID_H264, MKTAG('a','v','c','1') }, > - { AV_CODEC_ID_VP6A, MKTAG('V','P','6','A') }, > - { AV_CODEC_ID_VP6F, MKTAG('V','P','6','F') }, > - { AV_CODEC_ID_NONE, 0 }, > -}; > - > static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) > { > int tag; > > if (track->mode == MODE_MP4 || track->mode == MODE_PSP) > - tag = mp4_get_codec_tag(s, track); > - else if (track->mode == MODE_ISM) { > - tag = mp4_get_codec_tag(s, track); > - if (!tag && track->par->codec_id == AV_CODEC_ID_WMAPRO) > - tag = MKTAG('w', 'm', 'a', ' '); > - } else if (track->mode == MODE_IPOD) > - tag = ipod_get_codec_tag(s, track); > - else if (track->mode & MODE_3GP) > - tag = ff_codec_get_tag(codec_3gp_tags, track->par->codec_id); > + tag = get_codec_tag(s, track); > + else if (track->mode == MODE_ISM) > + tag = get_codec_tag(s, track); > + else if (track->mode == MODE_IPOD) { > + if (!av_match_ext(s->filename, "m4a") && > + !av_match_ext(s->filename, "m4v")) > + av_log(s, AV_LOG_WARNING, "Warning, extension is not .m4a nor > .m4v " > + "Quicktime/Ipod might not play the file\n"); > + tag = get_codec_tag(s, track ); > + } else if (track->mode & MODE_3GP) > + tag = get_codec_tag(s, track); > else if (track->mode == MODE_F4V) > - tag = ff_codec_get_tag(codec_f4v_tags, track->par->codec_id); > + tag = get_codec_tag(s, track); > else > tag = mov_get_codec_tag(s, track); > > @@ -4444,6 +4416,17 @@ error: > return res; > } > > +static const AVCodecTag codec_3gp_tags[] = { > + { AV_CODEC_ID_H263, MKTAG('s','2','6','3') }, > + { AV_CODEC_ID_H264, MKTAG('a','v','c','1') }, > + { AV_CODEC_ID_MPEG4, MKTAG('m','p','4','v') }, > + { AV_CODEC_ID_AAC, MKTAG('m','p','4','a') }, > + { AV_CODEC_ID_AMR_NB, MKTAG('s','a','m','r') }, > + { AV_CODEC_ID_AMR_WB, MKTAG('s','a','w','b') }, > + { AV_CODEC_ID_MOV_TEXT, MKTAG('t','x','3','g') }, > + { AV_CODEC_ID_NONE, 0 }, > +}; > + > const AVCodecTag codec_mp4_tags[] = { > { AV_CODEC_ID_MOV_TEXT , MKTAG('t', 'x', '3', 'g') }, > { AV_CODEC_ID_MPEG4 , MKTAG('m', 'p', '4', 'v') }, > @@ -4474,6 +4457,26 @@ const AVCodecTag codec_ism_tags[] = { > { AV_CODEC_ID_NONE , 0 }, > }; > > +static const AVCodecTag codec_ipod_tags[] = { > + { AV_CODEC_ID_H264, MKTAG('a','v','c','1') }, > + { AV_CODEC_ID_MPEG4, MKTAG('m','p','4','v') }, > + { AV_CODEC_ID_AAC, MKTAG('m','p','4','a') }, > + { AV_CODEC_ID_ALAC, MKTAG('a','l','a','c') }, > + { AV_CODEC_ID_AC3, MKTAG('a','c','-','3') }, > + { AV_CODEC_ID_MOV_TEXT, MKTAG('t','x','3','g') }, > + { AV_CODEC_ID_MOV_TEXT, MKTAG('t','e','x','t') }, > + { AV_CODEC_ID_NONE, 0 }, > +}; > + > +static const AVCodecTag codec_f4v_tags[] = { > + { AV_CODEC_ID_MP3, MKTAG('.','m','p','3') }, > + { AV_CODEC_ID_AAC, MKTAG('m','p','4','a') }, > + { AV_CODEC_ID_H264, MKTAG('a','v','c','1') }, > + { AV_CODEC_ID_VP6A, MKTAG('V','P','6','A') }, > + { AV_CODEC_ID_VP6F, MKTAG('V','P','6','F') }, > + { AV_CODEC_ID_NONE, 0 }, > +}; > + > #if CONFIG_MOV_MUXER > MOV_CLASS(mov) > AVOutputFormat ff_mov_muxer = {
Martin, I'm looking at this and thinking I've unnecessarily duplicated what has already been done in mux.c to look up codec_tag. When the codec_tag can be determined by a simple lookup in oformat->codec_tag table, mux.c has already done this and it's done the necessary validation. So I think I can eliminate both validate_codec_tag and get_codec_tag from this patch and simplify to "tag = track->par->codec_tag" in mov_find_codec_tag everywhere that I'm currently calling get_codec_tag. Do you agree? -- John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 D0F7
signature.asc
Description: OpenPGP digital signature
_______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel