On 06/14/2017 01:06 PM, Martin Storsjö wrote: > On Wed, 14 Jun 2017, John Stebbins wrote: > >> >> On 06/14/2017 12:13 PM, Martin Storsjö wrote: >>> On Wed, 14 Jun 2017, John Stebbins wrote: >>> >>>> On 06/13/2017 11:16 PM, Martin Storsjö wrote: >>>>> On Tue, 13 Jun 2017, John Stebbins wrote: >>>>> >>>>>> On 06/13/2017 12:13 PM, Martin Storsjö wrote: >>>>>>> On Mon, 12 Jun 2017, John Stebbins wrote: >>>>>>> >>>>>>>> If AVCodecParameters.codec_tag is 'hvc1' use it instead of 'hev1' for >>>>>>>> h.265 streams. QuickTime (and other Apple software) requires 'hvc1'. >>>>>>>> --- >>>>>>>> libavformat/movenc.c | 11 +++++++++-- >>>>>>>> 1 file changed, 9 insertions(+), 2 deletions(-) >>>>>>>> >>>>>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>>>>>>> index e389029..8d89a7a 100644 >>>>>>>> --- a/libavformat/movenc.c >>>>>>>> +++ b/libavformat/movenc.c >>>>>>>> @@ -726,7 +726,10 @@ static int mov_write_hvcc_tag(AVIOContext *pb, >>>>>>>> MOVTrack *track) >>>>>>>> >>>>>>>> avio_wb32(pb, 0); >>>>>>>> ffio_wfourcc(pb, "hvcC"); >>>>>>>> - ff_isom_write_hvcc(pb, track->vos_data, track->vos_len, 0); >>>>>>>> + if (track->tag == MKTAG('h','v','c','1')) >>>>>>>> + ff_isom_write_hvcc(pb, track->vos_data, track->vos_len, 1); >>>>>>>> + else >>>>>>>> + ff_isom_write_hvcc(pb, track->vos_data, track->vos_len, 0); >>>>>>>> return update_size(pb, pos); >>>>>>>> } >>>>>>>> >>>>>>>> @@ -786,6 +789,8 @@ static int mp4_get_codec_tag(AVFormatContext *s, >>>>>>>> MOVTrack *track) >>>>>>>> return 0; >>>>>>>> >>>>>>>> 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','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'); >>>>>>>> @@ -4496,7 +4501,9 @@ AVOutputFormat ff_mp4_muxer = { >>>>>>>> .write_packet = mov_write_packet, >>>>>>>> .write_trailer = mov_write_trailer, >>>>>>>> .flags = AVFMT_GLOBALHEADER | AVFMT_ALLOW_FLUSH | >>>>>>>> AVFMT_TS_NEGATIVE, >>>>>>>> - .codec_tag = (const AVCodecTag* const []){ >>>>>>>> ff_mp4_obj_type, 0 }, >>>>>>>> + .codec_tag = (const AVCodecTag* const []){ >>>>>>>> + ff_codec_movvideo_tags, ff_codec_movaudio_tags, >>>>>>>> + ff_codec_movsubtitle_tags, 0 }, >>>>>>>> .priv_class = &mp4_muxer_class, >>>>>>> I guess this change is because libavformat refuses to allow you to set a >>>>>>> custom codec_tag unless it is found in the codec_tag list, right? Does >>>>>>> this have any other practical implications? I feel a little uneasy about >>>>>>> this part of it... >>>>>>> >>>>>>> >>>>>> Yes, this change was to fix result of validate_codec_tag in mux.c. >>>>>> >>>>>> I was curious about this myself. I could only find 2 places that this >>>>>> codec_tag array is used. One in >>>>>> validate_codec_tag which is the instance that needed fixing, and another >>>>>> in avconv where it is checking if the source >>>>>> codec_tag is valid for the output format when doing stream copy. In >>>>>> both of these cases ff_mp4_obj_type is wrong for >>>>>> codec_tag so I'm not sure why this was used in the first place. >>>>>> According to git history, this was used from the >>>>>> beginning, but makes no sense. I also looked at who calls >>>>>> validate_codec_tag just to be sure it's being used >>>>>> consistently. It's only used by avformat_write_header. >>>>>> >>>>>> I also ran fate to check for regressions. >>>>> Ok, that makes sense. >>>>> >>>>> In that case though, I would very much prefer to do this in two separate >>>>> steps; one that removes the .codec_tag part from the AVOutputFormat, with >>>>> a thorough explanation of this (pretty much what you just wrote) and >>>>> saying that this will allow the caller to indicate that actual codec tag >>>>> he wants to be used, and then this one on top that allows you to use hvc1. >>>>> >>>>> >>>> Ok, I'll do that. There are 2 additional places ff_mp4_obj_type is used >>>> for codec_tag. psp and ismv also use it. >>>> Should I just do the same thing to them, or would you rather something >>>> more tailored for those use cases (as it is done >>>> for ipod)? I don't happen to know what fourcc are permitted in these >>>> variants. There's nothing in the code that >>>> currently limits what can be put in these. >>> Hmm, after removing codec_tag in this case, what happens if you try to mux >>> an unsupported codec? Would it be even better to use another list like the >>> current one, but which contains the actual real tags that can end up >>> written in the mp4 file, so you can have both HEVC alternatives in there? >>> >>> >> I think you misread something. I didn't "remove" codec_tag. I set it to { >> ff_codec_movvideo_tags, >> ff_codec_movaudio_tags, ff_codec_movsubtitle_tags, 0 } > Yep, and now on a second thought I think that's wrong. Wouldn't that allow > you to (attempt to) mux codecs which really aren't supposed to go into an > mp4, say SVQ3? > > So instead it'd be better to mirror the actual list of codecs that are > allowed in mp4, that currently are in ff_mp4_obj_type, but with sensible > tag names (including both hev1 and hvc1)? > >
Understood. I assumed mov and mp4 allowed pretty much the same stream types. But I see this is incorrect. I'll roll that into a new patch. Do you want to have a more limited list for psp and/or ismv? If so, do you happen to know a good source for what should be in that list? -- 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