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 }

-- 
John      GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01  83F0 49F1 D7B2 60D4 D0F7


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to