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
