diff mbox

[FFmpeg-devel,1/2] avformat/mov: parse sdtp atom and set the pkt disposable flag accordingly

Message ID 20190927143732.455148-1-matthieu.bouron@gmail.com
State New
Headers show

Commit Message

Matthieu Bouron Sept. 27, 2019, 2:37 p.m. UTC 
Allows the creation of the sdtp atom while remuxing MP4 to MP4. This
atom is required by Apple devices (iPhone, Apple TV) in order to accept
2160p medias.
---
 libavformat/isom.h |  2 ++
 libavformat/mov.c  | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

Comments

Carl Eugen Hoyos Sept. 27, 2019, 2:59 p.m. UTC | #1 
Am Fr., 27. Sept. 2019 um 16:45 Uhr schrieb Matthieu Bouron
<matthieu.bouron@gmail.com>:
>
> ---
>  tests/ref/fate/hapqa-extract-snappy1-to-hapalphaonly  | 2 +-
>  tests/ref/fate/hapqa-extract-snappy1-to-hapq          | 2 +-
>  tests/ref/fate/hapqa-extract-snappy16-to-hapalphaonly | 2 +-
>  tests/ref/fate/hapqa-extract-snappy16-to-hapq         | 2 +-

You have to merge this into the first patch (if that patch is ok).

Carl Eugen
Derek Buitenhuis Sept. 27, 2019, 3:14 p.m. UTC | #2 
On 27/09/2019 15:37, Matthieu Bouron wrote:
> Allows the creation of the sdtp atom while remuxing MP4 to MP4. This
> atom is required by Apple devices (iPhone, Apple TV) in order to accept
> 2160p medias.

Can you point to a document about this (for informational purposes)?

Is this valid for both QTFF and ISOBMFF?

>  static void mov_update_dts_shift(MOVStreamContext *sc, int duration)
>  {
>      if (duration < 0) {
> @@ -6767,6 +6801,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = {
>  { MKTAG('s','t','s','z'), mov_read_stsz }, /* sample size */
>  { MKTAG('s','t','t','s'), mov_read_stts },
>  { MKTAG('s','t','z','2'), mov_read_stsz }, /* compact sample size */
> +{ MKTAG('s','d','t','p'), mov_read_sdtp }, /* independant and disposable samples */

Spelling mistake.

- Derek
Matthieu Bouron Sept. 27, 2019, 3:45 p.m. UTC | #3 
On Fri, Sep 27, 2019 at 04:14:33PM +0100, Derek Buitenhuis wrote:
> On 27/09/2019 15:37, Matthieu Bouron wrote:
> > Allows the creation of the sdtp atom while remuxing MP4 to MP4. This
> > atom is required by Apple devices (iPhone, Apple TV) in order to accept
> > 2160p medias.
> 
> Can you point to a document about this (for informational purposes)?

For the MP4 container, tt is part of ISO/IEC14496-12 but it looks like it
is not public.

It is also described here for the QT container:
https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFChap2/qtff2.html#//apple_ref/doc/uid/TP40000939-CH204-SW46

The specification differs in wording and presentation between the two, but
the final encoded values of each flags are the same.

For both specs the flags are stored in 1 byte.

For sample_is_depended_on in QT, you have to use:

kQTSampleDependency_NoOtherSampleDependsOnThisSample = 1<<3,
kQTSampleDependency_OtherSamplesDependOnThisSample   = 1<<2,

In MP4, the spec describe the following layout for the flags:

      unsigned int(2) reserved = 0;  unsigned   int(2)
      sample_depends_on;             unsigned   int(2)
      sample_is_depended_on;         unsigned   int(2)
      sample_has_redundancy;         unsigned   int(2)

And values for sample_is_depended_on:

      0: the dependency of other samples on this sample is unknown
      1: other samples depend on this one (not disposable)
      2: no other sample depends on this one (disposable)
      3: reserved

NOTE: we already support the sdtp atom in movenc.c.

> 
> Is this valid for both QTFF and ISOBMFF?
> 
> >  static void mov_update_dts_shift(MOVStreamContext *sc, int duration)
> >  {
> >      if (duration < 0) {
> > @@ -6767,6 +6801,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = {
> >  { MKTAG('s','t','s','z'), mov_read_stsz }, /* sample size */
> >  { MKTAG('s','t','t','s'), mov_read_stts },
> >  { MKTAG('s','t','z','2'), mov_read_stsz }, /* compact sample size */
> > +{ MKTAG('s','d','t','p'), mov_read_sdtp }, /* independant and disposable samples */
> 
> Spelling mistake.

Fixed locally.

Thanks.
Derek Buitenhuis Sept. 27, 2019, 3:46 p.m. UTC | #4 
On 27/09/2019 16:45, Matthieu Bouron wrote:
> Fixed locally.

+1

- Derek
Andreas Rheinhardt Sept. 28, 2019, 11:27 p.m. UTC | #5 
Matthieu Bouron:
> On Fri, Sep 27, 2019 at 04:14:33PM +0100, Derek Buitenhuis wrote:
>> On 27/09/2019 15:37, Matthieu Bouron wrote:
>>> Allows the creation of the sdtp atom while remuxing MP4 to MP4. This
>>> atom is required by Apple devices (iPhone, Apple TV) in order to accept
>>> 2160p medias.
>>
>> Can you point to a document about this (for informational purposes)?
> 
> For the MP4 container, tt is part of ISO/IEC14496-12 but it looks like it
> is not public.
> 
ISO/IEC 14496-12 is one of the publicly available ISO standards [1],
as are two amendments to it.

- Andreas

[1]: https://standards.iso.org/ittf/PubliclyAvailableStandards/index.html
Matthieu Bouron Oct. 7, 2019, 3:06 p.m. UTC | #6 
On Fri, Sep 27, 2019 at 04:37:31PM +0200, Matthieu Bouron wrote:
> Allows the creation of the sdtp atom while remuxing MP4 to MP4. This
> atom is required by Apple devices (iPhone, Apple TV) in order to accept
> 2160p medias.
> ---
>  libavformat/isom.h |  2 ++
>  libavformat/mov.c  | 41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 69452cae8e..4943b80ccf 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -163,6 +163,8 @@ typedef struct MOVStreamContext {
>      int64_t *chunk_offsets;
>      unsigned int stts_count;
>      MOVStts *stts_data;
> +    unsigned int sdtp_count;
> +    uint8_t *sdtp_data;
>      unsigned int ctts_count;
>      unsigned int ctts_allocated_size;
>      MOVStts *ctts_data;
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 8e916a28c6..7dfa07b45e 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -2959,6 +2959,40 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      return 0;
>  }
>  
> +static int mov_read_sdtp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> +{
> +    AVStream *st;
> +    MOVStreamContext *sc;
> +    int64_t i, entries;
> +
> +    if (c->fc->nb_streams < 1)
> +        return 0;
> +    st = c->fc->streams[c->fc->nb_streams-1];
> +    sc = st->priv_data;
> +
> +    avio_r8(pb); /* version */
> +    avio_rb24(pb); /* flags */
> +    entries = atom.size - 4;
> +
> +    av_log(c->fc, AV_LOG_TRACE, "track[%u].sdtp.entries = %" PRId64 "\n",
> +           c->fc->nb_streams - 1, entries);
> +
> +    if (sc->sdtp_data)
> +        av_log(c->fc, AV_LOG_WARNING, "Duplicated SDTP atom\n");
> +    av_freep(&sc->sdtp_data);
> +    sc->sdtp_count = 0;
> +
> +    sc->sdtp_data = av_mallocz(entries);
> +    if (!sc->sdtp_data)
> +        return AVERROR(ENOMEM);
> +
> +    for (i = 0; i < entries && !pb->eof_reached; i++)
> +        sc->sdtp_data[i] = avio_r8(pb);
> +    sc->sdtp_count = i;
> +
> +    return 0;
> +}
> +
>  static void mov_update_dts_shift(MOVStreamContext *sc, int duration)
>  {
>      if (duration < 0) {
> @@ -6767,6 +6801,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = {
>  { MKTAG('s','t','s','z'), mov_read_stsz }, /* sample size */
>  { MKTAG('s','t','t','s'), mov_read_stts },
>  { MKTAG('s','t','z','2'), mov_read_stsz }, /* compact sample size */
> +{ MKTAG('s','d','t','p'), mov_read_sdtp }, /* independant and disposable samples */
>  { MKTAG('t','k','h','d'), mov_read_tkhd }, /* track header */
>  { MKTAG('t','f','d','t'), mov_read_tfdt },
>  { MKTAG('t','f','h','d'), mov_read_tfhd }, /* track fragment header */
> @@ -7231,6 +7266,7 @@ static int mov_read_close(AVFormatContext *s)
>          av_freep(&sc->sample_sizes);
>          av_freep(&sc->keyframes);
>          av_freep(&sc->stts_data);
> +        av_freep(&sc->sdtp_data);
>          av_freep(&sc->stps_data);
>          av_freep(&sc->elst_data);
>          av_freep(&sc->rap_group);
> @@ -7820,6 +7856,11 @@ static int mov_read_packet(AVFormatContext *s, AVPacket *pkt)
>      }
>      if (st->discard == AVDISCARD_ALL)
>          goto retry;
> +    if (sc->sdtp_data && sc->current_sample <= sc->sdtp_count) {
> +        uint8_t sample_flags = sc->sdtp_data[sc->current_sample - 1];
> +        uint8_t sample_is_depended_on = (sample_flags >> 2) & 0x3;
> +        pkt->flags |= sample_is_depended_on == MOV_SAMPLE_DEPENDENCY_NO ? AV_PKT_FLAG_DISPOSABLE : 0;
> +    }
>      pkt->flags |= sample->flags & AVINDEX_KEYFRAME ? AV_PKT_FLAG_KEY : 0;
>      pkt->pos = sample->pos;
>  
> -- 
> 2.23.0
> 

Ping.
Derek Buitenhuis Oct. 7, 2019, 9:19 p.m. UTC | #7 
On 07/10/2019 16:06, Matthieu Bouron wrote:
> Ping.
> 

No objections from me.

- Derek
Matthieu Bouron Oct. 12, 2019, 12:40 p.m. UTC | #8 
On Mon, Oct 07, 2019 at 10:19:59PM +0100, Derek Buitenhuis wrote:
> On 07/10/2019 16:06, Matthieu Bouron wrote:
> > Ping.
> > 
> 
> No objections from me.

Pushed (with the patch updating fate squashed into this patch).

Thanks,
diff mbox

Patch

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 69452cae8e..4943b80ccf 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -163,6 +163,8 @@  typedef struct MOVStreamContext {
     int64_t *chunk_offsets;
     unsigned int stts_count;
     MOVStts *stts_data;
+    unsigned int sdtp_count;
+    uint8_t *sdtp_data;
     unsigned int ctts_count;
     unsigned int ctts_allocated_size;
     MOVStts *ctts_data;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 8e916a28c6..7dfa07b45e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2959,6 +2959,40 @@  static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     return 0;
 }
 
+static int mov_read_sdtp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+    AVStream *st;
+    MOVStreamContext *sc;
+    int64_t i, entries;
+
+    if (c->fc->nb_streams < 1)
+        return 0;
+    st = c->fc->streams[c->fc->nb_streams-1];
+    sc = st->priv_data;
+
+    avio_r8(pb); /* version */
+    avio_rb24(pb); /* flags */
+    entries = atom.size - 4;
+
+    av_log(c->fc, AV_LOG_TRACE, "track[%u].sdtp.entries = %" PRId64 "\n",
+           c->fc->nb_streams - 1, entries);
+
+    if (sc->sdtp_data)
+        av_log(c->fc, AV_LOG_WARNING, "Duplicated SDTP atom\n");
+    av_freep(&sc->sdtp_data);
+    sc->sdtp_count = 0;
+
+    sc->sdtp_data = av_mallocz(entries);
+    if (!sc->sdtp_data)
+        return AVERROR(ENOMEM);
+
+    for (i = 0; i < entries && !pb->eof_reached; i++)
+        sc->sdtp_data[i] = avio_r8(pb);
+    sc->sdtp_count = i;
+
+    return 0;
+}
+
 static void mov_update_dts_shift(MOVStreamContext *sc, int duration)
 {
     if (duration < 0) {
@@ -6767,6 +6801,7 @@  static const MOVParseTableEntry mov_default_parse_table[] = {
 { MKTAG('s','t','s','z'), mov_read_stsz }, /* sample size */
 { MKTAG('s','t','t','s'), mov_read_stts },
 { MKTAG('s','t','z','2'), mov_read_stsz }, /* compact sample size */
+{ MKTAG('s','d','t','p'), mov_read_sdtp }, /* independant and disposable samples */
 { MKTAG('t','k','h','d'), mov_read_tkhd }, /* track header */
 { MKTAG('t','f','d','t'), mov_read_tfdt },
 { MKTAG('t','f','h','d'), mov_read_tfhd }, /* track fragment header */
@@ -7231,6 +7266,7 @@  static int mov_read_close(AVFormatContext *s)
         av_freep(&sc->sample_sizes);
         av_freep(&sc->keyframes);
         av_freep(&sc->stts_data);
+        av_freep(&sc->sdtp_data);
         av_freep(&sc->stps_data);
         av_freep(&sc->elst_data);
         av_freep(&sc->rap_group);
@@ -7820,6 +7856,11 @@  static int mov_read_packet(AVFormatContext *s, AVPacket *pkt)
     }
     if (st->discard == AVDISCARD_ALL)
         goto retry;
+    if (sc->sdtp_data && sc->current_sample <= sc->sdtp_count) {
+        uint8_t sample_flags = sc->sdtp_data[sc->current_sample - 1];
+        uint8_t sample_is_depended_on = (sample_flags >> 2) & 0x3;
+        pkt->flags |= sample_is_depended_on == MOV_SAMPLE_DEPENDENCY_NO ? AV_PKT_FLAG_DISPOSABLE : 0;
+    }
     pkt->flags |= sample->flags & AVINDEX_KEYFRAME ? AV_PKT_FLAG_KEY : 0;
     pkt->pos = sample->pos;