| Message ID | 1472643361-10118-5-git-send-email-erkki.seppala.ext@nokia.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi! 2016-08-31 13:35 GMT+02:00 Erkki Seppälä <erkki.seppala.ext@nokia.com>: Iirc, this has the (intended or unintended) side-effect of making mov output of ffmpeg (the app) suddenly defaulting to an additional track. Is this correct? Is it intended? Thank you, Carl Eugen
Hello! On 08/31/2016 04:04 PM, Carl Eugen Hoyos wrote: > Iirc, this has the (intended or unintended) side-effect of making > mov output of ffmpeg (the app) suddenly defaulting to an > additional track. > Is this correct? Is it intended? Hmm, it seems mov files still have one track according to these commands: % ffmpeg -i foo.png foo.mov % MP4Box -diso foo.mov % grep ' TrackID=' foo_info.xml <TrackHeaderBox CreationTime="0" ModificationTime="0" TrackID="1" Duration="40" Width="2.00" Height="2.00"> Certtainly that kind of variation would not be intended. The fate tests (lavf-moov) have bitwise comparisons for mov results, but of course they don't use the defaults. So I imagine you are referring to the enable_tracks function. The change is probably not strictly required for this functionality to work in general and it was added for uniformity in dealing with this the same as with other track types. Thanks for input!
Hi! 2016-08-31 15:28 GMT+02:00 Erkki Seppälä <erkki.seppala.ext@nokia.com>: > > On 08/31/2016 04:04 PM, Carl Eugen Hoyos wrote: >> >> Iirc, this has the (intended or unintended) side-effect of making >> mov output of ffmpeg (the app) suddenly defaulting to an >> additional track. >> Is this correct? Is it intended? > > Hmm, it seems mov files still have one track according to these > commands: > > % ffmpeg -i foo.png foo.mov Sorry! I meant "defaulting to an additional track in the output mov file if the input file contains such a data track (instead of ignoring this track of the input file by default)". Carl Eugen
On 08/31/2016 04:42 PM, Carl Eugen Hoyos wrote: > Sorry! No problem :). > I meant "defaulting to an additional track in the output mov file if the > input file contains such a data track (instead of ignoring this track > of the input file by default)". I'll look into how that part of code works and try it out a bit. Thanks!
Hi, (Cc'ing as it's been some time.) On 08/31/2016 04:42 PM, Carl Eugen Hoyos wrote: > I meant "defaulting to an additional track in the output mov file if the > input file contains such a data track (instead of ignoring this track > of the input file by default)". So I looked into it and indeed the data tracks end up with the enabled flag enabled. If this affects anything is a bit unclear to me, perhaps in some players? It seems to me they would unlikely try to play something they don't know how to - in a worse case perhaps the can stop or crash. It seems also 'tmcd' tracks end up being enabled (but not by this function). I imagine this is by design. I can remove the enabled-flag for the next patch set for the AVMEDIA_TYPE_DATA tracks (this does not include tmcd as it doesn't have a corresponding stream). Or perhaps the appropriate solution would be to not write such data tracks into QuickTime video files in the first place and only support them in .mp4 by reverting the codec support for MOV with these kind of codecs. Thanks for input!
(Oops, sorry about the subject, our MTA helpfully prepended it.)
Hi! 2016-09-06 13:13 GMT+02:00 Erkki Seppälä <erkki.seppala.ext@nokia.com>: > (Cc'ing as it's been some time.) Please don't. > On 08/31/2016 04:42 PM, Carl Eugen Hoyos wrote: >> >> I meant "defaulting to an additional track in the output mov file if the >> input file contains such a data track (instead of ignoring this track >> of the input file by default)". > > So I looked into it and indeed the data tracks end up with the enabled > flag enabled. If this affects anything is a bit unclear to me, perhaps in > some players? It seems to me they would unlikely try to play something > they don't know how to - in a worse case perhaps the can stop or crash. Sorry, that is not what I meant (although it may or may not be an issue). I meant that for the same ffmpeg command line and the same input file, I believe that with your patch, the output file (suddenly) has one track more than before (because the data codec is now listed in AVOutputFormat, the ffmpeg cli will try to add this track if the input file provides such a codec). Maybe this is wrong because the data codec does not produce a new track if it is passed to the muxer? Iirc, my question was equivalent to: Can you just remove the additional ".data_codec=" lines or is it intended that they are now default codecs? (Or am I simply wrong and they either do not lead to a new track and / or ffmpeg simply ignores them because it only fills video, audio and subtitle tracks?) Carl Eugen
On 09/06/2016 02:37 PM, Carl Eugen Hoyos wrote: > I meant that for the same ffmpeg command line and the same > input file, I believe that with your patch, the output file (suddenly) > has one track more than before (because the data codec is now > listed in AVOutputFormat, the ffmpeg cli will try to add this track > if the input file provides such a codec). Ok, so now I finally understood the case and indeed if I have a file with video+data, converting it to a .mov without any particular mapping options results in a video+data track, instead of just video as what would happen if the .data_codecs are not defined. (I used ffmpeg -i foo.mp4 -codec copy bar.mp4 (or .mov) for testing where foo.mp4 has a data track.) A curiosity: if I copy a file with two video tracks and two data tracks I end up with a file with one video track and two data tracks (unless I use -map : to copy all). Would it be even better to not copy the data tracks at all by default, so not set the .data_codec field for any format? Or another view is that it would seem useful though that all the data tracks would be copied (but so does copying all video tracks). Or it could copy all the tracks that have track references to tracks copied by default.. > Can you just remove the additional ".data_codec=" lines or is > it intended that they are now default codecs? > (Or am I simply wrong and they either do not lead to a new > track and / or ffmpeg simply ignores them because it only fills > video, audio and subtitle tracks?) This seems the correct solution. My next patch set will remove the .data_codec at least from output formats other than the .mp4. Thanks!
2016-09-06 15:07 GMT+02:00 Erkki Seppälä <erkki.seppala.ext@nokia.com>: > Would it be even better to not copy the data tracks at > all by default, so not set the .data_codec field for any > format? This was my original question for which I do not know the answer: If you feel it doesn't hurt not to copy the track I am all for not changing behaviour. Thank you, Carl Eugen
On 09/06/2016 09:26 PM, Carl Eugen Hoyos wrote: > 2016-09-06 15:07 GMT+02:00 Erkki Seppälä <erkki.seppala.ext@nokia.com>: > >> Would it be even better to not copy the data tracks at >> all by default, so not set the .data_codec field for any >> format? > > This was my original question for which I do not know > the answer: If you feel it doesn't hurt not to copy the > track I am all for not changing behaviour. I will then opt for not changing the behavior. If the behavior is deemed change-worthy in the future, it can be in its own patch. Thanks.
diff --git a/libavformat/isom.c b/libavformat/isom.c index cb457dd..1a90d00 100644 --- a/libavformat/isom.c +++ b/libavformat/isom.c @@ -355,6 +355,11 @@ const AVCodecTag ff_codec_movsubtitle_tags[] = { { AV_CODEC_ID_NONE, 0 }, }; +const AVCodecTag ff_codec_metadata_tags[] = { + { AV_CODEC_ID_META, MKTAG('m', 'e', 't', 'a') }, + { AV_CODEC_ID_NONE, 0 }, +}; + /* map numeric codes from mdhd atom to ISO 639 */ /* cf. QTFileFormat.pdf p253, qtff.pdf p205 */ /* http://developer.apple.com/documentation/mac/Text/Text-368.html */ diff --git a/libavformat/isom.h b/libavformat/isom.h index df6c15a..49c8996 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -33,6 +33,7 @@ extern const AVCodecTag ff_mp4_obj_type[]; extern const AVCodecTag ff_codec_movvideo_tags[]; extern const AVCodecTag ff_codec_movaudio_tags[]; extern const AVCodecTag ff_codec_movsubtitle_tags[]; +extern const AVCodecTag ff_codec_metadata_tags[]; int ff_mov_iso639_to_lang(const char lang[4], int mp4); int ff_mov_lang_to_iso639(unsigned code, char to[4]); diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 571c2a7..f02458b 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1470,6 +1470,8 @@ static int mov_get_codec_tag(AVFormatContext *s, MOVTrack *track) } } else if (track->par->codec_type == AVMEDIA_TYPE_SUBTITLE) tag = ff_codec_get_tag(ff_codec_movsubtitle_tags, track->par->codec_id); + else if (track->par->codec_type == AVMEDIA_TYPE_DATA) + tag = ff_codec_get_tag(ff_codec_metadata_tags, track->par->codec_id); } return tag; @@ -2242,6 +2244,9 @@ static int mov_write_hdlr_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra } else if (track->par->codec_tag == MKTAG('t','m','c','d')) { hdlr_type = "tmcd"; descr = "TimeCodeHandler"; + } else if (track->par->codec_type == AVMEDIA_TYPE_DATA) { + hdlr_type = "meta"; + descr = "DataHandler"; } else { char tag_buf[32]; av_get_codec_tag_string(tag_buf, sizeof(tag_buf), @@ -5308,6 +5313,7 @@ static void enable_tracks(AVFormatContext *s) case AVMEDIA_TYPE_VIDEO: case AVMEDIA_TYPE_AUDIO: case AVMEDIA_TYPE_SUBTITLE: + case AVMEDIA_TYPE_DATA: if (enabled[i] > 1) mov->per_stream_grouping = 1; if (!enabled[i] && first[i] >= 0) @@ -6110,6 +6116,7 @@ AVOutputFormat ff_mov_muxer = { .audio_codec = AV_CODEC_ID_AAC, .video_codec = CONFIG_LIBX264_ENCODER ? AV_CODEC_ID_H264 : AV_CODEC_ID_MPEG4, + .data_codec = AV_CODEC_ID_META, .write_header = mov_write_header, .write_packet = mov_write_packet, .write_trailer = mov_write_trailer, @@ -6129,6 +6136,7 @@ AVOutputFormat ff_tgp_muxer = { .priv_data_size = sizeof(MOVMuxContext), .audio_codec = AV_CODEC_ID_AMR_NB, .video_codec = AV_CODEC_ID_H263, + .data_codec = AV_CODEC_ID_META, .write_header = mov_write_header, .write_packet = mov_write_packet, .write_trailer = mov_write_trailer, @@ -6148,6 +6156,7 @@ AVOutputFormat ff_mp4_muxer = { .audio_codec = AV_CODEC_ID_AAC, .video_codec = CONFIG_LIBX264_ENCODER ? AV_CODEC_ID_H264 : AV_CODEC_ID_MPEG4, + .data_codec = AV_CODEC_ID_META, .write_header = mov_write_header, .write_packet = mov_write_packet, .write_trailer = mov_write_trailer, @@ -6166,6 +6175,7 @@ AVOutputFormat ff_psp_muxer = { .audio_codec = AV_CODEC_ID_AAC, .video_codec = CONFIG_LIBX264_ENCODER ? AV_CODEC_ID_H264 : AV_CODEC_ID_MPEG4, + .data_codec = AV_CODEC_ID_META, .write_header = mov_write_header, .write_packet = mov_write_packet, .write_trailer = mov_write_trailer, @@ -6183,6 +6193,7 @@ AVOutputFormat ff_tg2_muxer = { .priv_data_size = sizeof(MOVMuxContext), .audio_codec = AV_CODEC_ID_AMR_NB, .video_codec = AV_CODEC_ID_H263, + .data_codec = AV_CODEC_ID_META, .write_header = mov_write_header, .write_packet = mov_write_packet, .write_trailer = mov_write_trailer, @@ -6201,6 +6212,7 @@ AVOutputFormat ff_ipod_muxer = { .priv_data_size = sizeof(MOVMuxContext), .audio_codec = AV_CODEC_ID_AAC, .video_codec = AV_CODEC_ID_H264, + .data_codec = AV_CODEC_ID_META, .write_header = mov_write_header, .write_packet = mov_write_packet, .write_trailer = mov_write_trailer, @@ -6219,6 +6231,7 @@ AVOutputFormat ff_ismv_muxer = { .priv_data_size = sizeof(MOVMuxContext), .audio_codec = AV_CODEC_ID_AAC, .video_codec = AV_CODEC_ID_H264, + .data_codec = AV_CODEC_ID_META, .write_header = mov_write_header, .write_packet = mov_write_packet, .write_trailer = mov_write_trailer, @@ -6237,6 +6250,7 @@ AVOutputFormat ff_f4v_muxer = { .priv_data_size = sizeof(MOVMuxContext), .audio_codec = AV_CODEC_ID_AAC, .video_codec = AV_CODEC_ID_H264, + .data_codec = AV_CODEC_ID_META, .write_header = mov_write_header, .write_packet = mov_write_packet, .write_trailer = mov_write_trailer,