[4/4] movenc: allow alternative hvc1 h.265 codec tag

Message ID 20170615185254.10559-4-stebbins@jetheaddev.com
State New
Headers show

Commit Message

John Stebbins June 15, 2017, 6:52 p.m.
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 | 1 +
 1 file changed, 1 insertion(+)

Comments

Martin Storsjö June 19, 2017, 10:23 a.m. | #1
On Thu, 15 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 | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 0c1508f..2ff4270 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -4435,6 +4435,7 @@ const AVCodecTag codec_mp4_tags[] = {
>     { AV_CODEC_ID_MPEG4       , MKTAG('m', 'p', '4', 'v') },
>     { AV_CODEC_ID_H264        , MKTAG('a', 'v', 'c', '1') },
>     { AV_CODEC_ID_HEVC        , MKTAG('h', 'e', 'v', '1') },
> +    { AV_CODEC_ID_HEVC        , MKTAG('h', 'v', 'c', '1') },
>     { AV_CODEC_ID_AAC         , MKTAG('m', 'p', '4', 'a') },
>     { AV_CODEC_ID_MP4ALS      , MKTAG('m', 'p', '4', 'a') },
>     { AV_CODEC_ID_MPEG2VIDEO  , MKTAG('m', 'p', '4', 'v') },
> -- 
> 2.9.4

This (and the rest of your patches except 2/4 as you mentioned yourself) 
look good and are a sensible addition.

However what I'm left wondering is as a next step, after making this 
possible at all, is what should be the default? If the default isn't 
compatible with apple tools, I'm pretty sure it'll be a very frequently 
asked question at least. I guess it's doable via the -tag flag on the 
command line or something like that, but it feels a little clunky.

But don't let that hold up your patches, since they generally are a step 
in the right direction.

// Martin
John Stebbins June 19, 2017, 2:36 p.m. | #2
On 06/19/2017 03:23 AM, Martin Storsjö wrote:
> On Thu, 15 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 | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index 0c1508f..2ff4270 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -4435,6 +4435,7 @@ const AVCodecTag codec_mp4_tags[] = {
>>     { AV_CODEC_ID_MPEG4       , MKTAG('m', 'p', '4', 'v') },
>>     { AV_CODEC_ID_H264        , MKTAG('a', 'v', 'c', '1') },
>>     { AV_CODEC_ID_HEVC        , MKTAG('h', 'e', 'v', '1') },
>> +    { AV_CODEC_ID_HEVC        , MKTAG('h', 'v', 'c', '1') },
>>     { AV_CODEC_ID_AAC         , MKTAG('m', 'p', '4', 'a') },
>>     { AV_CODEC_ID_MP4ALS      , MKTAG('m', 'p', '4', 'a') },
>>     { AV_CODEC_ID_MPEG2VIDEO  , MKTAG('m', 'p', '4', 'v') },
>> -- 
>> 2.9.4
> This (and the rest of your patches except 2/4 as you mentioned yourself) 
> look good and are a sensible addition.
>
> However what I'm left wondering is as a next step, after making this 
> possible at all, is what should be the default? If the default isn't 
> compatible with apple tools, I'm pretty sure it'll be a very frequently 
> asked question at least. I guess it's doable via the -tag flag on the 
> command line or something like that, but it feels a little clunky.
>
> But don't let that hold up your patches, since they generally are a step 
> in the right direction.
>
>

Yes, it can be set with avconv "-tag:v hvc1". I'm inclined to agree that the default should change.  But I think we need
more feedback as well.  hvc1 works on the players I've tried on Linux (mpv, vlc, totem, mplayer).  But someone should
check the common windows players.  Changing the default is also likely to break fate.  So a patch to change the default
would probably also have to fix fate.
Jan Ekström June 19, 2017, 5:42 p.m. | #3
Hi,

On Mon, Jun 19, 2017 at 1:23 PM, Martin Storsjö <martin@martin.st> wrote:
> However what I'm left wondering is as a next step, after making this
> possible at all, is what should be the default? If the default isn't
> compatible with apple tools, I'm pretty sure it'll be a very frequently
> asked question at least. I guess it's doable via the -tag flag on the
> command line or something like that, but it feels a little clunky.
>

Just in case nobody had noticed yet, there are actual differences in
the limitations of what you can do with the streams in the container
between 'hev1' and 'hvc1'.

Quoting 14496-15:
- "For a video stream that a particular sample entry applies to, the
video parameter set, sequence parameter sets, and picture parameter
sets, shall be stored only in the sample entry when the sample entry
name is 'hvc1', and may be stored in the sample entry and the samples
when the sample entry name is 'hev1'."
- "When the sample entry name is 'hvc1', the default and mandatory
value of array_completeness is 1 for arrays of all types of parameter
sets, and 0 for all other arrays. When the sample entry name is
'hev1', the default value of array_completeness is 0 for all arrays."

So yea, HEVC1 let us do a lot of stuff including putting parameter
sets in-band, while HVC1 specifically notes that we don't have
anything like that in-band.

Jan
Martin Storsjö June 19, 2017, 6:17 p.m. | #4
On Mon, 19 Jun 2017, Jan Ekstrom wrote:

> Hi,
>
> On Mon, Jun 19, 2017 at 1:23 PM, Martin Storsjö <martin@martin.st> wrote:
>> However what I'm left wondering is as a next step, after making this
>> possible at all, is what should be the default? If the default isn't
>> compatible with apple tools, I'm pretty sure it'll be a very frequently
>> asked question at least. I guess it's doable via the -tag flag on the
>> command line or something like that, but it feels a little clunky.
>>
>
> Just in case nobody had noticed yet, there are actual differences in
> the limitations of what you can do with the streams in the container
> between 'hev1' and 'hvc1'.
>
> Quoting 14496-15:
> - "For a video stream that a particular sample entry applies to, the
> video parameter set, sequence parameter sets, and picture parameter
> sets, shall be stored only in the sample entry when the sample entry
> name is 'hvc1', and may be stored in the sample entry and the samples
> when the sample entry name is 'hev1'."
> - "When the sample entry name is 'hvc1', the default and mandatory
> value of array_completeness is 1 for arrays of all types of parameter
> sets, and 0 for all other arrays. When the sample entry name is
> 'hev1', the default value of array_completeness is 0 for all arrays."
>
> So yea, HEVC1 let us do a lot of stuff including putting parameter
> sets in-band, while HVC1 specifically notes that we don't have
> anything like that in-band.

Yup, I know that. Although this is pretty much what it was with avc1 in 
mp4 already, right? So in such a case, having to manually specify the tag 
when you want to do the less-common case of using in-band parameter set 
changes isn't all that bad to me.

Thinking even further, when writing non-fragmented mp4, you could even 
check the content of the muxed NAL units, and decide on what tag to use 
when finishing the moov.

// Martin

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0c1508f..2ff4270 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -4435,6 +4435,7 @@  const AVCodecTag codec_mp4_tags[] = {
     { AV_CODEC_ID_MPEG4       , MKTAG('m', 'p', '4', 'v') },
     { AV_CODEC_ID_H264        , MKTAG('a', 'v', 'c', '1') },
     { AV_CODEC_ID_HEVC        , MKTAG('h', 'e', 'v', '1') },
+    { AV_CODEC_ID_HEVC        , MKTAG('h', 'v', 'c', '1') },
     { AV_CODEC_ID_AAC         , MKTAG('m', 'p', '4', 'a') },
     { AV_CODEC_ID_MP4ALS      , MKTAG('m', 'p', '4', 'a') },
     { AV_CODEC_ID_MPEG2VIDEO  , MKTAG('m', 'p', '4', 'v') },